[patch 0/2] stop_machine: create kstop threads only when needed

[patch 0/2] stop_machine: create kstop threads only when needed

[Heiko Carstens]

Introduce stop_machine_create/destroy. With this interface subsystems
that need a non-failing stop_machine environment can create the
stop_machine threads before actually calling stop_machine.
When the threads aren't needed anymore they can be killed with
stop_machine_destroy again.

When stop_machine gets called and the threads aren't present they
will be created and destroyed automatically.

This restores the old behaviour of stop_machine from 2.6.27 and thus
we get rid of the normally not needed kstop threads.

The second patch converts the module loader/unloder code to the new
interface, since it needs a non-failing stop_machine infrastructure.

Looks like there are more users that need a non-failing version.
I'll take care of them when I'm back from vacation.

[patch 1/2] stop_machine: introduce stop_machine_create/destroy.

[Heiko Carstens]

[-- Attachment #1: 001_stop.diff --]
[-- Type: text/plain, Size: 5299 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

Introduce stop_machine_create/destroy. With this interface subsystems
that need a non-failing stop_machine environment can create the
stop_machine machine threads before actually calling stop_machine.
When the threads aren't needed anymore they can be killed with
stop_machine_destroy again.

When stop_machine gets called and the threads aren't present they
will be created and destroyed automatically. This restores the old
behaviour of stop_machine.

This patch also converts cpu hotplug to the new interface since it
is special: cpu_down calls __stop_machine instead of stop_machine.
However the kstop threads will only be created when stop_machine
gets called.

Changing the code so that the threads would be created automatically
on __stop_machine is currently not possible: when __stop_machine gets
called we hold cpu_add_remove_lock, which is the same lock that
create_rt_workqueue would take. So the workqueue needs to be created
before the cpu hotplug code locks cpu_add_remove_lock.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 include/linux/stop_machine.h |   22 +++++++++++++++++
 kernel/cpu.c                 |    6 +++-
 kernel/stop_machine.c        |   55 +++++++++++++++++++++++++++++++++++--------
 3 files changed, 72 insertions(+), 11 deletions(-)

Index: linux-2.6/kernel/stop_machine.c
===================================================================
--- linux-2.6.orig/kernel/stop_machine.c
+++ linux-2.6/kernel/stop_machine.c
@@ -38,7 +38,10 @@ struct stop_machine_data {
 static unsigned int num_threads;
 static atomic_t thread_ack;
 static DEFINE_MUTEX(lock);
-
+/* setup_lock protects refcount, stop_machine_wq and stop_machine_work. */
+static DEFINE_MUTEX(setup_lock);
+/* Users of stop_machine. */
+static int refcount;
 static struct workqueue_struct *stop_machine_wq;
 static struct stop_machine_data active, idle;
 static const cpumask_t *active_cpus;
@@ -109,6 +112,43 @@ static int chill(void *unused)
 	return 0;
 }
 
+int stop_machine_create(void)
+{
+	mutex_lock(&setup_lock);
+	if (refcount)
+		goto done;
+	stop_machine_wq = create_rt_workqueue("kstop");
+	if (!stop_machine_wq)
+		goto err_out;
+	stop_machine_work = alloc_percpu(struct work_struct);
+	if (!stop_machine_work)
+		goto err_out;
+done:
+	refcount++;
+	mutex_unlock(&setup_lock);
+	return 0;
+
+err_out:
+	if (stop_machine_wq)
+		destroy_workqueue(stop_machine_wq);
+	mutex_unlock(&setup_lock);
+	return -ENOMEM;
+}
+EXPORT_SYMBOL_GPL(stop_machine_create);
+
+void stop_machine_destroy(void)
+{
+	mutex_lock(&setup_lock);
+	refcount--;
+	if (refcount)
+		goto done;
+	destroy_workqueue(stop_machine_wq);
+	free_percpu(stop_machine_work);
+done:
+	mutex_unlock(&setup_lock);
+}
+EXPORT_SYMBOL_GPL(stop_machine_destroy);
+
 int __stop_machine(int (*fn)(void *), void *data, const cpumask_t *cpus)
 {
 	struct work_struct *sm_work;
@@ -146,19 +186,14 @@ int stop_machine(int (*fn)(void *), void
 {
 	int ret;
 
+	ret = stop_machine_create();
+	if (ret)
+		return ret;
 	/* No CPUs can come up or down during this. */
 	get_online_cpus();
 	ret = __stop_machine(fn, data, cpus);
 	put_online_cpus();
-
+	stop_machine_destroy();
 	return ret;
 }
 EXPORT_SYMBOL_GPL(stop_machine);
-
-static int __init stop_machine_init(void)
-{
-	stop_machine_wq = create_rt_workqueue("kstop");
-	stop_machine_work = alloc_percpu(struct work_struct);
-	return 0;
-}
-core_initcall(stop_machine_init);
Index: linux-2.6/kernel/cpu.c
===================================================================
--- linux-2.6.orig/kernel/cpu.c
+++ linux-2.6/kernel/cpu.c
@@ -290,8 +290,11 @@ out_release:
 
 int __ref cpu_down(unsigned int cpu)
 {
-	int err = 0;
+	int err;
 
+	err = stop_machine_create();
+	if (err)
+		return err;
 	cpu_maps_update_begin();
 
 	if (cpu_hotplug_disabled) {
@@ -318,6 +321,7 @@ int __ref cpu_down(unsigned int cpu)
 
 out:
 	cpu_maps_update_done();
+	stop_machine_destroy();
 	return err;
 }
 EXPORT_SYMBOL(cpu_down);
Index: linux-2.6/include/linux/stop_machine.h
===================================================================
--- linux-2.6.orig/include/linux/stop_machine.h
+++ linux-2.6/include/linux/stop_machine.h
@@ -35,6 +35,24 @@ int stop_machine(int (*fn)(void *), void
  * won't come or go while it's being called.  Used by hotplug cpu.
  */
 int __stop_machine(int (*fn)(void *), void *data, const cpumask_t *cpus);
+
+/**
+ * stop_machine_create: create all stop_machine threads
+ *
+ * Description: This causes all stop_machine threads to be created before
+ * stop_machine actually gets called. This can be used by subsystems that
+ * need a non failing stop_machine infrastructure.
+ */
+int stop_machine_create(void);
+
+/**
+ * stop_machine_destroy: destroy all stop_machine threads
+ *
+ * Description: This causes all stop_machine threads which were created with
+ * stop_machine_create to be destroyed again.
+ */
+void stop_machine_destroy(void);
+
 #else
 
 static inline int stop_machine(int (*fn)(void *), void *data,
@@ -46,5 +64,9 @@ static inline int stop_machine(int (*fn)
 	local_irq_enable();
 	return ret;
 }
+
+static inline int stop_machine_create(void) { return 0; }
+static inline void stop_machine_destroy(void) { }
+
 #endif /* CONFIG_SMP */
 #endif /* _LINUX_STOP_MACHINE */

-- 

[Bug 12492] Re: [patch 1/2] stop_machine: introduce stop_machine_create/destroy.

[Andrew Morton]

On Mon, 22 Dec 2008 12:36:30 +0100 Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> Introduce stop_machine_create/destroy. With this interface subsystems
> that need a non-failing stop_machine environment can create the
> stop_machine machine threads before actually calling stop_machine.
> When the threads aren't needed anymore they can be killed with
> stop_machine_destroy again.
> 
> When stop_machine gets called and the threads aren't present they
> will be created and destroyed automatically. This restores the old
> behaviour of stop_machine.
> 
> This patch also converts cpu hotplug to the new interface since it
> is special: cpu_down calls __stop_machine instead of stop_machine.
> However the kstop threads will only be created when stop_machine
> gets called.
> 
> Changing the code so that the threads would be created automatically
> on __stop_machine is currently not possible: when __stop_machine gets
> called we hold cpu_add_remove_lock, which is the same lock that
> create_rt_workqueue would take. So the workqueue needs to be created
> before the cpu hotplug code locks cpu_add_remove_lock.

In http://bugzilla.kernel.org/show_bug.cgi?id=12492, Thomas (cc'ed
here) reports

  Commit 9ea09af3bd3090e8349ca2899ca2011bd94cda85 introduced a
  regression that caused the kernel to fail to suspend.  The 'sleeping'
  LED on the laptop just keeps blinking and the laptop never shuts
  down.  I think this was eventually fixed because with 2.6.29-rc1 and
  -rc2 the laptop suspends fine, but fails to resume.  When I try to
  resume, all I see is a blinking cursor in the top left corner of the
  screen.

  I'm using acpi_sleep=s3_bios,s3_mode, suspending using a script
  that does: echo mem > /sys/power/state.

Re: [Bug 12492] Re: [patch 1/2] stop_machine: introduce stop_machine_create/destroy.

[Andrew Morton]

On Tue, 27 Jan 2009 00:04:32 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Mon, 22 Dec 2008 12:36:30 +0100 Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> 
> > Introduce stop_machine_create/destroy. With this interface subsystems
> > that need a non-failing stop_machine environment can create the
> > stop_machine machine threads before actually calling stop_machine.
> > When the threads aren't needed anymore they can be killed with
> > stop_machine_destroy again.
> > 
> > When stop_machine gets called and the threads aren't present they
> > will be created and destroyed automatically. This restores the old
> > behaviour of stop_machine.
> > 
> > This patch also converts cpu hotplug to the new interface since it
> > is special: cpu_down calls __stop_machine instead of stop_machine.
> > However the kstop threads will only be created when stop_machine
> > gets called.
> > 
> > Changing the code so that the threads would be created automatically
> > on __stop_machine is currently not possible: when __stop_machine gets
> > called we hold cpu_add_remove_lock, which is the same lock that
> > create_rt_workqueue would take. So the workqueue needs to be created
> > before the cpu hotplug code locks cpu_add_remove_lock.
> 
> In http://bugzilla.kernel.org/show_bug.cgi?id=12492, Thomas (cc'ed
> here) reports
> 
>   Commit 9ea09af3bd3090e8349ca2899ca2011bd94cda85 introduced a
>   regression that caused the kernel to fail to suspend.  The 'sleeping'
>   LED on the laptop just keeps blinking and the laptop never shuts
>   down.  I think this was eventually fixed because with 2.6.29-rc1 and
>   -rc2 the laptop suspends fine, but fails to resume.  When I try to
>   resume, all I see is a blinking cursor in the top left corner of the
>   screen.
> 
>   I'm using acpi_sleep=s3_bios,s3_mode, suspending using a script
>   that does: echo mem > /sys/power/state.
> 

hm.  Re-reading this, it seems to be saying that
9ea09af3bd3090e8349ca2899ca2011bd94cda85 might be innocent, and that
some other patch might have caused the resume regression?

Re: [Bug 12492] Re: [patch 1/2] stop_machine: introduce stop_machine_create/destroy.

[Heiko Carstens]

On Tue, 27 Jan 2009 01:52:02 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 27 Jan 2009 00:04:32 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:
> >   Commit 9ea09af3bd3090e8349ca2899ca2011bd94cda85 introduced a
> >   regression that caused the kernel to fail to suspend.  The 'sleeping'
> >   LED on the laptop just keeps blinking and the laptop never shuts
> >   down.  I think this was eventually fixed because with 2.6.29-rc1 and
> >   -rc2 the laptop suspends fine, but fails to resume.  When I try to
> >   resume, all I see is a blinking cursor in the top left corner of the
> >   screen.
> > 
> >   I'm using acpi_sleep=s3_bios,s3_mode, suspending using a script
> >   that does: echo mem > /sys/power/state.
> 
> hm.  Re-reading this, it seems to be saying that
> 9ea09af3bd3090e8349ca2899ca2011bd94cda85 might be innocent, and that
> some other patch might have caused the resume regression?

Yes, I already updated the bugzilla entry at kernel.org:

The bug introduced with the introduction of stop_machine_create/destroy
was fixed with a0e280e0f33f6c859a235fb69a875ed8f3420388
"stop_machine/cpu hotplug: fix disable_nonboot_cpus".
Must be something else.

[patch 2/2] module: convert to stop_machine_create/destroy.

[Heiko Carstens]

[-- Attachment #1: 002_module.diff --]
[-- Type: text/plain, Size: 1967 bytes --]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

The module code relies on a non-failing stop_machine call. So we create
the kstop threads in advance and with that make sure the call won't fail.

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 kernel/module.c |   23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Index: linux-2.6/kernel/module.c
===================================================================
--- linux-2.6.orig/kernel/module.c
+++ linux-2.6/kernel/module.c
@@ -757,8 +757,16 @@ sys_delete_module(const char __user *nam
 		return -EFAULT;
 	name[MODULE_NAME_LEN-1] = '\0';
 
-	if (mutex_lock_interruptible(&module_mutex) != 0)
-		return -EINTR;
+	/* Create stop_machine threads since free_module relies on
+	 * a non-failing stop_machine call. */
+	ret = stop_machine_create();
+	if (ret)
+		return ret;
+
+	if (mutex_lock_interruptible(&module_mutex) != 0) {
+		ret = -EINTR;
+		goto out_stop;
+	}
 
 	mod = find_module(name);
 	if (!mod) {
@@ -817,6 +825,8 @@ sys_delete_module(const char __user *nam
 
  out:
 	mutex_unlock(&module_mutex);
+out_stop:
+	stop_machine_destroy();
 	return ret;
 }
 
@@ -1865,6 +1875,13 @@ static noinline struct module *load_modu
 	/* vmalloc barfs on "unusual" numbers.  Check here */
 	if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL)
 		return ERR_PTR(-ENOMEM);
+
+	/* Create stop_machine threads since the error path relies on
+	 * a non-failing stop_machine call. */
+	err = stop_machine_create();
+	if (err)
+		goto free_hdr;
+
 	if (copy_from_user(hdr, umod, len) != 0) {
 		err = -EFAULT;
 		goto free_hdr;
@@ -2257,6 +2274,7 @@ static noinline struct module *load_modu
 	/* Get rid of temporary copy */
 	vfree(hdr);
 
+	stop_machine_destroy();
 	/* Done! */
 	return mod;
 
@@ -2279,6 +2297,7 @@ static noinline struct module *load_modu
 	kfree(args);
  free_hdr:
 	vfree(hdr);
+	stop_machine_destroy();
 	return ERR_PTR(err);
 
  truncated:

-- 

Re: [patch 0/2] stop_machine: create kstop threads only when needed

[Rusty Russell]

On Monday 22 December 2008 22:06:29 Heiko Carstens wrote:
> Introduce stop_machine_create/destroy. With this interface subsystems
> that need a non-failing stop_machine environment can create the
> stop_machine threads before actually calling stop_machine.
> When the threads aren't needed anymore they can be killed with
> stop_machine_destroy again.

Thanks, this is straightforward and effective.

I've applied both patches,
Rusty.