From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
Andrew Cooper <andrew.cooper3@citrix.com>,
Julien Grall <julien@xen.org>,
Stefano Stabellini <sstabellini@kernel.org>,
Anthony PERARD <anthony.perard@vates.tech>,
Michal Orzel <michal.orzel@amd.com>
Subject: Re: [PATCH] domain: use unsigned loop induction variable in complete_domain_destroy()
Date: Thu, 5 Mar 2026 09:15:59 +0100 [thread overview]
Message-ID: <aak7v6VGUcNREXAW@macbook.local> (raw)
In-Reply-To: <1f80f87e-8e58-45de-8785-1efec5169176@suse.com>
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.
prev parent reply other threads:[~2026-03-05 8:16 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-26 9:01 [PATCH] domain: use unsigned loop induction variable in complete_domain_destroy() Jan Beulich
2026-03-04 15:38 ` Roger Pau Monné
2026-03-04 15:48 ` Jan Beulich
2026-03-04 17:36 ` Roger Pau Monné
2026-03-05 8:07 ` Jan Beulich
2026-03-05 8:15 ` Roger Pau Monné [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aak7v6VGUcNREXAW@macbook.local \
--to=roger.pau@citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=anthony.perard@vates.tech \
--cc=jbeulich@suse.com \
--cc=julien@xen.org \
--cc=michal.orzel@amd.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.