From: James Morse <james.morse@arm.com>
Three of the five ACPI architectures create sysfs entries using
register_cpu() for present CPUs, whereas arm64, riscv and all
GENERIC_CPU_DEVICES do this for possible CPUs.
Registering a CPU is what causes them to show up in sysfs.
It makes very little sense to register all possible CPUs. Registering
a CPU is what triggers the udev notifications allowing user-space to
react to newly added CPUs.
To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change
it to use for_each_present_cpu(). Making the ACPI architectures use
GENERIC_CPU_DEVICES is a pre-requisite step to centralise their
cpu_register() logic, before moving it into the ACPI processor driver.
When ACPI is disabled this work would be done by
cpu_dev_register_generic().
Of the ACPI architectures that register possible CPUs, arm64 and riscv
do not support making possible CPUs present as they use the weak 'always
fails' version of arch_register_cpu().
Only two of the eight architectures that use GENERIC_CPU_DEVICES have a
distinction between present and possible CPUs.
The following architectures use GENERIC_CPU_DEVICES but are not SMP,
so possible == present:
* m68k
* microblaze
* nios2
The following architectures use GENERIC_CPU_DEVICES and consider
possible == present:
* csky: setup_smp()
* processor_probe() sets possible for all CPUs and present for all CPUs
except the boot cpu, which will have been done by
init/main.c::start_kernel().
um appears to be a subarchitecture of x86.
The remaining architecture using GENERIC_CPU_DEVICES are:
* openrisc and hexagon:
where smp_init_cpus() makes all CPUs < NR_CPUS possible,
whereas smp_prepare_cpus() only makes CPUs < setup_max_cpus present.
After this change, openrisc and hexagon systems that use the max_cpus
command line argument would not see the other CPUs present in sysfs.
This should not be a problem as these CPUs can't bre brought online as
_cpu_up() checks cpu_present().
After this change, only CPUs which are present appear in sysfs.
Signed-off-by: James Morse <james.morse@arm.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/base/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 9ea22e165acd..34b48f660b6b 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -533,7 +533,7 @@ static void __init cpu_dev_register_generic(void)
#ifdef CONFIG_GENERIC_CPU_DEVICES
int i;
- for_each_possible_cpu(i) {
+ for_each_present_cpu(i) {
if (register_cpu(&per_cpu(cpu_devices, i), i))
panic("Failed to register CPU device");
}
--
2.30.2On 11/7/23 20:29, Russell King (Oracle) wrote: > From: James Morse <james.morse@arm.com> > > Three of the five ACPI architectures create sysfs entries using > register_cpu() for present CPUs, whereas arm64, riscv and all > GENERIC_CPU_DEVICES do this for possible CPUs. > > Registering a CPU is what causes them to show up in sysfs. > > It makes very little sense to register all possible CPUs. Registering > a CPU is what triggers the udev notifications allowing user-space to > react to newly added CPUs. > > To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change > it to use for_each_present_cpu(). Making the ACPI architectures use > GENERIC_CPU_DEVICES is a pre-requisite step to centralise their > cpu_register() logic, before moving it into the ACPI processor driver. > When ACPI is disabled this work would be done by > cpu_dev_register_generic(). > > Of the ACPI architectures that register possible CPUs, arm64 and riscv > do not support making possible CPUs present as they use the weak 'always > fails' version of arch_register_cpu(). > > Only two of the eight architectures that use GENERIC_CPU_DEVICES have a > distinction between present and possible CPUs. > > The following architectures use GENERIC_CPU_DEVICES but are not SMP, > so possible == present: > * m68k > * microblaze > * nios2 > > The following architectures use GENERIC_CPU_DEVICES and consider > possible == present: > * csky: setup_smp() > * processor_probe() sets possible for all CPUs and present for all CPUs > except the boot cpu, which will have been done by > init/main.c::start_kernel(). > > um appears to be a subarchitecture of x86. > > The remaining architecture using GENERIC_CPU_DEVICES are: > * openrisc and hexagon: > where smp_init_cpus() makes all CPUs < NR_CPUS possible, > whereas smp_prepare_cpus() only makes CPUs < setup_max_cpus present. > > After this change, openrisc and hexagon systems that use the max_cpus > command line argument would not see the other CPUs present in sysfs. > This should not be a problem as these CPUs can't bre brought online as > _cpu_up() checks cpu_present(). > > After this change, only CPUs which are present appear in sysfs. > > Signed-off-by: James Morse <james.morse@arm.com> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > drivers/base/cpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Gavin Shan <gshan@redhat.com>
Hi Russell,
On 11/7/23 18:29, Russell King (Oracle) wrote:
> From: James Morse <james.morse@arm.com>
>
> Three of the five ACPI architectures create sysfs entries using
> register_cpu() for present CPUs, whereas arm64, riscv and all
> GENERIC_CPU_DEVICES do this for possible CPUs.
>
> Registering a CPU is what causes them to show up in sysfs.
>
> It makes very little sense to register all possible CPUs. Registering
> a CPU is what triggers the udev notifications allowing user-space to
> react to newly added CPUs.
>
> To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change
> it to use for_each_present_cpu(). Making the ACPI architectures use
> GENERIC_CPU_DEVICES is a pre-requisite step to centralise their
> cpu_register() logic, before moving it into the ACPI processor driver.
> When ACPI is disabled this work would be done by
> cpu_dev_register_generic().
What do you actually mean about when ACPI is disabled this work would be
done by cpu_dev_register_generic()? Is the work means register the cpu?
I'm not quite understand that, and how about when ACPI is enabled, which
function do this work?
>
> Of the ACPI architectures that register possible CPUs, arm64 and riscv
> do not support making possible CPUs present as they use the weak 'always
> fails' version of arch_register_cpu().
>
> Only two of the eight architectures that use GENERIC_CPU_DEVICES have a
> distinction between present and possible CPUs.
>
> The following architectures use GENERIC_CPU_DEVICES but are not SMP,
> so possible == present:
> * m68k
> * microblaze
> * nios2
>
> The following architectures use GENERIC_CPU_DEVICES and consider
> possible == present:
> * csky: setup_smp()
> * processor_probe() sets possible for all CPUs and present for all CPUs
> except the boot cpu, which will have been done by
> init/main.c::start_kernel().
>
> um appears to be a subarchitecture of x86.
>
> The remaining architecture using GENERIC_CPU_DEVICES are:
> * openrisc and hexagon:
> where smp_init_cpus() makes all CPUs < NR_CPUS possible,
> whereas smp_prepare_cpus() only makes CPUs < setup_max_cpus present.
>
> After this change, openrisc and hexagon systems that use the max_cpus
> command line argument would not see the other CPUs present in sysfs.
> This should not be a problem as these CPUs can't bre brought online as
^ nit: can't be
> _cpu_up() checks cpu_present().
>
> After this change, only CPUs which are present appear in sysfs.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Thanks,
Shaoqin
> ---
> drivers/base/cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 9ea22e165acd..34b48f660b6b 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -533,7 +533,7 @@ static void __init cpu_dev_register_generic(void)
> #ifdef CONFIG_GENERIC_CPU_DEVICES
> int i;
>
> - for_each_possible_cpu(i) {
> + for_each_present_cpu(i) {
> if (register_cpu(&per_cpu(cpu_devices, i), i))
> panic("Failed to register CPU device");
> }
On Thu, Nov 09, 2023 at 06:09:32PM +0800, Shaoqin Huang wrote: > Hi Russell, > > On 11/7/23 18:29, Russell King (Oracle) wrote: > > From: James Morse <james.morse@arm.com> > > > > Three of the five ACPI architectures create sysfs entries using > > register_cpu() for present CPUs, whereas arm64, riscv and all > > GENERIC_CPU_DEVICES do this for possible CPUs. > > > > Registering a CPU is what causes them to show up in sysfs. > > > > It makes very little sense to register all possible CPUs. Registering > > a CPU is what triggers the udev notifications allowing user-space to > > react to newly added CPUs. > > > > To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change > > it to use for_each_present_cpu(). Making the ACPI architectures use > > GENERIC_CPU_DEVICES is a pre-requisite step to centralise their > > cpu_register() logic, before moving it into the ACPI processor driver. > > When ACPI is disabled this work would be done by > > cpu_dev_register_generic(). > > What do you actually mean about when ACPI is disabled this work would be Firstly, please note that "you" is not appropriate here. This is James' commit message, not mine. > done by cpu_dev_register_generic()? Is the work means register the cpu? When ACPI is disabled _and_ CONFIG_GENERIC_CPU_DEVICES is enabled, then cpu_dev_register_generic() will call arch_register_cpu() for each present CPU after this commit, rather than for each _possible_ CPU (which is the actual code change here.) > I'm not quite understand that, and how about when ACPI is enabled, which > function do this work? This is what happens later in the series. "drivers: base: Allow parts of GENERIC_CPU_DEVICES to be overridden" adds a test for CONFIG_GENERIC_CPU_DEVICES, so this will only be used with architectures using GENERIC_CPU_DEVICES. Then in: "ACPI: processor: Register all CPUs from acpi_processor_get_info()" which is not part of this series, this adds a call to arch_register_cpu() in the ACPI code, and disables this path via a test for !acpi_disabled. Essentially, this path gets used to register the present CPUs when firmware (ACPI) isn't going to be registering the present CPUs. I've changed this to: "It makes very little sense to register all possible CPUs. Registering a CPU is what triggers the udev notifications allowing user-space to react to newly added CPUs. "To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change it to use for_each_present_cpu(). "Making the ACPI architectures use GENERIC_CPU_DEVICES is a pre-requisite step to centralise their register_cpu() logic, before moving it into the ACPI processor driver. When we add support for register CPUs from ACPI in a later patch, we will avoid registering CPUs in this path." which I hope makes it clearer. > > After this change, openrisc and hexagon systems that use the max_cpus > > command line argument would not see the other CPUs present in sysfs. > > This should not be a problem as these CPUs can't bre brought online as > ^ nit: can't be Thanks, I'll fix that. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On 11/9/23 18:29, Russell King (Oracle) wrote: > On Thu, Nov 09, 2023 at 06:09:32PM +0800, Shaoqin Huang wrote: >> Hi Russell, >> >> On 11/7/23 18:29, Russell King (Oracle) wrote: >>> From: James Morse <james.morse@arm.com> >>> >>> Three of the five ACPI architectures create sysfs entries using >>> register_cpu() for present CPUs, whereas arm64, riscv and all >>> GENERIC_CPU_DEVICES do this for possible CPUs. >>> >>> Registering a CPU is what causes them to show up in sysfs. >>> >>> It makes very little sense to register all possible CPUs. Registering >>> a CPU is what triggers the udev notifications allowing user-space to >>> react to newly added CPUs. >>> >>> To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change >>> it to use for_each_present_cpu(). Making the ACPI architectures use >>> GENERIC_CPU_DEVICES is a pre-requisite step to centralise their >>> cpu_register() logic, before moving it into the ACPI processor driver. >>> When ACPI is disabled this work would be done by >>> cpu_dev_register_generic(). >> >> What do you actually mean about when ACPI is disabled this work would be > > Firstly, please note that "you" is not appropriate here. This is James' > commit message, not mine. > Oh, Sorry for that. >> done by cpu_dev_register_generic()? Is the work means register the cpu? > > When ACPI is disabled _and_ CONFIG_GENERIC_CPU_DEVICES is enabled, then > cpu_dev_register_generic() will call arch_register_cpu() for each present > CPU after this commit, rather than for each _possible_ CPU (which is the > actual code change here.) > >> I'm not quite understand that, and how about when ACPI is enabled, which >> function do this work? > > This is what happens later in the series. > > "drivers: base: Allow parts of GENERIC_CPU_DEVICES to be overridden" > adds a test for CONFIG_GENERIC_CPU_DEVICES, so this will only be used > with architectures using GENERIC_CPU_DEVICES. Then in: > > "ACPI: processor: Register all CPUs from acpi_processor_get_info()" > which is not part of this series, this adds a call to arch_register_cpu() > in the ACPI code, and disables this path via a test for !acpi_disabled. > > Essentially, this path gets used to register the present CPUs when > firmware (ACPI) isn't going to be registering the present CPUs. > > I've changed this to: > > "It makes very little sense to register all possible CPUs. Registering > a CPU is what triggers the udev notifications allowing user-space to > react to newly added CPUs. > > "To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change > it to use for_each_present_cpu(). > > "Making the ACPI architectures use GENERIC_CPU_DEVICES is a pre-requisite > step to centralise their register_cpu() logic, before moving it into the > ACPI processor driver. When we add support for register CPUs from ACPI > in a later patch, we will avoid registering CPUs in this path." > > which I hope makes it clearer. > Thanks for your great explanation. Change commit message to this makes me understand well. Thanks, Shaoqin >>> After this change, openrisc and hexagon systems that use the max_cpus >>> command line argument would not see the other CPUs present in sysfs. >>> This should not be a problem as these CPUs can't bre brought online as >> ^ nit: can't be > > Thanks, I'll fix that. >
© 2016 - 2025 Red Hat, Inc.