linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable
@ 2015-02-14  0:42 Stephen Boyd
  2015-02-16  9:03 ` Geert Uytterhoeven
  2015-02-18 22:27 ` Simon Horman
  0 siblings, 2 replies; 7+ messages in thread
From: Stephen Boyd @ 2015-02-14  0:42 UTC (permalink / raw)
  To: Russell King
  Cc: linux-kernel, linux-arm-kernel, Mark Rutland, Nicolas Pitre,
	Dave Martin, Simon Horman, Magnus Damm, linux-sh

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 current users use the op to
indicate if a CPU can be hotplugged or not in a static fashion.
With PSCI we may need to introduce 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 1, 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@arm.com>
Cc: Nicolas Pitre <nico@linaro.org>
Cc: Dave Martin <Dave.Martin@arm.com>
Cc: Simon Horman <horms@verge.net.au>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: <linux-sh@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

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..dc91ed04c25a 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 int 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 1;
 }
 
 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..525c006fdf7c 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);
+	int  (*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 715ae19bc7c8..c61c09defc78 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -974,7 +974,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 fe0386c751b2..152f65ef0494 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -179,13 +179,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 ? 0 : 1;
 }
+
 /*
  * __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 309025efd4cf..e124df267edc 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 int 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..a614cef18db1 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)
+int shmobile_smp_cpu_can_disable(unsigned int cpu)
 {
-	return 0; /* Hotplug of any CPU is supported */
+	return 1; /* 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 9c3da1345b8b..67b86c434fb8 100644
--- a/arch/arm/mach-shmobile/smp-r8a7790.c
+++ b/arch/arm/mach-shmobile/smp-r8a7790.c
@@ -63,7 +63,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 7e49e0a52e32..24b7f12e20bb 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 c16dbfe9836c..4dddbd3844dd 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable
  2015-02-14  0:42 [PATCH v3] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable Stephen Boyd
@ 2015-02-16  9:03 ` Geert Uytterhoeven
  2015-02-18 22:27 ` Simon Horman
  1 sibling, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2015-02-16  9:03 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Russell King, linux-kernel, linux-arm-kernel, Mark Rutland,
	Nicolas Pitre, Dave Martin, Simon Horman, Magnus Damm,
	Linux-sh list

On Sat, Feb 14, 2015 at 1:42 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> --- 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);
> +       int  (*cpu_can_disable)(unsigned int cpu);

Perhaps this should be changed to return bool while you're adding the function?
That saves us from an atomic update later when someone does janitorial work.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable
  2015-02-14  0:42 [PATCH v3] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable Stephen Boyd
  2015-02-16  9:03 ` Geert Uytterhoeven
@ 2015-02-18 22:27 ` Simon Horman
  2015-02-18 23:27   ` Stephen Boyd
  1 sibling, 1 reply; 7+ messages in thread
From: Simon Horman @ 2015-02-18 22:27 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Russell King, linux-kernel, linux-arm-kernel, Mark Rutland,
	Nicolas Pitre, Dave Martin, Magnus Damm, linux-sh

On Fri, Feb 13, 2015 at 04:42:54PM -0800, 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 current users use the op to
> indicate if a CPU can be hotplugged or not in a static fashion.
> With PSCI we may need to introduce 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 1, 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@arm.com>
> Cc: Nicolas Pitre <nico@linaro.org>
> Cc: Dave Martin <Dave.Martin@arm.com>
> Cc: Simon Horman <horms@verge.net.au>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: <linux-sh@vger.kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> 
> 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(-)

I think it would make sense to separate the ARM-core changes
from the mach-shmobile integration changes.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable
  2015-02-18 22:27 ` Simon Horman
@ 2015-02-18 23:27   ` Stephen Boyd
  2015-02-19 22:14     ` Simon Horman
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Boyd @ 2015-02-18 23:27 UTC (permalink / raw)
  To: Simon Horman
  Cc: Russell King, linux-kernel, linux-arm-kernel, Mark Rutland,
	Nicolas Pitre, Dave Martin, Magnus Damm, linux-sh

On 02/18/15 14:27, Simon Horman wrote:
> On Fri, Feb 13, 2015 at 04:42:54PM -0800, 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 current users use the op to
>> indicate if a CPU can be hotplugged or not in a static fashion.
>> With PSCI we may need to introduce 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 1, 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@arm.com>
>> Cc: Nicolas Pitre <nico@linaro.org>
>> Cc: Dave Martin <Dave.Martin@arm.com>
>> Cc: Simon Horman <horms@verge.net.au>
>> Cc: Magnus Damm <magnus.damm@gmail.com>
>> Cc: <linux-sh@vger.kernel.org>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>
>> 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(-)
> I think it would make sense to separate the ARM-core changes
> from the mach-shmobile integration changes.

Are you saying two (three?) patches to add the op, and then move over
each struct smp_operations? It's all going through rmk's tree so I'll
leave that up to him.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable
  2015-02-18 23:27   ` Stephen Boyd
@ 2015-02-19 22:14     ` Simon Horman
  2015-04-06 17:19       ` Tyler Baker
  0 siblings, 1 reply; 7+ messages in thread
From: Simon Horman @ 2015-02-19 22:14 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Russell King, linux-kernel, linux-arm-kernel, Mark Rutland,
	Nicolas Pitre, Dave Martin, Magnus Damm, linux-sh

On Wed, Feb 18, 2015 at 03:27:57PM -0800, Stephen Boyd wrote:
> On 02/18/15 14:27, Simon Horman wrote:
> > On Fri, Feb 13, 2015 at 04:42:54PM -0800, 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 current users use the op to
> >> indicate if a CPU can be hotplugged or not in a static fashion.
> >> With PSCI we may need to introduce 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 1, 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@arm.com>
> >> Cc: Nicolas Pitre <nico@linaro.org>
> >> Cc: Dave Martin <Dave.Martin@arm.com>
> >> Cc: Simon Horman <horms@verge.net.au>
> >> Cc: Magnus Damm <magnus.damm@gmail.com>
> >> Cc: <linux-sh@vger.kernel.org>
> >> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> >> ---
> >>
> >> 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(-)
> > I think it would make sense to separate the ARM-core changes
> > from the mach-shmobile integration changes.
> 
> Are you saying two (three?) patches to add the op, and then move over
> each struct smp_operations? It's all going through rmk's tree so I'll
> leave that up to him.

I'm also happy to let RMK to decide what he thinks is best.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable
  2015-02-19 22:14     ` Simon Horman
@ 2015-04-06 17:19       ` Tyler Baker
  2015-04-06 17:47         ` Stephen Boyd
  0 siblings, 1 reply; 7+ messages in thread
From: Tyler Baker @ 2015-04-06 17:19 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Mark Rutland, Russell King, Nicolas Pitre, linux-sh, Magnus Damm,
	linux-kernel, Dave Martin, linux-arm-kernel, Simon Horman,
	Kevin's boot bot

On 19 February 2015 at 14:14, Simon Horman <horms@verge.net.au> wrote:
> On Wed, Feb 18, 2015 at 03:27:57PM -0800, Stephen Boyd wrote:
>> On 02/18/15 14:27, Simon Horman wrote:
>> > On Fri, Feb 13, 2015 at 04:42:54PM -0800, 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 current users use the op to
>> >> indicate if a CPU can be hotplugged or not in a static fashion.
>> >> With PSCI we may need to introduce 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 1, 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@arm.com>
>> >> Cc: Nicolas Pitre <nico@linaro.org>
>> >> Cc: Dave Martin <Dave.Martin@arm.com>
>> >> Cc: Simon Horman <horms@verge.net.au>
>> >> Cc: Magnus Damm <magnus.damm@gmail.com>
>> >> Cc: <linux-sh@vger.kernel.org>
>> >> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> >> ---
>> >>
>> >> 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(-)
>> > I think it would make sense to separate the ARM-core changes
>> > from the mach-shmobile integration changes.
>>
>> Are you saying two (three?) patches to add the op, and then move over
>> each struct smp_operations? It's all going through rmk's tree so I'll
>> leave that up to him.
>
> I'm also happy to let RMK to decide what he thinks is best.

Apologies for bringing up an older thread, but was this change ever
picked up? I recently hit this issue when I was running the kselftest
suite (specifically the cpu-hotplug test case) on the ARM boards from
kernelci.org. I don't see it in -next so I've tested the core changes
and confirmed it the fixes the cpu-hotplug behavior on platforms that
do not support it.

>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

- Tyler

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable
  2015-04-06 17:19       ` Tyler Baker
@ 2015-04-06 17:47         ` Stephen Boyd
  0 siblings, 0 replies; 7+ messages in thread
From: Stephen Boyd @ 2015-04-06 17:47 UTC (permalink / raw)
  To: Tyler Baker, Mark Rutland
  Cc: Russell King, Nicolas Pitre, linux-sh, Magnus Damm, linux-kernel,
	Dave Martin, linux-arm-kernel, Simon Horman,
	Kevin's boot bot

On 04/06/15 10:19, Tyler Baker wrote:
> On 19 February 2015 at 14:14, Simon Horman <horms@verge.net.au> wrote:
>> On Wed, Feb 18, 2015 at 03:27:57PM -0800, Stephen Boyd wrote:
>>> On 02/18/15 14:27, Simon Horman wrote:
>>>> On Fri, Feb 13, 2015 at 04:42:54PM -0800, 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 current users use the op to
>>>>> indicate if a CPU can be hotplugged or not in a static fashion.
>>>>> With PSCI we may need to introduce 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 1, 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@arm.com>
>>>>> Cc: Nicolas Pitre <nico@linaro.org>
>>>>> Cc: Dave Martin <Dave.Martin@arm.com>
>>>>> Cc: Simon Horman <horms@verge.net.au>
>>>>> Cc: Magnus Damm <magnus.damm@gmail.com>
>>>>> Cc: <linux-sh@vger.kernel.org>
>>>>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>>>>> ---
>>>>>
>>>>> 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(-)
>>>> I think it would make sense to separate the ARM-core changes
>>>> from the mach-shmobile integration changes.
>>> Are you saying two (three?) patches to add the op, and then move over
>>> each struct smp_operations? It's all going through rmk's tree so I'll
>>> leave that up to him.
>> I'm also happy to let RMK to decide what he thinks is best.
> Apologies for bringing up an older thread, but was this change ever
> picked up? I recently hit this issue when I was running the kselftest
> suite (specifically the cpu-hotplug test case) on the ARM boards from
> kernelci.org. I don't see it in -next so I've tested the core changes
> and confirmed it the fixes the cpu-hotplug behavior on platforms that
> do not support it.

No it hasn't gone anywhere. It would be good if Mark Rutland could ack
the patch, which I hope would give enough confidence to Russell that the
patch is good. Your tested-by would also be welcome. I'll go make the
bool change that Geert suggested and resend.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-04-06 17:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-14  0:42 [PATCH v3] ARM: smp: Only expose /sys/.../cpuX/online if hotpluggable Stephen Boyd
2015-02-16  9:03 ` Geert Uytterhoeven
2015-02-18 22:27 ` Simon Horman
2015-02-18 23:27   ` Stephen Boyd
2015-02-19 22:14     ` Simon Horman
2015-04-06 17:19       ` Tyler Baker
2015-04-06 17:47         ` Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).