[PATCH] cpu: mark cpu_possible_mask as __ro_after_init

[PATCH] cpu: mark cpu_possible_mask as __ro_after_init

[Alexey Dobriyan]

cpu_possible_mask is by definition "cpus which could be hotplugged
without reboot" -- property which is fixed after kernel enumerates
motheboard capabilities and hardware configuration.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 kernel/cpu.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -3107,10 +3107,10 @@ const DECLARE_BITMAP(cpu_all_bits, NR_CPUS) = CPU_BITS_ALL;
 EXPORT_SYMBOL(cpu_all_bits);
 
 #ifdef CONFIG_INIT_ALL_POSSIBLE
-struct cpumask __cpu_possible_mask __read_mostly
+struct cpumask __cpu_possible_mask __ro_after_init;
 	= {CPU_BITS_ALL};
 #else
-struct cpumask __cpu_possible_mask __read_mostly;
+struct cpumask __cpu_possible_mask __ro_after_init;
 #endif
 EXPORT_SYMBOL(__cpu_possible_mask);
 

[tip: smp/core] cpu: Mark cpu_possible_mask as __ro_after_init

[tip-bot2 for Alexey Dobriyan]

The following commit has been merged into the smp/core branch of tip:

Commit-ID:     da92df490eeab7a97a3390ff32e0ae091e0dc2eb
Gitweb:        https://git.kernel.org/tip/da92df490eeab7a97a3390ff32e0ae091e0dc2eb
Author:        Alexey Dobriyan <adobriyan@gmail.com>
AuthorDate:    Mon, 05 Feb 2024 13:01:19 +03:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 19 Feb 2024 18:05:47 +01:00

cpu: Mark cpu_possible_mask as __ro_after_init

cpu_possible_mask is by definition "cpus which could be hotplugged without
reboot". It's a property which is fixed after kernel enumerates the
hardware configuration.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/41cd78af-92a3-4f23-8c7a-4316a04a66d8@p183

---
 kernel/cpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index ad7d0b0..7b36b3a 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -3106,10 +3106,10 @@ const DECLARE_BITMAP(cpu_all_bits, NR_CPUS) = CPU_BITS_ALL;
 EXPORT_SYMBOL(cpu_all_bits);
 
 #ifdef CONFIG_INIT_ALL_POSSIBLE
-struct cpumask __cpu_possible_mask __read_mostly
+struct cpumask __cpu_possible_mask __ro_after_init;
 	= {CPU_BITS_ALL};
 #else
-struct cpumask __cpu_possible_mask __read_mostly;
+struct cpumask __cpu_possible_mask __ro_after_init;
 #endif
 EXPORT_SYMBOL(__cpu_possible_mask);
 

Re: [PATCH] cpu: mark cpu_possible_mask as __ro_after_init

[Guenter Roeck]

On Mon, Feb 05, 2024 at 01:01:19PM +0300, Alexey Dobriyan wrote:
> cpu_possible_mask is by definition "cpus which could be hotplugged
> without reboot" -- property which is fixed after kernel enumerates
> motheboard capabilities and hardware configuration.
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
> 
>  kernel/cpu.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -3107,10 +3107,10 @@ const DECLARE_BITMAP(cpu_all_bits, NR_CPUS) = CPU_BITS_ALL;
>  EXPORT_SYMBOL(cpu_all_bits);
>  
>  #ifdef CONFIG_INIT_ALL_POSSIBLE
> -struct cpumask __cpu_possible_mask __read_mostly
> +struct cpumask __cpu_possible_mask __ro_after_init;
>  	= {CPU_BITS_ALL};

I guess you didn't compile test this code.

Guenter

>  #else
> -struct cpumask __cpu_possible_mask __read_mostly;
> +struct cpumask __cpu_possible_mask __ro_after_init;
>  #endif
>  EXPORT_SYMBOL(__cpu_possible_mask);
>  

Re: [PATCH] cpu: mark cpu_possible_mask as __ro_after_init

[Alexey Dobriyan]

On Wed, Feb 21, 2024 at 10:00:26PM -0800, Guenter Roeck wrote:
> On Mon, Feb 05, 2024 at 01:01:19PM +0300, Alexey Dobriyan wrote:
> > cpu_possible_mask is by definition "cpus which could be hotplugged
> > without reboot" -- property which is fixed after kernel enumerates
> > motheboard capabilities and hardware configuration.
> > 
> > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> > ---
> > 
> >  kernel/cpu.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > --- a/kernel/cpu.c
> > +++ b/kernel/cpu.c
> > @@ -3107,10 +3107,10 @@ const DECLARE_BITMAP(cpu_all_bits, NR_CPUS) = CPU_BITS_ALL;
> >  EXPORT_SYMBOL(cpu_all_bits);
> >  
> >  #ifdef CONFIG_INIT_ALL_POSSIBLE
> > -struct cpumask __cpu_possible_mask __read_mostly
> > +struct cpumask __cpu_possible_mask __ro_after_init;
> >  	= {CPU_BITS_ALL};
> 
> I guess you didn't compile test this code.

On parisc, no. Oh well.

> >  #else
> > -struct cpumask __cpu_possible_mask __read_mostly;
> > +struct cpumask __cpu_possible_mask __ro_after_init;

[PATCH v2] cpu: mark cpu_possible_mask as __ro_after_init

[Alexey Dobriyan]

