2024-02-05 10:01:44

by Alexey Dobriyan

[permalink] [raw]
Subject: [PATCH] cpu: mark cpu_possible_mask as __ro_after_init

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 <[email protected]>
---

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);



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

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 <[email protected]>
AuthorDate: Mon, 05 Feb 2024 13:01:19 +03:00
Committer: Thomas Gleixner <[email protected]>
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 <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
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);


2024-02-22 06:00:41

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] cpu: mark cpu_possible_mask as __ro_after_init

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 <[email protected]>
> ---
>
> 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);
>

2024-02-22 10:36:31

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH] cpu: mark cpu_possible_mask as __ro_after_init

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 <[email protected]>
> > ---
> >
> > 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;

2024-02-22 11:19:55

by Alexey Dobriyan

[permalink] [raw]
Subject: [PATCH v2] 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 hardware
configuration.

Signed-off-by: Alexey Dobriyan <[email protected]>
---

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);


2024-03-27 18:36:40

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v2] cpu: mark cpu_possible_mask as __ro_after_init

On Wed, Mar 27, 2024 at 06:10:53PM +0000, Jonathan Cameron wrote:
> On Thu, 22 Feb 2024 14:19:35 +0300
> Alexey Dobriyan <[email protected]> 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 <[email protected]>
>
> 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

2024-03-27 18:15:10

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2] cpu: mark cpu_possible_mask as __ro_after_init

On Thu, 22 Feb 2024 14:19:35 +0300
Alexey Dobriyan <[email protected]> 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 <[email protected]>

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);
>


2024-04-03 13:44:29

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2] cpu: mark cpu_possible_mask as __ro_after_init

On Wed, 27 Mar 2024 11:36:28 -0700
Guenter Roeck <[email protected]> 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 <[email protected]> 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 <[email protected]>
> >
> > 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