Re: [PATCH v4] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable

From: Tyler Baker
Date: Tue Apr 07 2015 - 12:19:01 EST

Next message: Steven Rostedt: "Re: [RESEND PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions"
Previous message: Andreas Ruprecht: "[PATCH] firmware: iSCSI: Remove unneeded #ifdef and associated dead code"
In reply to: Simon Horman: "Re: [PATCH v4] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable"
Next in thread: Stephen Boyd: "Re: [PATCH v4] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Stephen,

On 6 April 2015 at 13:24, Stephen Boyd <sboyd@xxxxxxxxxxxxxx> 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 <mark.rutland@xxxxxxx>
> Cc: Nicolas Pitre <nico@xxxxxxxxxx>
> Cc: Dave Martin <Dave.Martin@xxxxxxx>
> Cc: Simon Horman <horms@xxxxxxxxxxxx>
> Cc: Magnus Damm <magnus.damm@xxxxxxxxx>
> Cc: <linux-sh@xxxxxxxxxxxxxxx>
> Cc: Tyler Baker <tyler.baker@xxxxxxxxxx>
> Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
> ---
>
> 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 | 10 ++++++++++
> 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 +-
> 9 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..e15856a1a380 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
> @@ -123,4 +124,13 @@ struct of_cpu_method {
> */
> extern void smp_set_ops(struct smp_operations *);
>
> +#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 /* ifndef __ASM_ARM_SMP_H */
> 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);
> }
>

When I tested this patch before I only tried building
multi_v7_defconfig (which built fine) however I've added this patch to
my to-build branch and sent it through the CI loop. It appears that
there quite a few build errors on the other defconfigs[0][1].

../arch/arm/kernel/setup.c: In function 'topology_init':
../arch/arm/kernel/setup.c:977:3: error: implicit declaration of
function 'platform_can_hotplug_cpu'
[-Werror=implicit-function-declaration]
cc1: some warnings being treated as errors
make[2]: *** [arch/arm/kernel/setup.o] Error 1

> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 86ef244c5a24..f747fdf1cc91 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -168,13 +168,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] http://kernelci.org/build/tbaker/kernel/v4.0-rc6-189-g9108e703ce6d/
[1] http://kernelci.org/boot/all/job/tbaker/kernel/v4.0-rc6-189-g9108e703ce6d/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Steven Rostedt: "Re: [RESEND PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions"
Previous message: Andreas Ruprecht: "[PATCH] firmware: iSCSI: Remove unneeded #ifdef and associated dead code"
In reply to: Simon Horman: "Re: [PATCH v4] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable"
Next in thread: Stephen Boyd: "Re: [PATCH v4] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]