Re: [PATCH v3] powerpc: Adjust config HOTPLUG_CPU dependency
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2023-11-24 02:40:52
Subsystem:
linux for powerpc (32-bit and 64-bit), the rest · Maintainers:
Madhavan Srinivasan, Michael Ellerman, Linus Torvalds
Hi Vishal, I think our wires got crossed here somewhere :) Vishal Chourasia [off-list ref] writes:
quoted hunk ↗ jump to hunk
Changed HOTPLUG_CPU dependency to SMP and either ARCH_HIBERNATION_POSSIBLE or ARCH_SUSPEND_POSSIBLE, aligning with systems' suspend/hibernation capabilities. This update link CPU hotplugging more logically with platforms' capabilities. configs ARCH_HIBERNATION_POSSIBLE and ARCH_SUSPEND_POSSIBLE are now selected only if required platform dependencies are met. Reported-by: Srikar Dronamraju <redacted> Suggested-by: Aneesh Kumar K V <redacted> Suggested-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Vishal Chourasia <redacted> v2: https://lore.kernel.org/all/20231122092303.223719-1-vishalc@linux.ibm.com (local) v1: https://lore.kernel.org/all/20231114082046.6018-1-vishalc@linux.ibm.com (local) --- During the configuration process with 'make randconfig' followed by 'make olddefconfig', we observed a warning indicating an unmet direct dependency for the HOTPLUG_CPU option. The dependency in question relates to various PowerPC configurations (PPC_PSERIES, PPC_PMAC, PPC_POWERNV, FSL_SOC_BOOKE) which were not enabled, yet the HOTPLUG_CPU was being erroneously selected due to an implicit assumption by the PM_SLEEP_SMP option. This misalignment in dependencies could potentially lead to inconsistent kernel configuration states, especially when considering the necessary hardware support for CPU hot-plugging on PowerPC platforms. The patch aims to correct this by ensuring that ARCH_HIBERNATION_POSSIBLE is contingent upon the appropriate PowerPC configurations being active. steps to reproduce (before applying the patch): Run 'make pseries_le_defconfig' Run 'make menuconfig' Enable hibernation [ Kernel options -> Hibernation (aka 'suspend to disk') ] Disable [ Platform support -> IBM PowerNV (Non-Virtualized) platform support ] Disable [ Platform support -> IBM pSeries & new (POWER5-based) iSeries ] Enable SMP [ Processor support -> Symmetric multi-processing support ] Save the config Run 'make olddefconfig' arch/powerpc/Kconfig | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 6f105ee4f3cf..87c8134da3da 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig@@ -166,6 +167,7 @@ config PPC select ARCH_STACKWALK select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_SUPPORTS_DEBUG_PAGEALLOC if PPC_BOOK3S || PPC_8xx || 40x + select ARCH_SUSPEND_POSSIBLE if (ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES || 44x || 40x)
I know Aneesh suggested moving symbols to under PPC, but I think this is too big and complicated to be under PPC.
quoted hunk ↗ jump to hunk
@@ -381,13 +383,9 @@ config DEFAULT_UIMAGE config ARCH_HIBERNATION_POSSIBLE bool - default y config ARCH_SUSPEND_POSSIBLE - def_bool y - depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \ - (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \ - || 44x || 40x + bool config ARCH_SUSPEND_NONZERO_CPU def_bool y@@ -568,8 +566,7 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY config HOTPLUG_CPU bool "Support for enabling/disabling CPUs" - depends on SMP && (PPC_PSERIES || \ - PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) + depends on SMP && (ARCH_HIBERNATION_POSSIBLE || ARCH_SUSPEND_POSSIBLE)
It's good to fix these warnings, but IMHO the result is that the dependencies are now backward. HOTPLUG_CPU should retain its original dependency list. It's easier to reason directly about "what platforms support CPU hotplug?", oh it's pseries/powernv/powermac/85xx, because they implement cpu_disable(). If there's some dependency from suspend/hibernate on CPU hotplug, then those symbols (suspend/hibernate) should depend on something to do with CPU hotplug. Can you try the patch below? Though, going back to your original reproduction case, that kernel is configured for Book3S 64, but with no platforms enabled, which is a non-sensical configuration (it can't boot on any actual machines). So possibly the real root cause is that, and we should find some way to block creating a config that has no platforms enabled. cheers
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6f105ee4f3cf..9fe656a17017 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig@@ -380,11 +380,12 @@ config DEFAULT_UIMAGE Used to allow a board to specify it wants a uImage built by default config ARCH_HIBERNATION_POSSIBLE - bool - default y + def_bool y + depends on !SMP || HAVE_HOTPLUG_CPU config ARCH_SUSPEND_POSSIBLE def_bool y + depends on !SMP || HAVE_HOTPLUG_CPU depends on ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || \ (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES \ || 44x || 40x
@@ -566,10 +567,14 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mlittle-endian) if PPC64 && CPU_LITTLE_ENDIAN def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mbig-endian) if PPC64 && CPU_BIG_ENDIAN +config HAVE_HOTPLUG_CPU + def_bool y + depends on SMP + depends on PPC_PSERIES || PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE + config HOTPLUG_CPU bool "Support for enabling/disabling CPUs" - depends on SMP && (PPC_PSERIES || \ - PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) + depends on HAVE_HOTPLUG_CPU help Say Y here to be able to disable and re-enable individual CPUs at runtime on SMP machines.