Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755383AbbDKRXH (ORCPT ); Sat, 11 Apr 2015 13:23:07 -0400 Received: from mail-wi0-f170.google.com ([209.85.212.170]:38384 "EHLO mail-wi0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753849AbbDKRXF (ORCPT ); Sat, 11 Apr 2015 13:23:05 -0400 MIME-Version: 1.0 In-Reply-To: <1428705191-15670-1-git-send-email-sboyd@codeaurora.org> References: <1428705191-15670-1-git-send-email-sboyd@codeaurora.org> Date: Sat, 11 Apr 2015 10:23:04 -0700 Message-ID: Subject: Re: [PATCH v5] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable From: Tyler Baker To: Stephen Boyd Cc: Russell King , "linux-kernel@vger.kernel.org" , linux-arm-kernel , Simon Horman , Mark Rutland , Nicolas Pitre , Dave Martin , Magnus Damm , "linux-sh@vger.kernel.org" , Geert Uytterhoeven Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 12502 Lines: 316 On 10 April 2015 at 15:33, Stephen Boyd wrote: > Writes to /sys/.../cpuX/online fail if we determine the platform > doesn't support hotplug for that CPU. Furthermore, if the cpu_die > op isn't specified the system hangs when we try to offline a CPU > and it comes right back online unexpectedly. Let's figure this > stuff out before we make the sysfs nodes so that the online file > doesn't even exist if it isn't (at least sometimes) possible to > hotplug the CPU. > > Add a new 'cpu_can_disable' op and repoint all 'cpu_disable' > implementations at it because all implementers use the op to > indicate if a CPU can be hotplugged or not in a static fashion. > With PSCI we may need to add a 'cpu_disable' op so that the > secure OS can be migrated off the CPU we're trying to hotplug. > In this case, the 'cpu_can_disable' op will indicate that all > CPUs are hotpluggable by returning true, but the 'cpu_disable' op > will make a PSCI migration call and occasionally fail, denying > the hotplug of a CPU. This shouldn't be any worse than x86 where > we may indicate that all CPUs are hotpluggable but occasionally > we can't offline a CPU due to check_irq_vectors_for_cpu_disable() > failing to find a CPU to move vectors to. > > Cc: Mark Rutland > Cc: Nicolas Pitre > Cc: Dave Martin > Acked-by: Simon Horman [shmobile portion] > Tested-by: Simon Horman > Cc: Magnus Damm > Cc: > Cc: Tyler Baker > Cc: Geert Uytterhoeven > Signed-off-by: Stephen Boyd > --- Tested-by: Tyler Baker I have tested v5 on top of next-20150410[0] and can confirm the build regression reported in v4 is resolved[1]. Additionally, 314 boot tests were performed[2] on arm, arm64 and x86 plaforms, no new regressions detected. I have executed the cpu-hotplug kselftest test case using the multi_v7_defconfig on an array of arm platforms. Platforms which do not support cpu hotplug show the behavior you would expect. The below result was executed on a sun9i-a80-optimus. Running tests in cpu-hotplug ======================================== CPU online/offline summary: Cpus in online state: 0 Cpus in offline state: 1-7 Limited scope test: one hotplug cpu (leaves cpu in the original state): online to offline to online: cpu 0 ./cpu-on-off-test.sh: line 186: can't create /sys/devices/system/cpu/cpu0/online: Permission denied 0: unexpected fail ./cpu-on-off-test.sh: line 186: can't create /sys/devices/system/cpu/cpu0/online: Permission denied 0: unexpected fail selftests: cpu-on-off-test.sh [XFAIL] Comparing this result to a tegra124-jetson-tk1 which supports cpu-hotplug. Running tests in cpu-hotplug ======================================== CPU online/offline summary: Cpus in online state: 0-3 Cpus in offline state: 0 Limited scope test: one hotplug cpu (leaves cpu in the original state): online to offline to online: cpu 3 [ 14.478074] CPU3: shutdown selftests: cpu-on-off-test.sh [PASS] Full list of kselftest cpu-hotplug results: sun9i-a80-optimus [XFAIL] sun9i-a80-cubieboard4 [XFAIL] sun7i-a20-cubietruck [PASS] tegra124-jetson-tk1 [PASS] exynos5800-peach-pi [PASS] exynos5422-odroidxu3 [XFAIL] exynos5250-arndale [PASS] exynos5250-snow [PASS] exynos5420-arndale-octa [PASS] exynos4412-odroidu3 [PASS] imx6q-cm-fx6 [PASS] imx6q-wandboard [PASS] imx6q-sabrelite [PASS] qcom-apq8084-ifc6540 [PASS] qcom-apq8064-ifc6410 [PASS] hip04-d01 [PASS] omap4-panda-es [PASS] am335x-boneblack [XFAIL] omap3-beagle-xm [XFAIL] ste-snowball [PASS zynq-parallella [PASS] (qemu) vexpress-v2p-ca15-tc1 [XFAIL] (qemu) vexpress-v2p-ca15_a7 [PASS] (qemu) vexpress-v2p-ca9 [XFAIL] > > Changes since v4: > * Fix build error on !SMP (Tyler Baker) > * Picked up Simon's ack/tested-by > > Changes since v3: > * Return bool instead of int from 'cpu_can_disable' op > > Changes since v2: > * Left cpu_disable op in place > * Split out shmobile function deletion > > arch/arm/common/mcpm_platsmp.c | 12 ++++-------- > arch/arm/include/asm/smp.h | 1 + > arch/arm/include/asm/smp_plat.h | 9 +++++++++ > arch/arm/kernel/setup.c | 2 +- > arch/arm/kernel/smp.c | 15 ++++++++++++++- > arch/arm/mach-shmobile/common.h | 2 +- > arch/arm/mach-shmobile/platsmp.c | 4 ++-- > arch/arm/mach-shmobile/smp-r8a7790.c | 2 +- > arch/arm/mach-shmobile/smp-r8a7791.c | 2 +- > arch/arm/mach-shmobile/smp-sh73a0.c | 2 +- > 10 files changed, 35 insertions(+), 16 deletions(-) > > diff --git a/arch/arm/common/mcpm_platsmp.c b/arch/arm/common/mcpm_platsmp.c > index 92e54d7c6f46..2b25b6038f66 100644 > --- a/arch/arm/common/mcpm_platsmp.c > +++ b/arch/arm/common/mcpm_platsmp.c > @@ -65,14 +65,10 @@ static int mcpm_cpu_kill(unsigned int cpu) > return !mcpm_wait_for_cpu_powerdown(pcpu, pcluster); > } > > -static int mcpm_cpu_disable(unsigned int cpu) > +static bool mcpm_cpu_can_disable(unsigned int cpu) > { > - /* > - * We assume all CPUs may be shut down. > - * This would be the hook to use for eventual Secure > - * OS migration requests as described in the PSCI spec. > - */ > - return 0; > + /* We assume all CPUs may be shut down. */ > + return true; > } > > static void mcpm_cpu_die(unsigned int cpu) > @@ -92,7 +88,7 @@ static struct smp_operations __initdata mcpm_smp_ops = { > .smp_secondary_init = mcpm_secondary_init, > #ifdef CONFIG_HOTPLUG_CPU > .cpu_kill = mcpm_cpu_kill, > - .cpu_disable = mcpm_cpu_disable, > + .cpu_can_disable = mcpm_cpu_can_disable, > .cpu_die = mcpm_cpu_die, > #endif > }; > diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h > index 18f5a554134f..2563453d7b37 100644 > --- a/arch/arm/include/asm/smp.h > +++ b/arch/arm/include/asm/smp.h > @@ -104,6 +104,7 @@ struct smp_operations { > #ifdef CONFIG_HOTPLUG_CPU > int (*cpu_kill)(unsigned int cpu); > void (*cpu_die)(unsigned int cpu); > + bool (*cpu_can_disable)(unsigned int cpu); > int (*cpu_disable)(unsigned int cpu); > #endif > #endif > diff --git a/arch/arm/include/asm/smp_plat.h b/arch/arm/include/asm/smp_plat.h > index 993e5224d8f7..f9080717fc88 100644 > --- a/arch/arm/include/asm/smp_plat.h > +++ b/arch/arm/include/asm/smp_plat.h > @@ -107,4 +107,13 @@ static inline u32 mpidr_hash_size(void) > extern int platform_can_secondary_boot(void); > extern int platform_can_cpu_hotplug(void); > > +#ifdef CONFIG_HOTPLUG_CPU > +extern int platform_can_hotplug_cpu(unsigned int cpu); > +#else > +static inline int platform_can_hotplug_cpu(unsigned int cpu) > +{ > + return 0; > +} > +#endif > + > #endif > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c > index 6c777e908a24..955d45d0f70c 100644 > --- a/arch/arm/kernel/setup.c > +++ b/arch/arm/kernel/setup.c > @@ -992,7 +992,7 @@ static int __init topology_init(void) > > for_each_possible_cpu(cpu) { > struct cpuinfo_arm *cpuinfo = &per_cpu(cpu_data, cpu); > - cpuinfo->cpu.hotpluggable = 1; > + cpuinfo->cpu.hotpluggable = platform_can_hotplug_cpu(cpu); > register_cpu(&cpuinfo->cpu, cpu); > } > > diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c > index cca5b8758185..8c834ecc09db 100644 > --- a/arch/arm/kernel/smp.c > +++ b/arch/arm/kernel/smp.c > @@ -173,13 +173,26 @@ static int platform_cpu_disable(unsigned int cpu) > if (smp_ops.cpu_disable) > return smp_ops.cpu_disable(cpu); > > + return 0; > +} > + > +int platform_can_hotplug_cpu(unsigned int cpu) > +{ > + /* cpu_die must be specified to support hotplug */ > + if (!smp_ops.cpu_die) > + return 0; > + > + if (smp_ops.cpu_can_disable) > + return smp_ops.cpu_can_disable(cpu); > + > /* > * By default, allow disabling all CPUs except the first one, > * since this is special on a lot of platforms, e.g. because > * of clock tick interrupts. > */ > - return cpu == 0 ? -EPERM : 0; > + return cpu != 0; > } > + > /* > * __cpu_disable runs on the processor to be shutdown. > */ > diff --git a/arch/arm/mach-shmobile/common.h b/arch/arm/mach-shmobile/common.h > index afc60bad6fd6..f2c4bf437ea7 100644 > --- a/arch/arm/mach-shmobile/common.h > +++ b/arch/arm/mach-shmobile/common.h > @@ -13,7 +13,7 @@ extern void shmobile_smp_boot(void); > extern void shmobile_smp_sleep(void); > extern void shmobile_smp_hook(unsigned int cpu, unsigned long fn, > unsigned long arg); > -extern int shmobile_smp_cpu_disable(unsigned int cpu); > +extern bool shmobile_smp_cpu_can_disable(unsigned int cpu); > extern void shmobile_invalidate_start(void); > extern void shmobile_boot_scu(void); > extern void shmobile_smp_scu_prepare_cpus(unsigned int max_cpus); > diff --git a/arch/arm/mach-shmobile/platsmp.c b/arch/arm/mach-shmobile/platsmp.c > index 3923e09e966d..b23378f3d7e1 100644 > --- a/arch/arm/mach-shmobile/platsmp.c > +++ b/arch/arm/mach-shmobile/platsmp.c > @@ -31,8 +31,8 @@ void shmobile_smp_hook(unsigned int cpu, unsigned long fn, unsigned long arg) > } > > #ifdef CONFIG_HOTPLUG_CPU > -int shmobile_smp_cpu_disable(unsigned int cpu) > +bool shmobile_smp_cpu_can_disable(unsigned int cpu) > { > - return 0; /* Hotplug of any CPU is supported */ > + return true; /* Hotplug of any CPU is supported */ > } > #endif > diff --git a/arch/arm/mach-shmobile/smp-r8a7790.c b/arch/arm/mach-shmobile/smp-r8a7790.c > index 930f45cbc08a..947e437cab68 100644 > --- a/arch/arm/mach-shmobile/smp-r8a7790.c > +++ b/arch/arm/mach-shmobile/smp-r8a7790.c > @@ -64,7 +64,7 @@ struct smp_operations r8a7790_smp_ops __initdata = { > .smp_prepare_cpus = r8a7790_smp_prepare_cpus, > .smp_boot_secondary = shmobile_smp_apmu_boot_secondary, > #ifdef CONFIG_HOTPLUG_CPU > - .cpu_disable = shmobile_smp_cpu_disable, > + .cpu_can_disable = shmobile_smp_cpu_can_disable, > .cpu_die = shmobile_smp_apmu_cpu_die, > .cpu_kill = shmobile_smp_apmu_cpu_kill, > #endif > diff --git a/arch/arm/mach-shmobile/smp-r8a7791.c b/arch/arm/mach-shmobile/smp-r8a7791.c > index 5e2d1db79afa..b2508c0d276b 100644 > --- a/arch/arm/mach-shmobile/smp-r8a7791.c > +++ b/arch/arm/mach-shmobile/smp-r8a7791.c > @@ -58,7 +58,7 @@ struct smp_operations r8a7791_smp_ops __initdata = { > .smp_prepare_cpus = r8a7791_smp_prepare_cpus, > .smp_boot_secondary = r8a7791_smp_boot_secondary, > #ifdef CONFIG_HOTPLUG_CPU > - .cpu_disable = shmobile_smp_cpu_disable, > + .cpu_can_disable = shmobile_smp_cpu_can_disable, > .cpu_die = shmobile_smp_apmu_cpu_die, > .cpu_kill = shmobile_smp_apmu_cpu_kill, > #endif > diff --git a/arch/arm/mach-shmobile/smp-sh73a0.c b/arch/arm/mach-shmobile/smp-sh73a0.c > index 2106d6b76a06..ae7c764fd6b4 100644 > --- a/arch/arm/mach-shmobile/smp-sh73a0.c > +++ b/arch/arm/mach-shmobile/smp-sh73a0.c > @@ -68,7 +68,7 @@ struct smp_operations sh73a0_smp_ops __initdata = { > .smp_prepare_cpus = sh73a0_smp_prepare_cpus, > .smp_boot_secondary = sh73a0_boot_secondary, > #ifdef CONFIG_HOTPLUG_CPU > - .cpu_disable = shmobile_smp_cpu_disable, > + .cpu_can_disable = shmobile_smp_cpu_can_disable, > .cpu_die = shmobile_smp_scu_cpu_die, > .cpu_kill = shmobile_smp_scu_cpu_kill, > #endif > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > Cheers, Tyler [0] https://git.linaro.org/people/tyler.baker/linux.git/shortlog/refs/heads/to-build [1] http://kernelci.org/build/tbaker/kernel/v4.0-rc7-10845-g43b069424ab4/ [2] http://kernelci.org/boot/all/job/tbaker/kernel/v4.0-rc7-10845-g43b069424ab4/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/