Message ID | 20230913163823.7880-3-james.morse@arm.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9ecd:0:b0:3f2:4152:657d with SMTP id t13csp1213338vqx; Wed, 13 Sep 2023 09:40:26 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGrB7nC6YyuOlg7CoorIDHV02wASFVoF4X3QGmOU+0xGxWtuhy0itSfM8mW4NC9fPUGPIJ3 X-Received: by 2002:a05:6808:3085:b0:3ab:73a6:1469 with SMTP id bl5-20020a056808308500b003ab73a61469mr4138736oib.14.1694623226065; Wed, 13 Sep 2023 09:40:26 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694623226; cv=none; d=google.com; s=arc-20160816; b=sbUePj81tXjbUXEg/hkNM8BTHldZn8B/oudSq3DiWskylgmBixJoX8KZbGWn3rSf0a KP2UW+fE3puZKsTVTQUdOGiWwfAYA3kT9j1Q/SU94kw9vU1WZNgdOiRkqeSauIWvyFu4 4fPXyB2KRwFtTmNKO6PF3lc0HzCox1K7MxQXdg0Q7YboVVA2Cs186YWUhXUu2V5lLEem nIYYm+S37MtmMRk2UoLqUHIE2DaCzN/B355aqgsp4dHS+pCOyNpLWSgOiQvqeeyMC60Z cUHjgAmAjt5tQ/SRaWHmBXz3Sq9KhlaqRkUeZIlNRTpAE0bZLhWM8Evh5dy0WkyVQXEn XSjA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=Mg7CAxI524LEV8wuMKKj25nOZl8IAzubfL5W7QikByk=; fh=2nhVWB0fZ+8YH0lLFpnF/Mo8eq+n1VMLe+b+IpvS0g0=; b=WUptvHrVK88em3aAurS1bQ1Wod6vbhJaMJiqPeoXUWd4SZfgp0tbtNejYD8mgd/rsh O45QOamo8pf/BP5d+fh89lEaJNL8ZJ5tyPj7PXyVawxA3QCG5Ee/m7wtcZV9VKJiIquQ M4zdhrBjyW9iqF+betu5/FGABHq79VTiK7Sevj+RFD+xnCHCDQ2bbX/ErcW09dUO6znA y6KkqK6Ij7RM4l3LpSgEraR2QW0cZZtbfkjDiC8SIiAZQtlm5J1nbNrPp8v/RxFTBlSz 4+80isWjUI+OkHZpc4Er3x/6Q/dQVgSCwQxjzZCTpRZjRbnjmOFVftJrJyWyIPvPN2sB OLbg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from agentk.vger.email (agentk.vger.email. [23.128.96.32]) by mx.google.com with ESMTPS id m1-20020a656a01000000b00573fdd098a7si11043850pgu.471.2023.09.13.09.40.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 09:40:26 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id 830C580BD393; Wed, 13 Sep 2023 09:39:09 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230335AbjIMQjD (ORCPT <rfc822;pwkd43@gmail.com> + 35 others); Wed, 13 Sep 2023 12:39:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57986 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230223AbjIMQjB (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 13 Sep 2023 12:39:01 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 4C20519B1; Wed, 13 Sep 2023 09:38:57 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4588CC15; Wed, 13 Sep 2023 09:39:34 -0700 (PDT) Received: from merodach.members.linode.com (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4A6263F5A1; Wed, 13 Sep 2023 09:38:55 -0700 (PDT) From: James Morse <james.morse@arm.com> To: linux-pm@vger.kernel.org, loongarch@lists.linux.dev, linux-acpi@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-riscv@lists.infradead.org, kvmarm@lists.linux.dev Cc: x86@kernel.org, Salil Mehta <salil.mehta@huawei.com>, Russell King <linux@armlinux.org.uk>, Jean-Philippe Brucker <jean-philippe@linaro.org>, jianyong.wu@arm.com, justin.he@arm.com Subject: [RFC PATCH v2 02/35] drivers: base: Use present CPUs in GENERIC_CPU_DEVICES Date: Wed, 13 Sep 2023 16:37:50 +0000 Message-Id: <20230913163823.7880-3-james.morse@arm.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20230913163823.7880-1-james.morse@arm.com> References: <20230913163823.7880-1-james.morse@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Wed, 13 Sep 2023 09:39:09 -0700 (PDT) X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1776941243973738186 X-GMAIL-MSGID: 1776941243973738186 |
Series |
ACPI/arm64: add support for virtual cpuhotplug
|
|
Commit Message
James Morse
Sept. 13, 2023, 4:37 p.m. UTC
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()
* parisc: smp_prepare_boot_cpu() marks the boot cpu as present,
processor_probe() sets possible for all CPUs and present for all CPUs
except the boot cpu.
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>
---
drivers/base/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Wed, Sep 13, 2023 at 04:37:50PM +0000, James Morse wrote: > 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() > * parisc: smp_prepare_boot_cpu() marks the boot cpu as present, > processor_probe() sets possible for all CPUs and present for all CPUs > except the boot cpu. However, init/main.c::start_kernel() calls boot_cpu_init() which sets the boot CPU in the online, active, present and possible masks. So, _every_ architecture gets the boot CPU in all these masks no matter what. Only of something then clears the boot CPU from these masks (which would be silly) would the boot CPU not be in all of these masks.
On Thu, 14 Sep 2023 09:20:54 +0100 "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > On Wed, Sep 13, 2023 at 04:37:50PM +0000, James Morse wrote: > > 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() > > * parisc: smp_prepare_boot_cpu() marks the boot cpu as present, > > processor_probe() sets possible for all CPUs and present for all CPUs > > except the boot cpu. > > However, init/main.c::start_kernel() calls boot_cpu_init() which sets > the boot CPU in the online, active, present and possible masks. So, > _every_ architecture gets the boot CPU in all these masks no matter > what. > > Only of something then clears the boot CPU from these masks (which > would be silly) would the boot CPU not be in all of these masks. Hi Russel, Upshot is that the code in parisc smp_prepare_boot_cpu() can be dropped? Seems like another useful simplification to add to front of this series. The function will end up with just a print then. Seems there are lots of other empty implementations of smp_prepare_boot_cpu() maybe worth making that optional whilst here and dropping all the empty ones? There seem to be some other architectures setting at least some of the cpu masks that could perhaps be tidied up a little via same logic? Jonathan >
> Signed-off-by: James Morse <james.morse@arm.com> Seems sensible and well reasoned cleanup to me. Technically an ABI change, but would be seriously odd if any real code relied on the current pointless behavior on the few architectures where is changing. FWIW review is really of your analysis rather than the change :) Seems like there may be some additional cleanup that makes sense from Russell's analysis, but that's perhaps a parallel job. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > 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, Sep 14, 2023 at 11:56:13AM +0100, Jonathan Cameron wrote: > On Thu, 14 Sep 2023 09:20:54 +0100 > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote: > > > On Wed, Sep 13, 2023 at 04:37:50PM +0000, James Morse wrote: > > > 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() > > > * parisc: smp_prepare_boot_cpu() marks the boot cpu as present, > > > processor_probe() sets possible for all CPUs and present for all CPUs > > > except the boot cpu. > > > > However, init/main.c::start_kernel() calls boot_cpu_init() which sets > > the boot CPU in the online, active, present and possible masks. So, > > _every_ architecture gets the boot CPU in all these masks no matter > > what. > > > > Only of something then clears the boot CPU from these masks (which > > would be silly) would the boot CPU not be in all of these masks. > Hi Russel, > > Upshot is that the code in parisc smp_prepare_boot_cpu() can be dropped? > Seems like another useful simplification to add to front of this series. Yes - but I personally (and probably others) would like to see progress made towards getting at least some of the changes in this series merged, rather than seeing this series hang around longer and grow. Nothing in this series touches any architecture's smp_prepare_boot_cpu(), so such a change would not interfere with this series. Therefore, I suggest that removing those two set_cpu_*() calls in smp_prepare_boot_cpu() is something that could happen irrespective of anything in this series, and I would encourage PA-RISC folk to do that anway. The same is true of Loongarch, mips, sh, and sparc32, and they can independently sort this. > Seems there are lots of other empty implementations of smp_prepare_boot_cpu() > maybe worth making that optional whilst here and dropping all the empty ones? Yes, and again, this could be a series separate from this one. If one arch wants to add the empty weak version of smp_prepare_boot_cpu(), then it would be a matter of others deleting their empty implementation (possibly after first having cleaned up the unnecessary set_cpu_*() calls). In any case, I would expect that patches doing any of the above would end up being cherry-picked from a series by arch maintainers, so at least to me it makes zero sense to include it with this already large series, and would make the management of this series more complex.
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"); }