Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1344558 > unrolled thread

[patch 10/20] cpu/hotplug: Make target state writeable

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [patch 10/20] cpu/hotplug: Make target state writeable Thomas Gleixner <tglx@linutronix.de> - 2016-02-26 19:50 +0100
    Re: [patch 10/20] cpu/hotplug: Make target state writeable "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-27 00:50 +0100
      Re: [patch 10/20] cpu/hotplug: Make target state writeable Thomas Gleixner <tglx@linutronix.de> - 2016-02-27 08:50 +0100
        Re: [patch 10/20] cpu/hotplug: Make target state writeable "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-27 15:50 +0100
          Re: [patch 10/20] cpu/hotplug: Make target state writeable Thomas Gleixner <tglx@linutronix.de> - 2016-02-28 16:00 +0100
            Re: [patch 10/20] cpu/hotplug: Make target state writeable Thomas Gleixner <tglx@linutronix.de> - 2016-02-29 17:00 +0100
              Re: [patch 10/20] cpu/hotplug: Make target state writeable "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-03-01 03:00 +0100
    [tip:smp/hotplug] cpu/hotplug: Make target state writeable tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2016-03-01 21:00 +0100
      Re: [tip:smp/hotplug] cpu/hotplug: Make target state writeable Peter Zijlstra <peterz@infradead.org> - 2016-03-02 13:50 +0100
        Re: [tip:smp/hotplug] cpu/hotplug: Make target state writeable Thomas Gleixner <tglx@linutronix.de> - 2016-03-02 21:00 +0100

#1344558 — [patch 10/20] cpu/hotplug: Make target state writeable

Make it possible to write a target state to the per cpu state file, so we can
switch between states.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/cpu.c      |   66 +++++++++++++++++++++++++++++++++++++++++++++++-------
 lib/Kconfig.debug |   13 ++++++++++
 2 files changed, 71 insertions(+), 8 deletions(-)

