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@xxxxxxx> > > > > 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!
