[patch 1/2] workqueue: mark init_workqueues() as early_initcall()

[patch 1/2] workqueue: mark init_workqueues() as early_initcall()

[Suresh Siddha]

[-- Attachment #1: init_workqueues_before_smp_init.patch --]
[-- Type: text/plain, Size: 2471 bytes --]

Mark init_workqueues() as early_initcall() and thus it will be initialized
before smp bringup. init_workqueues() registers for the hotcpu notifier
and thus it should cope with the processors that are brought online after
the workqueues are initialized.

x86 smp bringup code uses workqueues and uses a workaround for the
cold boot process (as the workqueues are initialized post smp_init()).
Marking init_workqueues() as early_initcall() will pave the way for
cleaning up this code.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 include/linux/workqueue.h |    1 -
 init/main.c               |    2 --
 kernel/workqueue.c        |    4 +++-
 3 files changed, 3 insertions(+), 4 deletions(-)

Index: tip/init/main.c
===================================================================
--- tip.orig/init/main.c
+++ tip/init/main.c
@@ -32,7 +32,6 @@
 #include <linux/start_kernel.h>
 #include <linux/security.h>
 #include <linux/smp.h>
-#include <linux/workqueue.h>
 #include <linux/profile.h>
 #include <linux/rcupdate.h>
 #include <linux/moduleparam.h>
@@ -788,7 +787,6 @@ static void __init do_initcalls(void)
  */
 static void __init do_basic_setup(void)
 {
-	init_workqueues();
 	cpuset_init_smp();
 	usermodehelper_init();
 	init_tmpfs();
Index: tip/include/linux/workqueue.h
===================================================================
--- tip.orig/include/linux/workqueue.h
+++ tip/include/linux/workqueue.h
@@ -234,7 +234,6 @@ extern int schedule_on_each_cpu(work_fun
 extern int current_is_keventd(void);
 extern int keventd_up(void);
 
-extern void init_workqueues(void);
 int execute_in_process_context(work_func_t fn, struct execute_work *);
 
 extern int flush_work(struct work_struct *work);
Index: tip/kernel/workqueue.c
===================================================================
--- tip.orig/kernel/workqueue.c
+++ tip/kernel/workqueue.c
@@ -1216,7 +1216,7 @@ long work_on_cpu(unsigned int cpu, long 
 EXPORT_SYMBOL_GPL(work_on_cpu);
 #endif /* CONFIG_SMP */
 
-void __init init_workqueues(void)
+static int __init init_workqueues(void)
 {
 	alloc_cpumask_var(&cpu_populated_map, GFP_KERNEL);
 
@@ -1226,4 +1226,6 @@ void __init init_workqueues(void)
 	hotcpu_notifier(workqueue_cpu_callback, 0);
 	keventd_wq = create_workqueue("events");
 	BUG_ON(!keventd_wq);
+	return 0;
 }
+early_initcall(init_workqueues);

[patch 2/2] x86, smp: use workqueues unconditionally during do_boot_cpu()

[Suresh Siddha]

[-- Attachment #1: x86_smpboot_cleanup.patch --]
[-- Type: text/plain, Size: 1083 bytes --]

Workqueues are now initialized as part of the early_initcall(). So
they are available for use during cold boot process aswell.

And also ACPI hotplug notifier no longer calls do_boot_cpu() in the keventd_wq
context.  So we can unconditionally use workqueues in do_boot_cpu()

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Zhang Rui <rui.zhang@intel.com>
---
 arch/x86/kernel/smpboot.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Index: tip/arch/x86/kernel/smpboot.c
===================================================================
--- tip.orig/arch/x86/kernel/smpboot.c
+++ tip/arch/x86/kernel/smpboot.c
@@ -735,12 +735,8 @@ static int __cpuinit do_boot_cpu(int api
 		goto do_rest;
 	}
 
-	if (!keventd_up() || current_is_keventd())
-		c_idle.work.func(&c_idle.work);
-	else {
-		schedule_work(&c_idle.work);
-		wait_for_completion(&c_idle.done);
-	}
+	schedule_work(&c_idle.work);
+	wait_for_completion(&c_idle.done);
 
 	if (IS_ERR(c_idle.idle)) {
 		printk("failed fork for CPU %d\n", cpu);

Re: [patch 1/2] workqueue: mark init_workqueues() as early_initcall()

[Andrew Morton]

On Fri, 30 Jul 2010 14:57:37 -0700
Suresh Siddha <suresh.b.siddha@intel.com> wrote:

> Mark init_workqueues() as early_initcall() and thus it will be initialized
> before smp bringup. init_workqueues() registers for the hotcpu notifier
> and thus it should cope with the processors that are brought online after
> the workqueues are initialized.
> 
> x86 smp bringup code uses workqueues and uses a workaround for the
> cold boot process (as the workqueues are initialized post smp_init()).
> Marking init_workqueues() as early_initcall() will pave the way for
> cleaning up this code.
> 

I sure hope this has been tested against linux-next. 
kernel/workqueue.c has been vastly changed and -tip doesn't know about
that.  linux-next should include -tip and is hence a better tree to
develop and test against.

AFAICT the main thing which needs checking is that the new
init_workqueues() doesn't do anything which requires that
sched_init_smp() has been executed.

The patch otherwise looks OK and killing that hack in the x86 code was
most merciful.

for_each_gcwq_cpu(), for_each_online_gcwq_cpu() and for_each_cwq_cpu()
make me cry.

Re: [patch 1/2] workqueue: mark init_workqueues() as early_initcall()

[Suresh Siddha]

On Fri, 2010-07-30 at 16:55 -0700, Andrew Morton wrote:
> On Fri, 30 Jul 2010 14:57:37 -0700
> Suresh Siddha <suresh.b.siddha@intel.com> wrote:
> 
> > Mark init_workqueues() as early_initcall() and thus it will be initialized
> > before smp bringup. init_workqueues() registers for the hotcpu notifier
> > and thus it should cope with the processors that are brought online after
> > the workqueues are initialized.
> > 
> > x86 smp bringup code uses workqueues and uses a workaround for the
> > cold boot process (as the workqueues are initialized post smp_init()).
> > Marking init_workqueues() as early_initcall() will pave the way for
> > cleaning up this code.
> > 
> 
> I sure hope this has been tested against linux-next. 
> kernel/workqueue.c has been vastly changed and -tip doesn't know about
> that.  linux-next should include -tip and is hence a better tree to
> develop and test against.
> 
> AFAICT the main thing which needs checking is that the new
> init_workqueues() doesn't do anything which requires that
> sched_init_smp() has been executed.
> 
> The patch otherwise looks OK and killing that hack in the x86 code was
> most merciful.
> 
> for_each_gcwq_cpu(), for_each_online_gcwq_cpu() and for_each_cwq_cpu()
> make me cry.

hmm.. too many changes in linux-next.

Yes, as far as these patches are concerned, they work against linux-next
aswell. Just posted -v2 which is on top of linux-next. ia64 also had the
same issue, addressed in -v2 aswell.

thanks,
suresh

Re: [patch 1/2] workqueue: mark init_workqueues() as early_initcall()

[Tejun Heo]

Hello,

On 07/31/2010 02:48 AM, Suresh Siddha wrote:
> hmm.. too many changes in linux-next.
> 
> Yes, as far as these patches are concerned, they work against linux-next
> aswell. Just posted -v2 which is on top of linux-next. ia64 also had the
> same issue, addressed in -v2 aswell.

I think it'll be best to route the changes through workqueue tree.
I'll audit init path and if everything looks okay will add the
workqueue patch to wq#for-next.

Thank you.

-- 
tejun

Re: [patch 1/2] workqueue: mark init_workqueues() as early_initcall()

[Tejun Heo]

Hello,

On 07/31/2010 01:55 AM, Andrew Morton wrote:
> On Fri, 30 Jul 2010 14:57:37 -0700
> for_each_gcwq_cpu(), for_each_online_gcwq_cpu() and for_each_cwq_cpu()
> make me cry.

Yeah, those are new additions for unbound workqueue support and I
should have added comments there.  Will do so.

Thanks.

-- 
tejun

[PATCH wq#for-next] workqueue: explain for_each_*cwq_cpu() iterators

[Tejun Heo]

for_each_*cwq_cpu() are similar to regular CPU iterators except that
it also considers the pseudo CPU number used for unbound workqueues.
Explain them.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 kernel/workqueue.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index e5cb7fa..1105c47 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -271,6 +271,19 @@ static inline int __next_wq_cpu(int cpu, const struct cpumask *mask,
 	return __next_gcwq_cpu(cpu, mask, !(wq->flags & WQ_UNBOUND) ? 1 : 2);
 }

+/*
+ * CPU iterators
+ *
+ * An extra gcwq is defined for an invalid cpu number
+ * (WORK_CPU_UNBOUND) to host workqueues which are not bound to any
+ * specific CPU.  The following iterators are similar to
+ * for_each_*_cpu() iterators but also considers the unbound gcwq.
+ *
+ * for_each_gcwq_cpu()		: possible CPUs + WORK_CPU_UNBOUND
+ * for_each_online_gcwq_cpu()	: online CPUs + WORK_CPU_UNBOUND
+ * for_each_cwq_cpu()		: possible CPUs for bound workqueues,
+ *				  WORK_CPU_UNBOUND for unbound workqueues
+ */
 #define for_each_gcwq_cpu(cpu)						\
 	for ((cpu) = __next_gcwq_cpu(-1, cpu_possible_mask, 3);		\
 	     (cpu) < WORK_CPU_NONE;					\
-- 
1.7.1

[PATCH] workqueue: mark init_workqueues() as early_initcall()

[Tejun Heo]

>From 6ee0578b4daaea01c96b172c6aacca43fd9807a6 Mon Sep 17 00:00:00 2001
From: Suresh Siddha <suresh.b.siddha@intel.com>
Date: Fri, 30 Jul 2010 14:57:37 -0700

Mark init_workqueues() as early_initcall() and thus it will be initialized
before smp bringup. init_workqueues() registers for the hotcpu notifier
and thus it should cope with the processors that are brought online after
the workqueues are initialized.

x86 smp bringup code uses workqueues and uses a workaround for the
cold boot process (as the workqueues are initialized post smp_init()).
Marking init_workqueues() as early_initcall() will pave the way for
cleaning up this code.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
Audited and tested.  It should be fine.  Patch applied and pushed out
to linux-next.  How shall we route the second patch?

Thanks.

 include/linux/workqueue.h |    1 -
 init/main.c               |    2 --
 kernel/workqueue.c        |    4 +++-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 5f76001..51dc9a7 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -327,7 +327,6 @@ extern int schedule_delayed_work_on(int cpu, struct delayed_work *work,
 extern int schedule_on_each_cpu(work_func_t func);
 extern int keventd_up(void);

-extern void init_workqueues(void);
 int execute_in_process_context(work_func_t fn, struct execute_work *);

 extern int flush_work(struct work_struct *work);
diff --git a/init/main.c b/init/main.c
index 3bdb152..5f2ec2c 100644
--- a/init/main.c
+++ b/init/main.c
@@ -32,7 +32,6 @@
 #include <linux/start_kernel.h>
 #include <linux/security.h>
 #include <linux/smp.h>
-#include <linux/workqueue.h>
 #include <linux/profile.h>
 #include <linux/rcupdate.h>
 #include <linux/moduleparam.h>
@@ -786,7 +785,6 @@ static void __init do_initcalls(void)
  */
 static void __init do_basic_setup(void)
 {
-	init_workqueues();
 	cpuset_init_smp();
 	usermodehelper_init();
 	init_tmpfs();
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 1105c47..e2eb351 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3507,7 +3507,7 @@ out_unlock:
 }
 #endif /* CONFIG_FREEZER */

-void __init init_workqueues(void)
+static int __init init_workqueues(void)
 {
 	unsigned int cpu;
 	int i;
@@ -3559,4 +3559,6 @@ void __init init_workqueues(void)
 	system_unbound_wq = alloc_workqueue("events_unbound", WQ_UNBOUND,
 					    WQ_UNBOUND_MAX_ACTIVE);
 	BUG_ON(!system_wq || !system_long_wq || !system_nrt_wq);
+	return 0;
 }
+early_initcall(init_workqueues);
-- 
1.7.1

Re: [PATCH] workqueue: mark init_workqueues() as early_initcall()

[Suresh Siddha]

On Sun, 2010-08-01 at 12:07 +0100, Tejun Heo wrote:
> Audited and tested.  It should be fine.  Patch applied and pushed out
> to linux-next.  How shall we route the second patch?
> 

Tejun, v2 version of the second patch had both x86 and ia64 changes.

http://marc.info/?l=linux-kernel&m=128053714303108&w=2

So I am ok if you carry this with the rest of your workqueue changes
that touch these same files.

thanks,
suresh

Re: [PATCH] workqueue: mark init_workqueues() as early_initcall()

[Tejun Heo]

On 08/02/2010 08:41 PM, Suresh Siddha wrote:
> On Sun, 2010-08-01 at 12:07 +0100, Tejun Heo wrote:
>> Audited and tested.  It should be fine.  Patch applied and pushed out
>> to linux-next.  How shall we route the second patch?
>>
> 
> Tejun, v2 version of the second patch had both x86 and ia64 changes.
> 
> http://marc.info/?l=linux-kernel&m=128053714303108&w=2
> 
> So I am ok if you carry this with the rest of your workqueue changes
> that touch these same files.

Andrew seemed to have already picked up the patch.  I've sent the pull
request with the first patch.

Thank you.

-- 
tejun