Index: b/kernel/cpu.c
===================================================================
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -48,12 +48,14 @@ static DEFINE_PER_CPU(struct cpuhp_cpu_s
  * @teardown:	Teardown function of the step
  * @skip_onerr:	Do not invoke the functions on error rollback
  *		Will go away once the notifiers	are gone
+ * @cant_stop:	Bringup/teardown can't be stopped at this step
  */
 struct cpuhp_step {
 	const char	*name;
 	int		(*startup)(unsigned int cpu);
 	int		(*teardown)(unsigned int cpu);
 	bool		skip_onerr;
+	bool		cant_stop;
 };
 
 static DEFINE_MUTEX(cpuhp_state_mutex);
@@ -558,7 +560,7 @@ static int __ref _cpu_down(unsigned int
 	if (num_online_cpus() == 1)
 		return -EBUSY;
 
-	if (!cpu_online(cpu))
+	if (!cpu_present(cpu))
 		return -EINVAL;
 
 	cpu_hotplug_begin();
@@ -683,16 +685,25 @@ static int _cpu_up(unsigned int cpu, int
 
 	cpu_hotplug_begin();
 
-	if (cpu_online(cpu) || !cpu_present(cpu)) {
+	if (!cpu_present(cpu)) {
 		ret = -EINVAL;
 		goto out;
 	}
 
-	/* Let it fail before we try to bring the cpu up */
-	idle = idle_thread_get(cpu);
-	if (IS_ERR(idle)) {
-		ret = PTR_ERR(idle);
+	/*
+	 * The caller of do_cpu_up might have raced with another
+	 * caller. Ignore it for now.
+	 */
+	if (st->state >= target)
 		goto out;
+
+	if (st->state == CPUHP_OFFLINE) {
+		/* Let it fail before we try to bring the cpu up */
+		idle = idle_thread_get(cpu);
+		if (IS_ERR(idle)) {
+			ret = PTR_ERR(idle);
+			goto out;
+		}
 	}
 
 	cpuhp_tasks_frozen = tasks_frozen;
@@ -909,27 +920,32 @@ static struct cpuhp_step cpuhp_bp_states
 		.name			= "threads:create",
 		.startup		= smpboot_create_threads,
 		.teardown		= NULL,
+		.cant_stop		= true,
 	},
 	[CPUHP_NOTIFY_PREPARE] = {
 		.name			= "notify:prepare",
 		.startup		= notify_prepare,
 		.teardown		= notify_dead,
 		.skip_onerr		= true,
+		.cant_stop		= true,
 	},
 	[CPUHP_BRINGUP_CPU] = {
 		.name			= "cpu:bringup",
 		.startup		= bringup_cpu,
 		.teardown		= NULL,
+		.cant_stop		= true,
 	},
 	[CPUHP_TEARDOWN_CPU] = {
 		.name			= "cpu:teardown",
 		.startup		= NULL,
 		.teardown		= takedown_cpu,
+		.cant_stop		= true,
 	},
 	[CPUHP_NOTIFY_ONLINE] = {
 		.name			= "notify:online",
 		.startup		= notify_online,
 		.teardown		= notify_down_prepare,
+		.cant_stop		= true,
 	},
 #endif
 	[CPUHP_ONLINE] = {
@@ -947,6 +963,7 @@ static struct cpuhp_step cpuhp_ap_states
 		.startup		= notify_starting,
 		.teardown		= notify_dying,
 		.skip_onerr		= true,
+		.cant_stop		= true,
 	},
 #endif
 	[CPUHP_ONLINE] = {
@@ -979,6 +996,39 @@ static ssize_t show_cpuhp_state(struct d
 }
 static DEVICE_ATTR(state, 0444, show_cpuhp_state, NULL);
 
+static ssize_t write_cpuhp_target(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct cpuhp_cpu_state *st = per_cpu_ptr(&cpuhp_state, dev->id);
+	struct cpuhp_step *sp;
+	int target, ret;
+
+	ret = kstrtoint(buf, 10, &target);
+	if (ret)
+		return ret;
+
+#ifdef CONFIG_CPU_HOTPLUG_STATE_CONTROL
+	if (target < CPUHP_OFFLINE || target > CPUHP_ONLINE)
+		return -EINVAL;
+#else
+	if (target != CPUHP_OFFLINE && target != CPUHP_ONLINE)
+		return -EINVAL;
+#endif
+	mutex_lock(&cpuhp_state_mutex);
+	sp = cpuhp_get_step(target);
+	ret = !sp->name || sp->cant_stop ? -EINVAL : 0;
+	mutex_unlock(&cpuhp_state_mutex);
+	if (ret)
+		return ret;
+
+	if (st->state < target)
+		ret = do_cpu_up(dev->id, target);
+	else
+		ret = do_cpu_down(dev->id, target);
+	return ret ? ret : count;
+}
+
 static ssize_t show_cpuhp_target(struct device *dev,
 				 struct device_attribute *attr, char *buf)
 {
@@ -986,7 +1036,7 @@ static ssize_t show_cpuhp_target(struct
 
 	return sprintf(buf, "%d\n", st->target);
 }
-static DEVICE_ATTR(target, 0444, show_cpuhp_target, NULL);
+static DEVICE_ATTR(target, 0644, show_cpuhp_target, write_cpuhp_target);
 
 static struct attribute *cpuhp_cpu_attrs[] = {
 	&dev_attr_state.attr,
@@ -1007,7 +1057,7 @@ static ssize_t show_cpuhp_states(struct
 	int i;
 
 	mutex_lock(&cpuhp_state_mutex);
-	for (i = 0; i <= CPUHP_ONLINE; i++) {
+	for (i = CPUHP_OFFLINE; i <= CPUHP_ONLINE; i++) {
 		struct cpuhp_step *sp = cpuhp_get_step(i);
 
 		if (sp->name) {
Index: b/lib/Kconfig.debug
===================================================================
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1442,6 +1442,19 @@ config DEBUG_BLOCK_EXT_DEVT
 
 	  Say N if you are unsure.
 
+config CPU_HOTPLUG_STATE_CONTROL
+	bool "Enable CPU hotplug state control"
+	depends on DEBUG_KERNEL
+	depends on HOTPLUG_CPU
+	default n
+	help
+	  Allows to write steps between "offline" and "online" to the CPUs
+	  sysfs target file so states can be stepped granular. This is a debug
+	  option for now as the hotplug machinery cannot be stopped and
+	  restarted at arbitrary points yet.
+
+	  Say N if your are unsure.
+
 config NOTIFIER_ERROR_INJECTION
 	tristate "Notifier error injection"
 	depends on DEBUG_KERNEL

[toc] | [next] | [standalone]

#1344849

Hi Thomas,

On Friday, February 26, 2016 06:43:32 PM Thomas Gleixner wrote:
> Make it possible to write a target state to the per cpu state file, so we can
> switch between states.

One thing that potentially may be problematic here is that any kind of
"offline" operations needs to be carried out under device_hotplug_lock,
because there are cases in which devices (including CPUs) are taken
offline in groups and if one offline fails, the whole operation has to
be rolled back.

So if you put a CPU into one of the intermediate states manually and
something like the above happens in parallel with it, they may not
play well together IMO.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1344938

Rafael,

On Sat, 27 Feb 2016, Rafael J. Wysocki wrote:
> On Friday, February 26, 2016 06:43:32 PM Thomas Gleixner wrote:
> > Make it possible to write a target state to the per cpu state file, so we can
> > switch between states.
> 
> One thing that potentially may be problematic here is that any kind of
> "offline" operations needs to be carried out under device_hotplug_lock,
> because there are cases in which devices (including CPUs) are taken
> offline in groups and if one offline fails, the whole operation has to
> be rolled back.
>
> So if you put a CPU into one of the intermediate states manually and
> something like the above happens in parallel with it, they may not
> play well together IMO.

I don't see how that is related. device_hotplug_lock is completely independent
of cpu hotplug today, unless I'm missing some magic connection here.

Physical CPU hotplug is a different story, but that's about bringing the cpus
into the system or taking them out. Sure, if you want to take one or more cpus
physically out, you have to bring them offline first. If you plug them in then
it's not necessarily related to actually bringing them online. That's a
different set of operations.

We surely need to look into that aspect, but I don't see a reason why e.g. a
device hotplug operation should be in any way related to the intermediate
state of a particular cpu. If that's the case, then there is something really
wrong.

I'm aware that we have a gazillion of silly assumptions all over the place and
some of them are wrong today and just do not explode in our face simply
because it's extremly hard to trigger. That's one reason why we need to go
through all the cpu notifier related sites and inspect them deeply.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1345025

On Saturday, February 27, 2016 08:39:42 AM Thomas Gleixner wrote:
> Rafael,
> 
> On Sat, 27 Feb 2016, Rafael J. Wysocki wrote:
> > On Friday, February 26, 2016 06:43:32 PM Thomas Gleixner wrote:
> > > Make it possible to write a target state to the per cpu state file, so we can
> > > switch between states.
> > 
> > One thing that potentially may be problematic here is that any kind of
> > "offline" operations needs to be carried out under device_hotplug_lock,
> > because there are cases in which devices (including CPUs) are taken
> > offline in groups and if one offline fails, the whole operation has to
> > be rolled back.
> >
> > So if you put a CPU into one of the intermediate states manually and
> > something like the above happens in parallel with it, they may not
> > play well together IMO.
> 
> I don't see how that is related. device_hotplug_lock is completely independent
> of cpu hotplug today, unless I'm missing some magic connection here.

Well, there is a magic connection which is my point.  That's mostly about
physical hot-remove.

> Physical CPU hotplug is a different story, but that's about bringing the cpus
> into the system or taking them out. Sure, if you want to take one or more cpus
> physically out, you have to bring them offline first. If you plug them in then
> it's not necessarily related to actually bringing them online. That's a
> different set of operations.

So that's mostly about the hot-remove part.  Namely, devices may need to go
away together (like in one package), so we need to offline them together first.

That's because generally offline may fail, for example for memory, and now
if a CPU is bundled to a set of memory that cannot be taken offline, it
can't be hot-removed too.  If offline fails for one component, we roll back,
but if it is successful for all of them, we can eject the whole bundle and
that's where the problem resides.

Say we've taken all of them offline and now we are ready to eject.  If an
online from sysfs (or any other place) comes in at this point, we'll be
ejecting a CPU that's potentially doing something which is not awesome.

That's why we have device_hotplug_lock and some ugly code related to it.

It extends to parents and children somewhat because of device objects
representing packages (we want those to be "offline" only if all their
children are offline) and that's why the lock is held around offline from
sysfs too.

I'm not entirely happy with this for quite obvious reasons, but it gets
the job done ATM.

> We surely need to look into that aspect, but I don't see a reason why e.g. a
> device hotplug operation should be in any way related to the intermediate
> state of a particular cpu. If that's the case, then there is something really
> wrong.

If that state is different from complete offline, we should not try to eject
(the package containing) that CPU.

> I'm aware that we have a gazillion of silly assumptions all over the place and
> some of them are wrong today and just do not explode in our face simply
> because it's extremly hard to trigger. That's one reason why we need to go
> through all the cpu notifier related sites and inspect them deeply.

Agreed, but the particular concern I'm talking about is not in that category IMO.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1345298

Rafael,

On Sat, 27 Feb 2016, Rafael J. Wysocki wrote:
> Say we've taken all of them offline and now we are ready to eject.  If an
> online from sysfs (or any other place) comes in at this point, we'll be
> ejecting a CPU that's potentially doing something which is not awesome.
> 
> That's why we have device_hotplug_lock and some ugly code related to it.
> 
> It extends to parents and children somewhat because of device objects
> representing packages (we want those to be "offline" only if all their
> children are offline) and that's why the lock is held around offline from
> sysfs too.
> 
> I'm not entirely happy with this for quite obvious reasons, but it gets
> the job done ATM.

Understood. I'll fix that thing up so that won't happen and I put it on the
list of things to look at deeper.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1345938

On Sun, 28 Feb 2016, Thomas Gleixner wrote:
> 
> Understood. I'll fix that thing up so that won't happen and I put it on the
> list of things to look at deeper.

The below delta patch should do what you need, right?

Thanks,

	tglx

8<-------------

--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1461,6 +1461,11 @@ static ssize_t write_cpuhp_target(struct
 	if (target != CPUHP_OFFLINE && target != CPUHP_ONLINE)
 		return -EINVAL;
 #endif
+
+	ret = lock_device_hotplug_sysfs();
+	if (ret)
+		return ret;
+
 	mutex_lock(&cpuhp_state_mutex);
 	sp = cpuhp_get_step(target);
 	ret = !sp->name || sp->cant_stop ? -EINVAL : 0;
@@ -1472,6 +1477,8 @@ static ssize_t write_cpuhp_target(struct
 		ret = do_cpu_up(dev->id, target);
 	else
 		ret = do_cpu_down(dev->id, target);
+
+	unlock_device_hotplug();
 	return ret ? ret : count;
 }
 

[toc] | [prev] | [next] | [standalone]

#1346286

On Monday, February 29, 2016 04:49:24 PM Thomas Gleixner wrote:
> On Sun, 28 Feb 2016, Thomas Gleixner wrote:
> > 
> > Understood. I'll fix that thing up so that won't happen and I put it on the
> > list of things to look at deeper.
> 
> The below delta patch should do what you need, right?

Yup, thanks!

Rafael


> 8<-------------
> 
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1461,6 +1461,11 @@ static ssize_t write_cpuhp_target(struct
>  	if (target != CPUHP_OFFLINE && target != CPUHP_ONLINE)
>  		return -EINVAL;
>  #endif
> +
> +	ret = lock_device_hotplug_sysfs();
> +	if (ret)
> +		return ret;
> +
>  	mutex_lock(&cpuhp_state_mutex);
>  	sp = cpuhp_get_step(target);
>  	ret = !sp->name || sp->cant_stop ? -EINVAL : 0;
> @@ -1472,6 +1477,8 @@ static ssize_t write_cpuhp_target(struct
>  		ret = do_cpu_up(dev->id, target);
>  	else
>  		ret = do_cpu_down(dev->id, target);
> +
> +	unlock_device_hotplug();
>  	return ret ? ret : count;
>  }

[toc] | [prev] | [next] | [standalone]

#1346993 — [tip:smp/hotplug] cpu/hotplug: Make target state writeable

Commit-ID:  757c989b9994f51b42d6be1bd33c7c12d16a3ac7
Gitweb:     http://git.kernel.org/tip/757c989b9994f51b42d6be1bd33c7c12d16a3ac7
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Fri, 26 Feb 2016 18:43:32 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 1 Mar 2016 20:36:55 +0100

cpu/hotplug: Make target state writeable

Make it possible to write a target state to the per cpu state file, so we can
switch between states.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arch@vger.kernel.org
Cc: Rik van Riel <riel@redhat.com>
Cc: Rafael Wysocki <rafael.j.wysocki@intel.com>
Cc: "Srivatsa S. Bhat" <srivatsa@mit.edu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Paul McKenney <paulmck@linux.vnet.ibm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul Turner <pjt@google.com>
Link: http://lkml.kernel.org/r/20160226182341.022814799@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/cpu.c      | 73 +++++++++++++++++++++++++++++++++++++++++++++++++------
 lib/Kconfig.debug | 13 ++++++++++
 2 files changed, 78 insertions(+), 8 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 1979b89..be9335d 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -48,12 +48,14 @@ static DEFINE_PER_CPU(struct cpuhp_cpu_state, cpuhp_state);
  * @teardown:	Teardown function of the step
  * @skip_onerr:	Do not invoke the functions on error rollback
  *		Will go away once the notifiers	are gone
+ * @cant_stop:	Bringup/teardown can't be stopped at this step
  */
 struct cpuhp_step {
 	const char	*name;
 	int		(*startup)(unsigned int cpu);
 	int		(*teardown)(unsigned int cpu);
 	bool		skip_onerr;
+	bool		cant_stop;
 };
 
 static DEFINE_MUTEX(cpuhp_state_mutex);
@@ -558,7 +560,7 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen,
 	if (num_online_cpus() == 1)
 		return -EBUSY;
 
-	if (!cpu_online(cpu))
+	if (!cpu_present(cpu))
 		return -EINVAL;
 
 	cpu_hotplug_begin();
@@ -683,16 +685,25 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen, enum cpuhp_state target)
 
 	cpu_hotplug_begin();
 
-	if (cpu_online(cpu) || !cpu_present(cpu)) {
+	if (!cpu_present(cpu)) {
 		ret = -EINVAL;
 		goto out;
 	}
 
-	/* Let it fail before we try to bring the cpu up */
-	idle = idle_thread_get(cpu);
-	if (IS_ERR(idle)) {
-		ret = PTR_ERR(idle);
+	/*
+	 * The caller of do_cpu_up might have raced with another
+	 * caller. Ignore it for now.
+	 */
+	if (st->state >= target)
 		goto out;
+
+	if (st->state == CPUHP_OFFLINE) {
+		/* Let it fail before we try to bring the cpu up */
+		idle = idle_thread_get(cpu);
+		if (IS_ERR(idle)) {
+			ret = PTR_ERR(idle);
+			goto out;
+		}
 	}
 
 	cpuhp_tasks_frozen = tasks_frozen;
@@ -909,27 +920,32 @@ static struct cpuhp_step cpuhp_bp_states[] = {
 		.name			= "threads:create",
 		.startup		= smpboot_create_threads,
 		.teardown		= NULL,
+		.cant_stop		= true,
 	},
 	[CPUHP_NOTIFY_PREPARE] = {
 		.name			= "notify:prepare",
 		.startup		= notify_prepare,
 		.teardown		= notify_dead,
 		.skip_onerr		= true,
+		.cant_stop		= true,
 	},
 	[CPUHP_BRINGUP_CPU] = {
 		.name			= "cpu:bringup",
 		.startup		= bringup_cpu,
 		.teardown		= NULL,
+		.cant_stop		= true,
 	},
 	[CPUHP_TEARDOWN_CPU] = {
 		.name			= "cpu:teardown",
 		.startup		= NULL,
 		.teardown		= takedown_cpu,
+		.cant_stop		= true,
 	},
 	[CPUHP_NOTIFY_ONLINE] = {
 		.name			= "notify:online",
 		.startup		= notify_online,
 		.teardown		= notify_down_prepare,
+		.cant_stop		= true,
 	},
 #endif
 	[CPUHP_ONLINE] = {
@@ -947,6 +963,7 @@ static struct cpuhp_step cpuhp_ap_states[] = {
 		.startup		= notify_starting,
 		.teardown		= notify_dying,
 		.skip_onerr		= true,
+		.cant_stop		= true,
 	},
 #endif
 	[CPUHP_ONLINE] = {
@@ -979,6 +996,46 @@ static ssize_t show_cpuhp_state(struct device *dev,
 }
 static DEVICE_ATTR(state, 0444, show_cpuhp_state, NULL);
 
+static ssize_t write_cpuhp_target(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct cpuhp_cpu_state *st = per_cpu_ptr(&cpuhp_state, dev->id);
+	struct cpuhp_step *sp;
+	int target, ret;
+
+	ret = kstrtoint(buf, 10, &target);
+	if (ret)
+		return ret;
+
+#ifdef CONFIG_CPU_HOTPLUG_STATE_CONTROL
+	if (target < CPUHP_OFFLINE || target > CPUHP_ONLINE)
+		return -EINVAL;
+#else
+	if (target != CPUHP_OFFLINE && target != CPUHP_ONLINE)
+		return -EINVAL;
+#endif
+
+	ret = lock_device_hotplug_sysfs();
+	if (ret)
+		return ret;
+
+	mutex_lock(&cpuhp_state_mutex);
+	sp = cpuhp_get_step(target);
+	ret = !sp->name || sp->cant_stop ? -EINVAL : 0;
+	mutex_unlock(&cpuhp_state_mutex);
+	if (ret)
+		return ret;
+
+	if (st->state < target)
+		ret = do_cpu_up(dev->id, target);
+	else
+		ret = do_cpu_down(dev->id, target);
+
+	unlock_device_hotplug();
+	return ret ? ret : count;
+}
+
 static ssize_t show_cpuhp_target(struct device *dev,
 				 struct device_attribute *attr, char *buf)
 {
@@ -986,7 +1043,7 @@ static ssize_t show_cpuhp_target(struct device *dev,
 
 	return sprintf(buf, "%d\n", st->target);
 }
-static DEVICE_ATTR(target, 0444, show_cpuhp_target, NULL);
+static DEVICE_ATTR(target, 0644, show_cpuhp_target, write_cpuhp_target);
 
 static struct attribute *cpuhp_cpu_attrs[] = {
 	&dev_attr_state.attr,
@@ -1007,7 +1064,7 @@ static ssize_t show_cpuhp_states(struct device *dev,
 	int i;
 
 	mutex_lock(&cpuhp_state_mutex);
-	for (i = 0; i <= CPUHP_ONLINE; i++) {
+	for (i = CPUHP_OFFLINE; i <= CPUHP_ONLINE; i++) {
 		struct cpuhp_step *sp = cpuhp_get_step(i);
 
 		if (sp->name) {
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 8bfd1ac..f28f7fa 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1442,6 +1442,19 @@ config DEBUG_BLOCK_EXT_DEVT
 
 	  Say N if you are unsure.
 
+config CPU_HOTPLUG_STATE_CONTROL
+	bool "Enable CPU hotplug state control"
+	depends on DEBUG_KERNEL
+	depends on HOTPLUG_CPU
+	default n
+	help
+	  Allows to write steps between "offline" and "online" to the CPUs
+	  sysfs target file so states can be stepped granular. This is a debug
+	  option for now as the hotplug machinery cannot be stopped and
+	  restarted at arbitrary points yet.
+
+	  Say N if your are unsure.
+
 config NOTIFIER_ERROR_INJECTION
 	tristate "Notifier error injection"
 	depends on DEBUG_KERNEL

[toc] | [prev] | [next] | [standalone]

#1348012 — Re: [tip:smp/hotplug] cpu/hotplug: Make target state writeable

On Tue, Mar 01, 2016 at 11:55:01AM -0800, tip-bot for Thomas Gleixner wrote:
> Commit-ID:  757c989b9994f51b42d6be1bd33c7c12d16a3ac7
> Gitweb:     http://git.kernel.org/tip/757c989b9994f51b42d6be1bd33c7c12d16a3ac7
> Author:     Thomas Gleixner <tglx@linutronix.de>
> AuthorDate: Fri, 26 Feb 2016 18:43:32 +0000
> Committer:  Thomas Gleixner <tglx@linutronix.de>
> CommitDate: Tue, 1 Mar 2016 20:36:55 +0100
> 
> cpu/hotplug: Make target state writeable
> 
> Make it possible to write a target state to the per cpu state file, so we can
> switch between states.

But you've marked nearly all of them cant_stop, so while this implements
the interface, its not really functional just yet, right?

[toc] | [prev] | [next] | [standalone]

#1348433 — Re: [tip:smp/hotplug] cpu/hotplug: Make target state writeable

On Wed, 2 Mar 2016, Peter Zijlstra wrote:
> On Tue, Mar 01, 2016 at 11:55:01AM -0800, tip-bot for Thomas Gleixner wrote:
> > Commit-ID:  757c989b9994f51b42d6be1bd33c7c12d16a3ac7
> > Gitweb:     http://git.kernel.org/tip/757c989b9994f51b42d6be1bd33c7c12d16a3ac7
> > Author:     Thomas Gleixner <tglx@linutronix.de>
> > AuthorDate: Fri, 26 Feb 2016 18:43:32 +0000
> > Committer:  Thomas Gleixner <tglx@linutronix.de>
> > CommitDate: Tue, 1 Mar 2016 20:36:55 +0100
> > 
> > cpu/hotplug: Make target state writeable
> > 
> > Make it possible to write a target state to the per cpu state file, so we can
> > switch between states.
> 
> But you've marked nearly all of them cant_stop, so while this implements
> the interface, its not really functional just yet, right?

Yes, the current state is more or less wrapping the existing notifiers and
stopping them is not a brilliant idea. I've tried and it exploded nicely in my
face. The ones we convert over to the state machine will be stoppable and
reversible. And at the end the notifiers will be gone and with them the can't
stop stuff, except maybe for a few crucial points.

Thanks,

	tglx

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web