Re: [PATCH v3] powerpc: Adjust config HOTPLUG_CPU dependency
On 24/11/23 8:09 am, Michael Ellerman wrote: > Hi Vishal, > > I think our wires got crossed here somewhere :) > > Vishal Chourasia writes: >> 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 >> Suggested-by: Aneesh Kumar K V >> Suggested-by: Michael Ellerman >> Signed-off-by: Vishal Chourasia >> >> v2: https://lore.kernel.org/all/20231122092303.223719-1-vish...@linux.ibm.com >> v1: https://lore.kernel.org/all/20231114082046.6018-1-vish...@linux.ibm.com >> --- >> 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_PAGEALLOCif PPC_BOOK3S || PPC_8xx || 40x >> +select ARCH_SUSPEND_POSSIBLEif (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. > >> @@ -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 > poss
[PATCH v3] powerpc: Adjust config HOTPLUG_CPU dependency
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 Suggested-by: Aneesh Kumar K V Suggested-by: Michael Ellerman Signed-off-by: Vishal Chourasia v2: https://lore.kernel.org/all/20231122092303.223719-1-vish...@linux.ibm.com v1: https://lore.kernel.org/all/20231114082046.6018-1-vish...@linux.ibm.com --- 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 @@ -156,6 +156,7 @@ config PPC select ARCH_HAS_UACCESS_FLUSHCACHE select ARCH_HAS_UBSAN_SANITIZE_ALL select ARCH_HAVE_NMI_SAFE_CMPXCHG + select ARCH_HIBERNATION_POSSIBLEif (PPC_PSERIES || PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) select ARCH_KEEP_MEMBLOCK select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE if PPC_RADIX_MMU select ARCH_MIGHT_HAVE_PC_PARPORT @@ -166,6 +167,7 @@ config PPC select ARCH_STACKWALK select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_SUPPORTS_DEBUG_PAGEALLOCif PPC_BOOK3S || PPC_8xx || 40x + select ARCH_SUSPEND_POSSIBLEif (ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES || 44x || 40x) select ARCH_USE_BUILTIN_BSWAP select ARCH_USE_CMPXCHG_LOCKREF if PPC64 select ARCH_USE_MEMTEST @@ -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) help Say Y here to be able to disable and re-enable individual CPUs at runtime on SMP machines. -- 2.41.0
Re: [PATCH] powerpc: Adjust config HOTPLUG_CPU dependency
On 22/11/23 3:03 pm, Aneesh Kumar K V wrote: > On 11/22/23 2:53 PM, Vishal Chourasia wrote: >> Changed HOTPLUG_CPU dependency to SMP and either ARCH_HIBERNATION_POSSIBLE or >> ARCH_SUSPEND_POSSIBLE, aligning with systems' suspend/hibernation >> capabilities. >> This update links CPU hotplugging more logically with platforms' >> capabilities. >> >> Other changes include >> >> 1. configs ARCH_HIBERNATION_POSSIBLE and ARCH_SUSPEND_POSSIBLE are now >> selected >>only if required platform dependencies are met. >> >> 2. Defaults for configs ARCH_HIBERNATION_POSSIBLE and >>ARCH_SUSPEND_POSSIBLE has been set to 'N' >> >> Reported-by: Srikar Dronamraju >> Suggested-by: Aneesh Kumar K V >> Suggested-by: Michael Ellerman >> Signed-off-by: Vishal Chourasia >> >> v1: https://lore.kernel.org/all/20231114082046.6018-1-vish...@linux.ibm.com >> --- >> 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 | 13 + >> 1 file changed, 5 insertions(+), 8 deletions(-) >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index 6f105ee4f3cf..6e7e9af2f0c1 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -155,6 +155,8 @@ config PPC >> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST >> select ARCH_HAS_UACCESS_FLUSHCACHE >> select ARCH_HAS_UBSAN_SANITIZE_ALL >> +select ARCH_HIBERNATION_POSSIBLEif (PPC_PSERIES || PPC_PMAC || >> PPC_POWERNV || FSL_SOC_BOOKE) >> +select ARCH_SUSPEND_POSSIBLEif (ADB_PMU || PPC_EFIKA || >> PPC_LITE5200 || PPC_83xx || (PPC_85xx && !PPC_E500MC) || PPC_86xx || >> PPC_PSERIES || 44x || 40x) > We should keep that sorted as explained in comment around that. Sure. I will send out another patch > >> select ARCH_HAVE_NMI_SAFE_CMPXCHG >> select ARCH_KEEP_MEMBLOCK >> select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE if PPC_RADIX_MMU >> @@ -380,14 +382,10 @@ 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 n > > We should be able to keep this > config ARCH_HIBERNATION_POSSIBLE > bool > > > That is how we have rest of the ARCH_* config. I am not sure we need to > explicitly say "def_bool n" I believed it improves readability, that all. I can revert it back. > >> >> 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 >> +def_bool n >> >> 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) >> help >>Say Y here to be able to disable and re-enable individual >>CPUs at runtime on SMP machines.
[PATCH] powerpc: Adjust config HOTPLUG_CPU dependency
Changed HOTPLUG_CPU dependency to SMP and either ARCH_HIBERNATION_POSSIBLE or ARCH_SUSPEND_POSSIBLE, aligning with systems' suspend/hibernation capabilities. This update links CPU hotplugging more logically with platforms' capabilities. Other changes include 1. configs ARCH_HIBERNATION_POSSIBLE and ARCH_SUSPEND_POSSIBLE are now selected only if required platform dependencies are met. 2. Defaults for configs ARCH_HIBERNATION_POSSIBLE and ARCH_SUSPEND_POSSIBLE has been set to 'N' Reported-by: Srikar Dronamraju Suggested-by: Aneesh Kumar K V Suggested-by: Michael Ellerman Signed-off-by: Vishal Chourasia v1: https://lore.kernel.org/all/20231114082046.6018-1-vish...@linux.ibm.com --- 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 | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 6f105ee4f3cf..6e7e9af2f0c1 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -155,6 +155,8 @@ config PPC select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAS_UACCESS_FLUSHCACHE select ARCH_HAS_UBSAN_SANITIZE_ALL + select ARCH_HIBERNATION_POSSIBLEif (PPC_PSERIES || PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE) + select ARCH_SUSPEND_POSSIBLEif (ADB_PMU || PPC_EFIKA || PPC_LITE5200 || PPC_83xx || (PPC_85xx && !PPC_E500MC) || PPC_86xx || PPC_PSERIES || 44x || 40x) select ARCH_HAVE_NMI_SAFE_CMPXCHG select ARCH_KEEP_MEMBLOCK select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE if PPC_RADIX_MMU @@ -380,14 +382,10 @@ 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 n 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 + def_bool n 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) help Say Y here to be able to disable and re-enable individual CPUs at runtime on SMP machines. -- 2.41.0
Re: [PATCH] powerpc: Restrict ARCH_HIBERNATION_POSSIBLE to supported configurations
On 17/11/23 4:52 am, Michael Ellerman wrote: > Vishal Chourasia writes: >> On 15/11/23 5:46 pm, Aneesh Kumar K V wrote: >>> On 11/15/23 5:23 PM, Vishal Chourasia wrote: >>>> On 15/11/23 1:39 pm, Aneesh Kumar K.V wrote: >>>>> Vishal Chourasia writes: >>>>> >>>>>> This patch modifies the ARCH_HIBERNATION_POSSIBLE option to ensure that >>>>>> it >>>>>> correctly depends on these PowerPC configurations being enabled. As a >>>>>> result, >>>>>> it prevents the HOTPLUG_CPU from being selected when the required >>>>>> dependencies >>>>>> are not satisfied. >>>>>> >>>>>> This change aligns the dependency tree with the expected hardware >>>>>> support for >>>>>> CPU hot-plugging under PowerPC architectures, ensuring that the kernel >>>>>> configuration steps do not lead to inconsistent states. >>>>>> >>>>>> Signed-off-by: Vishal Chourasia >>>>>> --- >>>>>> 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 >>>>>> beingDuring 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' >>>>>> 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 ] &
Re: [PATCH] powerpc: Restrict ARCH_HIBERNATION_POSSIBLE to supported configurations
On 17/11/23 4:52 am, Michael Ellerman wrote: > Vishal Chourasia writes: >> On 15/11/23 5:46 pm, Aneesh Kumar K V wrote: >>> On 11/15/23 5:23 PM, Vishal Chourasia wrote: >>>> On 15/11/23 1:39 pm, Aneesh Kumar K.V wrote: >>>>> Vishal Chourasia writes: >>>>> >>>>>> This patch modifies the ARCH_HIBERNATION_POSSIBLE option to ensure that >>>>>> it >>>>>> correctly depends on these PowerPC configurations being enabled. As a >>>>>> result, >>>>>> it prevents the HOTPLUG_CPU from being selected when the required >>>>>> dependencies >>>>>> are not satisfied. >>>>>> >>>>>> This change aligns the dependency tree with the expected hardware >>>>>> support for >>>>>> CPU hot-plugging under PowerPC architectures, ensuring that the kernel >>>>>> configuration steps do not lead to inconsistent states. >>>>>> >>>>>> Signed-off-by: Vishal Chourasia >>>>>> --- >>>>>> 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 | 5 +++-- >>>>>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >>>>>> index 6f105ee4f3cf..bf99ff9869f6 100644 >>>>>> --- a/arch/powerpc/Kconfig >>>>>> +++ b/arch/powerpc/Kconfig >>>>>> @@ -380,8 +380,9 @@ 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 PPC_PSERIES || \ >>>>>> +PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE >>>>>> >>>>>> config ARCH_SUSPEND_POSSIBLE >>>>>> def_bool y >>>>>> >>>>> I am wondering whether it should be switched to using select from >>>>> config PPC? >>>>> >>>>> selecting ARCH_HIBERNATION_POSSIBLE based on value of config PPC >>>>> will not guarantee config PPC_PSERIES being set >>>>> >>>>> PPC_PSERIES can be set to N, even when config PPC is set. >> I understand what you meant before. Having ARCH_HIBERNATION_POSSIBLE under >> config PPC makes more sense. >>>>> grep -A 5 -i "config ppc_pseries" arch/powerpc/platforms/pseries/Kconfig >>>>> config PPC_PSERIES >>>>> depends on PPC64 &&am
Re: [PATCH] powerpc: Restrict ARCH_HIBERNATION_POSSIBLE to supported configurations
On 15/11/23 5:46 pm, Aneesh Kumar K V wrote: > On 11/15/23 5:23 PM, Vishal Chourasia wrote: >> On 15/11/23 1:39 pm, Aneesh Kumar K.V wrote: >>> Vishal Chourasia writes: >>> >>>> This patch modifies the ARCH_HIBERNATION_POSSIBLE option to ensure that it >>>> correctly depends on these PowerPC configurations being enabled. As a >>>> result, >>>> it prevents the HOTPLUG_CPU from being selected when the required >>>> dependencies >>>> are not satisfied. >>>> >>>> This change aligns the dependency tree with the expected hardware support >>>> for >>>> CPU hot-plugging under PowerPC architectures, ensuring that the kernel >>>> configuration steps do not lead to inconsistent states. >>>> >>>> Signed-off-by: Vishal Chourasia >>>> --- >>>> 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 | 5 +++-- >>>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >>>> index 6f105ee4f3cf..bf99ff9869f6 100644 >>>> --- a/arch/powerpc/Kconfig >>>> +++ b/arch/powerpc/Kconfig >>>> @@ -380,8 +380,9 @@ 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 PPC_PSERIES || \ >>>> + PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE >>>> >>>> config ARCH_SUSPEND_POSSIBLE >>>>def_bool y >>>> >>> I am wondering whether it should be switched to using select from >>> config PPC? >>> >>> selecting ARCH_HIBERNATION_POSSIBLE based on value of config PPC >>> will not guarantee config PPC_PSERIES being set >>> >>> PPC_PSERIES can be set to N, even when config PPC is set. I understand what you meant before. Having ARCH_HIBERNATION_POSSIBLE under config PPC makes more sense. >>> grep -A 5 -i "config ppc_pseries" arch/powerpc/platforms/pseries/Kconfig >>> config PPC_PSERIES >>> depends on PPC64 && PPC_BOOK3S >>> bool "IBM pSeries & new (POWER5-based) iSeries" >>> select HAVE_PCSPKR_PLATFORM >>> select MPIC >>> select OF_DYNAMIC >>> > modified arch/powerpc/Kconfig > @@ -156,6 +156,7 @@ config PPC > select ARCH_HAS_UACCESS_FLUSHCACHE > select ARCH_HAS_UBSAN_SANITIZE_ALL > select ARCH_HAVE_NMI_SAFE_CMPXCHG > + select ARCH_HIBERNATION_POSSIBLEif (PPC_PSERIES || PPC_PMAC || > PPC_POWERNV || FSL_SOC_BOOKE) > select ARCH_KEEP_MEMBLOCK > select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE if PPC_RADIX_MMU > select ARCH_MIGHT_HAVE_PC_PARPORT Though, even with these changes I was able to reproduce same warnings. (using steps from above) It's because one can enable HIBERNATION manually. As these warnings were observed through make randconfig, there is still a chance that randconfig may result in a permutation that may produce these warnings again. > > -aneesh
Re: [PATCH] powerpc: Restrict ARCH_HIBERNATION_POSSIBLE to supported configurations
On 15/11/23 1:39 pm, Aneesh Kumar K.V wrote: > Vishal Chourasia writes: > >> This patch modifies the ARCH_HIBERNATION_POSSIBLE option to ensure that it >> correctly depends on these PowerPC configurations being enabled. As a result, >> it prevents the HOTPLUG_CPU from being selected when the required >> dependencies >> are not satisfied. >> >> This change aligns the dependency tree with the expected hardware support for >> CPU hot-plugging under PowerPC architectures, ensuring that the kernel >> configuration steps do not lead to inconsistent states. >> >> Signed-off-by: Vishal Chourasia >> --- >> 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 | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index 6f105ee4f3cf..bf99ff9869f6 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -380,8 +380,9 @@ 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 PPC_PSERIES || \ >> +PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE >> >> config ARCH_SUSPEND_POSSIBLE >> def_bool y >> > I am wondering whether it should be switched to using select from > config PPC? selecting ARCH_HIBERNATION_POSSIBLE based on value of config PPC will not guarantee config PPC_PSERIES being set PPC_PSERIES can be set to N, even when config PPC is set. grep -A 5 -i "config ppc_pseries" arch/powerpc/platforms/pseries/Kconfig config PPC_PSERIES depends on PPC64 && PPC_BOOK3S bool "IBM pSeries & new (POWER5-based) iSeries" select HAVE_PCSPKR_PLATFORM select MPIC select OF_DYNAMIC > > -aneesh
[PATCH] powerpc: Restrict ARCH_HIBERNATION_POSSIBLE to supported configurations
This patch modifies the ARCH_HIBERNATION_POSSIBLE option to ensure that it correctly depends on these PowerPC configurations being enabled. As a result, it prevents the HOTPLUG_CPU from being selected when the required dependencies are not satisfied. This change aligns the dependency tree with the expected hardware support for CPU hot-plugging under PowerPC architectures, ensuring that the kernel configuration steps do not lead to inconsistent states. Signed-off-by: Vishal Chourasia --- 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 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 6f105ee4f3cf..bf99ff9869f6 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -380,8 +380,9 @@ 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 PPC_PSERIES || \ + PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE config ARCH_SUSPEND_POSSIBLE def_bool y -- 2.41.0
Re: [PATCH] powerpc/crypto: Fix aes-gcm-p10 link errors
On 5/25/23 20:35, Michael Ellerman wrote:
The recently added P10 AES/GCM code added some files containing
CRYPTOGAMS perl-asm code which are near duplicates of the p8 files
found in drivers/crypto/vmx.
In particular the newly added files produce functions with identical
names to the existing code.
When the kernel is built with CONFIG_CRYPTO_AES_GCM_P10=y and
CONFIG_CRYPTO_DEV_VMX_ENCRYPT=y that leads to link errors, eg:
ld: drivers/crypto/vmx/aesp8-ppc.o: in function `aes_p8_set_encrypt_key':
(.text+0xa0): multiple definition of `aes_p8_set_encrypt_key';
arch/powerpc/crypto/aesp8-ppc.o:(.text+0xa0): first defined here
...
ld: drivers/crypto/vmx/ghashp8-ppc.o: in function `gcm_ghash_p8':
(.text+0x140): multiple definition of `gcm_ghash_p8';
arch/powerpc/crypto/ghashp8-ppc.o:(.text+0x2e4): first defined here
Fix it for now by renaming the newly added files and functions to use
"p10" instead of "p8" in the names.
Fixes: 45a4672b9a6e ("crypto: p10-aes-gcm - Update Kconfig and Makefile")
Signed-off-by: Michael Ellerman
---
arch/powerpc/crypto/Makefile | 10 +-
arch/powerpc/crypto/aes-gcm-p10-glue.c | 18 +-
.../crypto/{aesp8-ppc.pl => aesp10-ppc.pl} | 2 +-
.../crypto/{ghashp8-ppc.pl => ghashp10-ppc.pl} | 12 ++--
4 files changed, 21 insertions(+), 21 deletions(-)
rename arch/powerpc/crypto/{aesp8-ppc.pl => aesp10-ppc.pl} (99%)
rename arch/powerpc/crypto/{ghashp8-ppc.pl => ghashp10-ppc.pl} (97%)
I missed adding tested-by in previous reply.
After applying the patch, I was able to successfully build the Linux
kernel v6.4-rc4.
I encountered no errors during the build process. The issue pertaining
to multiple
definitions of certain functions appears to be resolved.
λ grep -i CRYPTO_AES_GCM_P10 .config
CONFIG_CRYPTO_AES_GCM_P10=y
λ grep -i CRYPTO_DEV_VMX_ENCRYPT .config
CONFIG_CRYPTO_DEV_VMX_ENCRYPT=y
+ make O=${BUILD_DIR} CC=clang ARCH=powerpc
CROSS_COMPILE=powerpc64le-linux-gnu- -j16 -s vmlinux modules
Thank you for the well-detailed fix.
Tested-by: Vishal Chourasia
Re: [PATCH] powerpc/crypto: Fix aes-gcm-p10 link errors
After applying the patch, I was able to successfully build the Linux
kernel v6.4-rc4.
I encountered no errors during the build process. The issue pertaining
to multiple
definitions of certain functions appears to be resolved.
λ grep -i CRYPTO_AES_GCM_P10 .config
CONFIG_CRYPTO_AES_GCM_P10=y
λ grep -i CRYPTO_DEV_VMX_ENCRYPT .config
CONFIG_CRYPTO_DEV_VMX_ENCRYPT=y
+ make O=${BUILD_DIR} CC=clang ARCH=powerpc
CROSS_COMPILE=powerpc64le-linux-gnu- -j16 -s vmlinux modules
Thank you for the well-detailed fix.
-- vishal.c
On 5/25/23 20:35, Michael Ellerman wrote:
The recently added P10 AES/GCM code added some files containing
CRYPTOGAMS perl-asm code which are near duplicates of the p8 files
found in drivers/crypto/vmx.
In particular the newly added files produce functions with identical
names to the existing code.
When the kernel is built with CONFIG_CRYPTO_AES_GCM_P10=y and
CONFIG_CRYPTO_DEV_VMX_ENCRYPT=y that leads to link errors, eg:
ld: drivers/crypto/vmx/aesp8-ppc.o: in function `aes_p8_set_encrypt_key':
(.text+0xa0): multiple definition of `aes_p8_set_encrypt_key';
arch/powerpc/crypto/aesp8-ppc.o:(.text+0xa0): first defined here
...
ld: drivers/crypto/vmx/ghashp8-ppc.o: in function `gcm_ghash_p8':
(.text+0x140): multiple definition of `gcm_ghash_p8';
arch/powerpc/crypto/ghashp8-ppc.o:(.text+0x2e4): first defined here
Fix it for now by renaming the newly added files and functions to use
"p10" instead of "p8" in the names.
Fixes: 45a4672b9a6e ("crypto: p10-aes-gcm - Update Kconfig and Makefile")
Signed-off-by: Michael Ellerman
---
arch/powerpc/crypto/Makefile | 10 +-
arch/powerpc/crypto/aes-gcm-p10-glue.c | 18 +-
.../crypto/{aesp8-ppc.pl => aesp10-ppc.pl} | 2 +-
.../crypto/{ghashp8-ppc.pl => ghashp10-ppc.pl} | 12 ++--
4 files changed, 21 insertions(+), 21 deletions(-)
rename arch/powerpc/crypto/{aesp8-ppc.pl => aesp10-ppc.pl} (99%)
rename arch/powerpc/crypto/{ghashp8-ppc.pl => ghashp10-ppc.pl} (97%)
diff --git a/arch/powerpc/crypto/Makefile b/arch/powerpc/crypto/Makefile
index 05c7486f42c5..7b4f516abec1 100644
--- a/arch/powerpc/crypto/Makefile
+++ b/arch/powerpc/crypto/Makefile
@@ -22,15 +22,15 @@ sha1-ppc-spe-y := sha1-spe-asm.o sha1-spe-glue.o
sha256-ppc-spe-y := sha256-spe-asm.o sha256-spe-glue.o
crc32c-vpmsum-y := crc32c-vpmsum_asm.o crc32c-vpmsum_glue.o
crct10dif-vpmsum-y := crct10dif-vpmsum_asm.o crct10dif-vpmsum_glue.o
-aes-gcm-p10-crypto-y := aes-gcm-p10-glue.o aes-gcm-p10.o ghashp8-ppc.o
aesp8-ppc.o
+aes-gcm-p10-crypto-y := aes-gcm-p10-glue.o aes-gcm-p10.o ghashp10-ppc.o
aesp10-ppc.o
quiet_cmd_perl = PERL$@
cmd_perl = $(PERL) $< $(if $(CONFIG_CPU_LITTLE_ENDIAN), linux-ppc64le,
linux-ppc64) > $@
-targets += aesp8-ppc.S ghashp8-ppc.S
+targets += aesp10-ppc.S ghashp10-ppc.S
-$(obj)/aesp8-ppc.S $(obj)/ghashp8-ppc.S: $(obj)/%.S: $(src)/%.pl FORCE
+$(obj)/aesp10-ppc.S $(obj)/ghashp10-ppc.S: $(obj)/%.S: $(src)/%.pl FORCE
$(call if_changed,perl)
-OBJECT_FILES_NON_STANDARD_aesp8-ppc.o := y
-OBJECT_FILES_NON_STANDARD_ghashp8-ppc.o := y
+OBJECT_FILES_NON_STANDARD_aesp10-ppc.o := y
+OBJECT_FILES_NON_STANDARD_ghashp10-ppc.o := y
diff --git a/arch/powerpc/crypto/aes-gcm-p10-glue.c
b/arch/powerpc/crypto/aes-gcm-p10-glue.c
index bd3475f5348d..4b6e899895e7 100644
--- a/arch/powerpc/crypto/aes-gcm-p10-glue.c
+++ b/arch/powerpc/crypto/aes-gcm-p10-glue.c
@@ -30,15 +30,15 @@ MODULE_AUTHOR("Danny Tsen
-asmlinkage int aes_p8_set_encrypt_key(const u8 *userKey, const int bits,
+asmlinkage int aes_p10_set_encrypt_key(const u8 *userKey, const int bits,
void *key);
-asmlinkage void aes_p8_encrypt(const u8 *in, u8 *out, const void *key);
+asmlinkage void aes_p10_encrypt(const u8 *in, u8 *out, const void *key);
asmlinkage void aes_p10_gcm_encrypt(u8 *in, u8 *out, size_t len,
void *rkey, u8 *iv, void *Xi);
asmlinkage void aes_p10_gcm_decrypt(u8 *in, u8 *out, size_t len,
void *rkey, u8 *iv, void *Xi);
asmlinkage void gcm_init_htable(unsigned char htable[256], unsigned char
Xi[16]);
-asmlinkage void gcm_ghash_p8(unsigned char *Xi, unsigned char *Htable,
+asmlinkage void gcm_ghash_p10(unsigned char *Xi, unsigned char *Htable,
unsigned char *aad, unsigned int alen);
struct aes_key {
@@ -93,7 +93,7 @@ static void set_aad(struct gcm_ctx *gctx, struct Hash_ctx
*hash,
gctx->aadLen = alen;
i = alen & ~0xf;
if (i) {
- gcm_ghash_p8(nXi, hash->Htable+32, aad, i);
+ gcm_ghash_p10(nXi, hash->Htable+32, aad, i);
aad += i;
alen -= i;
}
@@ -102,7 +102,7 @@ static void set_aad(struct gcm_ctx *gctx, struct Hash_ctx
*hash,
nXi[i] ^= aad[i];
memset(gctx->aad_hash, 0, 16);
- gcm_ghash_p8(gctx->aad_hash,
Re: [PATCH v2] powerpc/cpuidle: Set CPUIDLE_FLAG_POLLING for snooze state
On Mon, Nov 14, 2022 at 08:26:11PM +0530, Aboorva Devarajan wrote:
> During the comparative study of cpuidle governors, it is noticed that the
> menu governor does not select CEDE state in some scenarios even though when
> the sleep duration of the CPU exceeds the target residency of the CEDE idle
> state this is because the CPU exits the snooze "polling" state when snooze
> time limit is reached in the snooze_loop(), which is not a real wake up
> and it just means that the polling state selection was not adequate.
>
> cpuidle governors rely on CPUIDLE_FLAG_POLLING flag to be set for the
> polling states to handle the condition mentioned above.
>
> Hence, set the CPUIDLE_FLAG_POLLING flag for snooze state (polling state)
> in powerpc arch to make the cpuidle governor work as expected.
>
> Reference Commits:
>
> - Timeout enabled for snooze state:
> commit 78eaa10f027c
> ("cpuidle: powernv/pseries: Auto-promotion of snooze to deeper idle state")
>
> - commit dc2251bf98c6
> ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
>
> - Fix wakeup stats in governor for polling states
> commit 5f26bdceb9c0
> ("cpuidle: menu: Fix wakeup statistics updates for polling state")
>
> Signed-off-by: Aboorva Devarajan
> ---
>
> Changelog: (v1 -> v2)
>
> Added CPUIDLE_POLLING_FLAG to the correct cpuidle_state struct.
>
> Previous version of the patch is stale which was sent by mistake, this
> is the correct version which is tested on powernv, pseries (shared and
> dedicated partitions)
>
> drivers/cpuidle/cpuidle-powernv.c | 5 -
> drivers/cpuidle/cpuidle-pseries.c | 8 ++--
> 2 files changed, 10 insertions(+), 3 deletions(-)
Thanks for the patch.
Tested it on top of v6.0-rc4
Against workload:
https://github.com/gautshen/misc/tree/master/cpuidle-smt-performance
Reviewed-by: Vishal Chourasia
Tested-by: Vishal Chourasia mailto:vish...@linux.vnet.ibm.com>>
|++---+--|
| wake up period | state | % time spent (before) | % time spent (after) |
|++---+--|
| 110 us | snooze | 95.40 % | 1.17 % |
|| CEDE | 0.03 %| 92.67 % |
|++---+--|
| 120 us | snooze | 96.37 % | 1.18 % |
|| CEDE | 0.05 %| 94.57 % |
|++---+--|
| 130 us | snooze | 17.12 % | 1.21 % |
|| CEDE | 78.16 % | 94.71 % |
|++---+--|
| 230 us | snooze | 95.38 % | 0.64 % |
|| CEDE | 2.55 %| 97.06 % |
|++---+--|
| 240 us | snooze | 96.86 % | 0.62 % |
|| CEDE | 1.14 %| 97.17 % |
|++---+--|
| 250 us | snooze | 1.38 %| 0.59 % |
|| CEDE | 96.46 % | 97.28 % |
|++---+--|
| 350 us | snooze | 62.91 % | 0.42 % |
|| CEDE | 35.56 % | 98.04 % |
|++---+--|
| 360 us | snooze | 11.93 % | 0.34 % |
|| CEDE | 86.56 % | 98.18 % |
|++---+--|
| 370 us | snooze | 6.21 %| 0.40 % |
|| CEDE | 92.31 % | 98.16 % |
|++---+--|
| 470 us | snooze | 42.06 % | 0.31 % |
|| CEDE | 56.74 % | 98.54 % |
|++---+--|
| 480 us | snooze | 64.67 % | 0.30 % |
|| CEDE | 34.14 % | 98.56 % |
|++---+--|
| 490 us | snooze | 0.57 %| 0.30 % |
|| CEDE | 98.31 % | 98.60 % |
|++---+--
Re: sched/debug: CPU hotplug operation suffers in a large cpu systems
Thanks Greg & Peter for your direction.
While we pursue the idea of having debugfs based on kernfs, we thought about
having a boot time parameter which would disable creating and updating of the
sched_domain debugfs files and this would also be useful even when the kernfs
solution kicks in, as users who may not care about these debugfs files would
benefit from a faster CPU hotplug operation.
However, these sched_domain debugfs files are created by default.
-- vishal.c
-->8-8<--
From f66f66ee05a9f719b58822d13e501d65391dd9d3 Mon Sep 17 00:00:00 2001
From: Vishal Chourasia
Date: Tue, 8 Nov 2022 14:21:15 +0530
Subject: [PATCH] Add kernel parameter to disable creation of sched_domain
files
For large systems, creation of sched_domain debug files takes unusually long
time. In which case, sched_sd_export can be passed as kernel command line
parameter during boot time to prevent kernel from creating sched_domain files.
This commit adds a kernel command line parameter, sched_sd_export, which can be
used to, optionally, disable the creation of sched_domain debug files.
---
kernel/sched/debug.c| 9 ++---
kernel/sched/sched.h| 1 +
kernel/sched/topology.c | 11 ++-
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index bb3d63bdf4ae..bd307847b76a 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -279,6 +279,7 @@ static const struct file_operations sched_dynamic_fops = {
#endif /* CONFIG_PREEMPT_DYNAMIC */
__read_mostly bool sched_debug_verbose;
+__read_mostly int sched_debug_export = 1;
static const struct seq_operations sched_debug_sops;
@@ -321,9 +322,11 @@ static __init int sched_init_debug(void)
debugfs_create_u32("migration_cost_ns", 0644, debugfs_sched,
_sched_migration_cost);
debugfs_create_u32("nr_migrate", 0644, debugfs_sched,
_sched_nr_migrate);
- mutex_lock(_domains_mutex);
- update_sched_domain_debugfs();
- mutex_unlock(_domains_mutex);
+ if (likely(sched_debug_export)) {
+ mutex_lock(_domains_mutex);
+ update_sched_domain_debugfs();
+ mutex_unlock(_domains_mutex);
+ }
#endif
#ifdef CONFIG_NUMA_BALANCING
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index e26688d387ae..a4d06588d876 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2738,6 +2738,7 @@ extern struct sched_entity *__pick_last_entity(struct
cfs_rq *cfs_rq);
#ifdef CONFIG_SCHED_DEBUG
extern bool sched_debug_verbose;
+extern int sched_debug_export;
extern void print_cfs_stats(struct seq_file *m, int cpu);
extern void print_rt_stats(struct seq_file *m, int cpu);
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 8739c2a5a54e..7bcdbc2f856d 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -19,6 +19,13 @@ static int __init sched_debug_setup(char *str)
}
early_param("sched_verbose", sched_debug_setup);
+static int __init sched_debug_disable_export(char *str)
+{
+ sched_debug_export = 0;
+ return 0;
+}
+early_param("sched_sd_export", sched_debug_disable_export);
+
static inline bool sched_debug(void)
{
return sched_debug_verbose;
@@ -152,6 +159,7 @@ static void sched_domain_debug(struct sched_domain *sd, int
cpu)
#else /* !CONFIG_SCHED_DEBUG */
# define sched_debug_verbose 0
+# define sched_debug_export 1
# define sched_domain_debug(sd, cpu) do { } while (0)
static inline bool sched_debug(void)
{
@@ -2632,7 +2640,8 @@ void partition_sched_domains_locked(int ndoms_new,
cpumask_var_t doms_new[],
dattr_cur = dattr_new;
ndoms_cur = ndoms_new;
- update_sched_domain_debugfs();
+ if (likely(sched_debug_export))
+ update_sched_domain_debugfs();
}
/*
base-commit: 7e18e42e4b280c85b76967a9106a13ca61c16179
--
2.31.1
signature.asc
Description: PGP signature
Re: sched/debug: CPU hotplug operation suffers in a large cpu systems
On Tue, Oct 18, 2022 at 01:04:40PM +0200, Greg Kroah-Hartman wrote: > Why do you need to? What tools require these debugfs files to be > present? We are not entirely sure what applications (if any) might be using this interface. > And if you only have 7-8 files per CPU, that does not seem like a lot of > files overall (14000-16000)? If you only offline 1 cpu, how is removing > 7 or 8 files a bottleneck? Do you really offline 1999 cpus for a 2k > system? It's 7-8 files per domain per cpu, so, in a system with approx 2k cpus and five domains, the total file count goes above 70k-80k files. And, when we offline 1 CPU, the entire directory is rebuilt, resulting in creation of all the files again. Thanks -- vishal.c signature.asc Description: PGP signature