cpu_possible_mask is by definition "cpus which could be hotplugged without
reboot". It's a property which is fixed after kernel enumerates hardware
configuration.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

	v2: fix parisc compilation

 kernel/cpu.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -3107,10 +3107,10 @@ const DECLARE_BITMAP(cpu_all_bits, NR_CPUS) = CPU_BITS_ALL;
 EXPORT_SYMBOL(cpu_all_bits);
 
 #ifdef CONFIG_INIT_ALL_POSSIBLE
-struct cpumask __cpu_possible_mask __read_mostly
+struct cpumask __cpu_possible_mask __ro_after_init
 	= {CPU_BITS_ALL};
 #else
-struct cpumask __cpu_possible_mask __read_mostly;
+struct cpumask __cpu_possible_mask __ro_after_init;
 #endif
 EXPORT_SYMBOL(__cpu_possible_mask);
 

Re: [PATCH v2] cpu: mark cpu_possible_mask as __ro_after_init

[Jonathan Cameron]

On Thu, 22 Feb 2024 14:19:35 +0300
Alexey Dobriyan <adobriyan@gmail.com> wrote:

> cpu_possible_mask is by definition "cpus which could be hotplugged without
> reboot". It's a property which is fixed after kernel enumerates hardware
> configuration.
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>

Causes a crash in this path (via CPU HP testing on qemu)
Pretending to be an AMD Genoa, but I doubt that matters.

topology_hotplug_apic()
-> topo_set_cpuids()
 --> set_cpu_possible(cpu, true);
It should be already set, but the code doesn't check that.

Various possible fixes. Probably easiest is to pass in a 
bool hotplug to topo_set_cpuids() so we don't set the 
possible value if it's coming from hotplug calls.

I can spin a patch, but it will next week probably.

Jonathan

> ---
> 
> 	v2: fix parisc compilation
> 
>  kernel/cpu.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -3107,10 +3107,10 @@ const DECLARE_BITMAP(cpu_all_bits, NR_CPUS) = CPU_BITS_ALL;
>  EXPORT_SYMBOL(cpu_all_bits);
>  
>  #ifdef CONFIG_INIT_ALL_POSSIBLE
> -struct cpumask __cpu_possible_mask __read_mostly
> +struct cpumask __cpu_possible_mask __ro_after_init
>  	= {CPU_BITS_ALL};
>  #else
> -struct cpumask __cpu_possible_mask __read_mostly;
> +struct cpumask __cpu_possible_mask __ro_after_init;
>  #endif
>  EXPORT_SYMBOL(__cpu_possible_mask);
>  

Re: [PATCH v2] cpu: mark cpu_possible_mask as __ro_after_init

[Guenter Roeck]

On Wed, Mar 27, 2024 at 06:10:53PM +0000, Jonathan Cameron wrote:
> On Thu, 22 Feb 2024 14:19:35 +0300
> Alexey Dobriyan <adobriyan@gmail.com> wrote:
> 
> > cpu_possible_mask is by definition "cpus which could be hotplugged without
> > reboot". It's a property which is fixed after kernel enumerates hardware
> > configuration.
> > 
> > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> 
> Causes a crash in this path (via CPU HP testing on qemu)
> Pretending to be an AMD Genoa, but I doubt that matters.
> 

Can you send me the configuration, qemu command line, and commands
executed ? I'd like to add that to my test setup if possible.

Thanks,
Guenter

Re: [PATCH v2] cpu: mark cpu_possible_mask as __ro_after_init

[Jonathan Cameron]

On Wed, 27 Mar 2024 11:36:28 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On Wed, Mar 27, 2024 at 06:10:53PM +0000, Jonathan Cameron wrote:
> > On Thu, 22 Feb 2024 14:19:35 +0300
> > Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >   
> > > cpu_possible_mask is by definition "cpus which could be hotplugged without
> > > reboot". It's a property which is fixed after kernel enumerates hardware
> > > configuration.
> > > 
> > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>  
> > 
> > Causes a crash in this path (via CPU HP testing on qemu)
> > Pretending to be an AMD Genoa, but I doubt that matters.
> >   
> 
> Can you send me the configuration, qemu command line, and commands
> executed ? I'd like to add that to my test setup if possible.
> 
> Thanks,
> Guenter

Hi Guenter,

Great to get some regular tests running on this.

I haven't minimized this that much, so probably don't need most of this
setup.

qemu-system-x86_64 -M q35,cxl=on,sata=off,smbus=off
 -m 12g -cpu EPYC-Genoa -smp 2,maxcpus=4,sockets=2 \
 -kernel bzImage \
 -drive if=none,file=/mnt/d/images-x86/full2.qcow2,format=qcow2,id=hd \
 -device ioh3420,id=root_port1 \
 -device nvme,serial=deadbeef,drive=hd \
 -nographic -no-reboot -append 'earlycon console=ttyS0 root=/dev/nvme0n1p3 fsck.mode=skip tp_printk maxcpus=4' \
 -monitor telnet:127.0.0.1:1235,server,nowait \
 -object memory-backend-ram,size=12G,id=mem0 \
 -numa node,nodeid=0,memdev=mem0 \
 -numa node,nodeid=1 \
 -numa cpu,node-id=0,socket-id=0 \
 -numa cpu,node-id=0,socket-id=0 \
 -numa cpu,node-id=1,socket-id=1 \
 -numa cpu,node-id=1,socket-id=1

telnet 1235
(qemu) device_add EPYC-Genoa-x86_64-cpu,node-id=1,socket-id=1,core-id=0,thread-id=0 

Jonathan