[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] domain: use unsigned loop induction variable in complete_domain_destroy()


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Thu, 5 Mar 2026 09:15:59 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=tXrrTFjBnPUpxvOLmHQfRDee3fworYRtsARCth8g27o=; b=WDQ+r2/VagFGjOQbeNOPzEqSEd63UP4YFNjhhFJFyli2+wTvaTYb0knDqzZ9cn/0MD7D4f4AtLxI6nwaXHRI9EIwghntaXp3VoDmh5g7+frhmloTM214KXHpAZ8ujbgmsQCCyZVbSlrT9kr5BWzOFNgQ9ItBmehucM1clPh+z2j1j28QFSL+dppVplE91+l+bVLOpIVdftWt2FH/Pu3SuoDRYU8eDXRZUO1BsMDgiivgsLmKGoVEbHCTJ2AQWkJ6EcYGvu8T++k/YEOHpuvHJaa+dmzhPHaATRZl4in32ISXr2tXAnqaoVlhoEvDWMT7/iiDEDfia//7u4sa2W+yRg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=C8EscSIkYhwy37DDGXCQKkzLGtK9lLCV1+hvrG/xQ4DI4sbWdSmsI5+YII3LRvnu78RloW+OXUP5MwaSnx+Ce2TmGKYuytpZd2A6JOm87IWOp+K7UvNKnOhnRGBtpRaNOIugRjEv6SNp/jH8w69mcmwURPlwp383RbfP9Ve3CdPhgtFTcmabDabQ6zsRkiR7SSlV305DkZnY5FoeuirH2Va+m+sIRILPhXVgbOpg/Qqdz2GFaqycerqLw5znbZsTZBl7/iw7XTzFccUXebgNYFjcyrLw63/17CE6Cp+BY3wey760r7OkhCQaaboTEcIHiekEzFLY3QIg0zvFJj/Mrg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>
  • Delivery-date: Thu, 05 Mar 2026 08:16:13 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.