On Thu, Mar 05, 2026 at 09:07:55AM +0100, Jan Beulich wrote:
> On 04.03.2026 18:36, Roger Pau Monné wrote:
> > On Wed, Mar 04, 2026 at 04:48:07PM +0100, Jan Beulich wrote:
> >> On 04.03.2026 16:38, Roger Pau Monné wrote:
> >>> On Thu, Feb 26, 2026 at 10:01:45AM +0100, Jan Beulich wrote:
> >>>> --- a/xen/common/domain.c
> >>>> +++ b/xen/common/domain.c
> >>>> @@ -1475,7 +1475,7 @@ static void cf_check complete_domain_des
> >>>> {
> >>>> struct domain *d = container_of(head, struct domain, rcu);
> >>>> struct vcpu *v;
> >>>> - int i;
> >>>> + unsigned int i;
> >>>>
> >>>> /*
> >>>> * Flush all state for the vCPU previously having run on the
> >>>> current CPU.
> >>>> @@ -1485,7 +1485,7 @@ static void cf_check complete_domain_des
> >>>> */
> >>>> sync_local_execstate();
> >>>>
> >>>> - for ( i = d->max_vcpus - 1; i >= 0; i-- )
> >>>> + for ( i = d->max_vcpus; i-- > 0; )
> >>>
> >>> Is there any reason we need to do those loops backwards?
> >>>
> >>> I would rather do:
> >>>
> >>> for ( i = 0; i < d->max_vcpus; i++ )
> >>>
> >>> ?
> >>
> >> I think it's better to keep like this. The latter of the loops would better
> >> clear d->vcpu[i] (to not leave a dangling pointer), and there may be code
> >> which assumes that for ordinary domains d->vcpu[] is populated
> >> contiguously.
> >> Hardly any code should touch the vCPU-s of a domain destructed this far,
> >> but
> >> still better safe than sorry, I guess.
> >
> > Yes, you are right. sched_destroy_vcpu() relies on this specific
> > top-down calling.
> >
> > Since you are adjusting the code anyway, it might be worth writing
> > down that some functions (like sched_destroy_vcpu()) expect to be
> > called with a top-down order of vCPUs.
>
> I've added
>
> /*
> * Iterating downwards is a requirement here, as e.g. sched_destroy_vcpu()
> * relies on this.
> */
>
> ahead of the first of the two loops.
Thank you for adjusting that.
Roger.
