linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Woodhouse <dwmw2@infradead.org>
To: Thomas Gleixner <tglx@linutronix.de>,
	Usama Arif <usama.arif@bytedance.com>,
	arjan@linux.intel.com
Cc: mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
	hpa@zytor.com, x86@kernel.org, pbonzini@redhat.com,
	paulmck@kernel.org, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org, rcu@vger.kernel.org, mimoja@mimoja.de,
	hewenliang4@huawei.com, thomas.lendacky@amd.com,
	seanjc@google.com, pmenzel@molgen.mpg.de,
	fam.zheng@bytedance.com, punit.agrawal@bytedance.com,
	simon.evans@bytedance.com, liangma@liangbit.com
Subject: Re: [PATCH v6 01/11] x86/apic/x2apic: Fix parallel handling of cluster_mask
Date: Tue, 07 Feb 2023 11:27:20 +0000	[thread overview]
Message-ID: <921cfe295fcd398168e5454e01193045de312688.camel@infradead.org> (raw)
In-Reply-To: <d37f3af69df09ff542024ed93a37865b28dfa86e.camel@infradead.org>

[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]

On Tue, 2023-02-07 at 10:57 +0000, David Woodhouse wrote:
> 
> > +       /*
> > +        * On post boot hotplug iterate over the present CPUs to handle the
> > +        * case of partial clusters as they might be presented by
> > +        * virtualization.
> > +        */
> > +       for_each_present_cpu(cpu_i) {
> 
> 
> So... if this CPU was *present* at boot time (and if any other CPU in
> this cluster was present), it will already have a cluster_mask.
> 
> Which means we get here in two cases: 
> 
>  • This CPU wasn't actually present (was just 'possible') at boot time.
>    (Is that actually a thing that happens?)
> 
>  • This CPU was present but no other CPU in this cluster was actually
>    brought up at boot time so the cluster_mask wasn't allocated.
> 
> The code looks right, I don't grok the comment about partial clusters
> and virtualization, and would have worded it something along the above
> lines?

As I get my head around that, I think the code needs to change too.
What if we *unplug* the only CPU in a cluster (present→possible), then
add a new one in the same cluster? The new one would get a new
cluster_mask. Which is kind of OK for now but then if we re-add the
original CPU it'd continue to use its old cluster_mask.

Now, that's kind of weird if it's physical CPUs because that cluster is
within a given chip, isn't it? But with virtualization maybe that's
something that could happen, and it doesn't hurt to be completely safe
by using for_each_possible_cpu() instead?

Now looks like this:


	/*
	 * On post boot hotplug for a CPU which was not present at boot time,
	 * iterate over all possible CPUs (even those which are not present
	 * any more) to find any existing cluster mask.
	 */
	for_each_possible_cpu(cpu_i) {


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

  reply	other threads:[~2023-02-07 11:27 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-02 21:56 [PATCH v6 00/11] Parallel CPU bringup for x86_64 Usama Arif
2023-02-02 21:56 ` [PATCH v6 01/11] x86/apic/x2apic: Fix parallel handling of cluster_mask Usama Arif
2023-02-06 23:20   ` Thomas Gleixner
2023-02-07 10:57     ` David Woodhouse
2023-02-07 11:27       ` David Woodhouse [this message]
2023-02-07 14:24         ` Thomas Gleixner
2023-02-07 19:53           ` David Woodhouse
2023-02-07 20:58             ` Thomas Gleixner
2023-02-07 14:22       ` Thomas Gleixner
2023-02-02 21:56 ` [PATCH v6 02/11] cpu/hotplug: Move idle_thread_get() to <linux/smpboot.h> Usama Arif
2023-02-06 23:33   ` Thomas Gleixner
2023-02-07  1:24     ` Paul E. McKenney
2023-02-02 21:56 ` [PATCH v6 03/11] cpu/hotplug: Add dynamic parallel bringup states before CPUHP_BRINGUP_CPU Usama Arif
2023-02-06 23:43   ` Thomas Gleixner
2023-02-02 21:56 ` [PATCH v6 04/11] x86/smpboot: Reference count on smpboot_setup_warm_reset_vector() Usama Arif
2023-02-06 23:48   ` Thomas Gleixner
     [not found]     ` <57195f701f6d1d70ec440c9a28cbee4cfb81dc41.camel@amazon.co.uk>
2023-02-07 14:39       ` Thomas Gleixner
2023-02-07 16:50         ` Sean Christopherson
2023-02-07 19:48         ` [EXTERNAL][PATCH " David Woodhouse
2023-02-02 21:56 ` [PATCH v6 05/11] x86/smpboot: Split up native_cpu_up into separate phases and document them Usama Arif
2023-02-06 23:59   ` Thomas Gleixner
2023-02-02 21:56 ` [PATCH v6 06/11] x86/smpboot: Support parallel startup of secondary CPUs Usama Arif
2023-02-02 22:30   ` David Woodhouse
2023-02-02 22:50     ` [External] " Usama Arif
2023-02-03  8:14       ` David Woodhouse
2023-02-03 14:41         ` Arjan van de Ven
2023-02-03 18:17   ` Sean Christopherson
2023-02-07  0:07   ` Thomas Gleixner
2023-02-02 21:56 ` [PATCH v6 07/11] x86/smpboot: Disable parallel boot for AMD CPUs Usama Arif
2023-02-03 19:48   ` Kim Phillips
     [not found]     ` <d5ec64236ba75f0d3f3718fb69b2cb9169d8af0a.camel@amazon.co.uk>
2023-02-03 21:45       ` Kim Phillips
2023-02-03 22:25         ` [EXTERNAL][PATCH " David Woodhouse
2023-02-04  9:07     ` [PATCH " David Woodhouse
2023-02-04 10:09     ` David Woodhouse
2023-02-04 15:40     ` David Woodhouse
2023-02-04 18:18       ` Arjan van de Ven
2023-02-04 22:31         ` David Woodhouse
2023-02-05 22:13           ` [External] " Usama Arif
2023-02-06  8:05             ` David Woodhouse
2023-02-06 12:11               ` Usama Arif
2023-02-06 18:07                 ` Sean Christopherson
2023-02-06 17:58       ` Kim Phillips
2023-02-07 16:27         ` Kim Phillips
2023-02-07  0:23       ` Thomas Gleixner
2023-02-07 10:04         ` David Woodhouse
2023-02-07 14:44           ` Thomas Gleixner
2023-02-07  0:09   ` Thomas Gleixner
     [not found]     ` <cbd9e88e738dc0c479e87121ca82431731905c73.camel@amazon.co.uk>
2023-02-07 14:46       ` Thomas Gleixner
2023-02-02 21:56 ` [PATCH v6 08/11] x86/smpboot: Send INIT/SIPI/SIPI to secondary CPUs in parallel Usama Arif
2023-02-07  0:28   ` Thomas Gleixner
2023-02-02 21:56 ` [PATCH v6 09/11] x86/mtrr: Avoid repeated save of MTRRs on boot-time CPU bringup Usama Arif
2023-02-02 21:56 ` [PATCH v6 10/11] x86/smpboot: Serialize topology updates for secondary bringup Usama Arif
2023-02-02 21:56 ` [PATCH v6 11/11] x86/smpboot: reuse timer calibration Usama Arif
2023-02-07  0:31   ` Thomas Gleixner
2023-02-07 23:16   ` Arjan van de Ven
2023-02-07 23:55     ` Thomas Gleixner
2023-02-05 19:17 ` [PATCH v6 00/11] Parallel CPU bringup for x86_64 Russ Anderson
2023-02-06  8:28   ` David Woodhouse
2023-02-06 12:18     ` [External] " Usama Arif

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=921cfe295fcd398168e5454e01193045de312688.camel@infradead.org \
    --to=dwmw2@infradead.org \
    --cc=arjan@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=fam.zheng@bytedance.com \
    --cc=hewenliang4@huawei.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=liangma@liangbit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mimoja@mimoja.de \
    --cc=mingo@redhat.com \
    --cc=paulmck@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=pmenzel@molgen.mpg.de \
    --cc=punit.agrawal@bytedance.com \
    --cc=rcu@vger.kernel.org \
    --cc=seanjc@google.com \
    --cc=simon.evans@bytedance.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --cc=usama.arif@bytedance.com \
    --cc=x86@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).