Re: [PATCH v2] domain: skip more stuff for idle's vCPU-s in vcpu_create()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.02.2026 12:46, Roger Pau Monné wrote:
> On Tue, Feb 17, 2026 at 12:17:35PM +0100, Jan Beulich wrote:
>> On 17.02.2026 12:04, Roger Pau Monné wrote:
>>> On Mon, Feb 16, 2026 at 04:54:30PM +0100, Jan Beulich wrote:
>>>> @@ -516,7 +516,8 @@ struct vcpu *vcpu_create(struct domain *
>>>>      }
>>>>  
>>>>      /* Must be called after making new vcpu visible to for_each_vcpu(). */
>>>> -    vcpu_check_shutdown(v);
>>>> +    if ( !is_idle_domain(d) )
>>>> +        vcpu_check_shutdown(v);
>>>
>>> I would possibly leave this as-is.  I agree that the idle domain will
>>> never shut down, but it's possibly best to needlessly call into
>>> vcpu_check_shutdown() for the idle domain rather than adding the extra
>>> conditional for the common case?
>>
>> I'd prefer to keep it conditional: Calling the function for the idle
>> domain gives a wrong impression, plus it may be the only one where the
>> shutdown lock is used for that domain. We may want to make lock init
>> conditional in domain_create() as well (possibly with other things we
>> needlessly do for idle or more generally system domains).
> 
> I've been thinking about this, and I'm not sure whether it's the best
> approach to avoid initializing locks or lists for the idle
> vCPUs/domain.
> 
> It's certainly good to avoid initializing stuff that consumes memory
> or other resources, but skipping plain initialization (iow: setting of
> values) of fields that are in the respective structs seems dangerous
> to a certain degree.  If for whatever reason we end up with stray
> calls from the idle vCPUs/domain into functions that use those fields
> it's likely safer to have them initialized, rather than tripping into
> some uninitialized pointer or deadlock trying to acquire and
> uninitiated lock.

Otoh without doing so it's pretty unlikely that we would spot such stray
calls. Which better would be avoided imo.

Jan