[PATCH] speedup flush_workqueue for singlethread_workqueue

[PATCH] speedup flush_workqueue for singlethread_workqueue

[Anil]

	In the flush_workqueue function for a single_threaded_worqueue case the code seemed to loop the same cpu_workqueue_struct
for each_online_cpu's. The attached patch checks this condition and bails out of for loop there by speeding up the flush_workqueue
for a singlethreaded_workqueue.

Please apply.

Thanks,
-anil

---

Name: speedup_flush_workqueue_for_single_thread
Status: Test Passed

 linux-2.6.7-rc2-mm2-root/kernel/workqueue.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff -puN kernel/workqueue.c~flush_work_queue_fix kernel/workqueue.c
--- linux-2.6.7-rc2-mm2/kernel/workqueue.c~flush_work_queue_fix	2004-06-04 21:38:49.848195790 -0700
+++ linux-2.6.7-rc2-mm2-root/kernel/workqueue.c	2004-06-04 21:42:50.013357817 -0700
@@ -236,6 +236,7 @@ void fastcall flush_workqueue(struct wor
 {
 	struct cpu_workqueue_struct *cwq;
 	int cpu;
+	int run_once = 0;
 
 	might_sleep();
 
@@ -244,9 +245,12 @@ void fastcall flush_workqueue(struct wor
 		DEFINE_WAIT(wait);
 		long sequence_needed;
 
-		if (is_single_threaded(wq))
+		if (is_single_threaded(wq)) {
+			if (run_once)
+				break;
 			cwq = wq->cpu_wq + 0; /* Always use cpu 0's area. */
-		else
+			run_once = 1;
+		} else
 			cwq = wq->cpu_wq + cpu;
 
 		if (cwq->thread == current) {

_

Re: [PATCH] speedup flush_workqueue for singlethread_workqueue

[Andrew Morton]

"Anil" <anil.s.keshavamurthy@intel.com> wrote:
>
> 	In the flush_workqueue function for a single_threaded_worqueue case the code seemed to loop the same cpu_workqueue_struct
> for each_online_cpu's. The attached patch checks this condition and bails out of for loop there by speeding up the flush_workqueue
> for a singlethreaded_workqueue.


OK, thanks.  I think it's a bit clearer to do it as below.  I haven't
tested it though.



From: "Anil" <anil.s.keshavamurthy@intel.com>

In flush_workqueue() for a single_threaded_worqueue case the code flushes the
same cpu_workqueue_struct for each online_cpu.

Change things so that we only perform the flush once in this case.

Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 25-akpm/kernel/workqueue.c |   66 +++++++++++++++++++++++----------------------
 1 files changed, 35 insertions(+), 31 deletions(-)

diff -puN kernel/workqueue.c~speedup-flush_workqueue-for-singlethread_workqueue kernel/workqueue.c
--- 25/kernel/workqueue.c~speedup-flush_workqueue-for-singlethread_workqueue	Fri Jun  4 15:22:50 2004
+++ 25-akpm/kernel/workqueue.c	Fri Jun  4 15:27:24 2004
@@ -218,6 +218,33 @@ static int worker_thread(void *__cwq)
 	return 0;
 }
 
+static void flush_cpu_workqueue(struct cpu_workqueue_struct *cwq)
+{
+	if (cwq->thread == current) {
+		/*
+		 * Probably keventd trying to flush its own queue. So simply run
+		 * it by hand rather than deadlocking.
+		 */
+		run_workqueue(cwq);
+	} else {
+		DEFINE_WAIT(wait);
+		long sequence_needed;
+
+		spin_lock_irq(&cwq->lock);
+		sequence_needed = cwq->insert_sequence;
+
+		while (sequence_needed - cwq->remove_sequence > 0) {
+			prepare_to_wait(&cwq->work_done, &wait,
+					TASK_UNINTERRUPTIBLE);
+			spin_unlock_irq(&cwq->lock);
+			schedule();
+			spin_lock_irq(&cwq->lock);
+		}
+		finish_wait(&cwq->work_done, &wait);
+		spin_unlock_irq(&cwq->lock);
+	}
+}
+
 /*
  * flush_workqueue - ensure that any scheduled work has run to completion.
  *
@@ -234,42 +261,19 @@ static int worker_thread(void *__cwq)
  */
 void fastcall flush_workqueue(struct workqueue_struct *wq)
 {
-	struct cpu_workqueue_struct *cwq;
-	int cpu;
-
 	might_sleep();
-
 	lock_cpu_hotplug();
-	for_each_online_cpu(cpu) {
-		DEFINE_WAIT(wait);
-		long sequence_needed;
 
-		if (is_single_threaded(wq))
-			cwq = wq->cpu_wq + 0; /* Always use cpu 0's area. */
-		else
-			cwq = wq->cpu_wq + cpu;
-
-		if (cwq->thread == current) {
-			/*
-			 * Probably keventd trying to flush its own queue.
-			 * So simply run it by hand rather than deadlocking.
-			 */
-			run_workqueue(cwq);
-			continue;
-		}
-		spin_lock_irq(&cwq->lock);
-		sequence_needed = cwq->insert_sequence;
+	if (is_single_threaded(wq)) {
+		/* Always use cpu 0's area. */
+		flush_cpu_workqueue(wq->cpu_wq + 0);
+	} else {
+		int cpu;
 
-		while (sequence_needed - cwq->remove_sequence > 0) {
-			prepare_to_wait(&cwq->work_done, &wait,
-					TASK_UNINTERRUPTIBLE);
-			spin_unlock_irq(&cwq->lock);
-			schedule();
-			spin_lock_irq(&cwq->lock);
-		}
-		finish_wait(&cwq->work_done, &wait);
-		spin_unlock_irq(&cwq->lock);
+		for_each_online_cpu(cpu)
+			flush_cpu_workqueue(wq->cpu_wq + cpu);
 	}
+
 	unlock_cpu_hotplug();
 }
 
_

Re: [PATCH] speedup flush_workqueue for singlethread_workqueue

[Rusty Russell]

On Sat, 2004-06-05 at 08:30, Andrew Morton wrote:
> "Anil" <anil.s.keshavamurthy@intel.com> wrote:
> >
> > 	In the flush_workqueue function for a single_threaded_worqueue case the code seemed to loop the same cpu_workqueue_struct
> > for each_online_cpu's. The attached patch checks this condition and bails out of for loop there by speeding up the flush_workqueue
> > for a singlethreaded_workqueue.
> 
> 
> OK, thanks.  I think it's a bit clearer to do it as below.  I haven't
> tested it though.

Me neither, but agree.

Rusty.
-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell

RE: [PATCH] speedup flush_workqueue for singlethread_workqueue

[Anil]

Hi Andrew,
	In the flush_workqueue, can we move lock_cpu_hotplug()/unlock_cpu_hotplug only for _non_single_threaded_ case as shown below
as these locks are not required for single_threaded_workqueue.

Thanks,
Anil	

The attached patch applies on top of your changes.
Signed-off-by: Anil Keshavamurthy <anil.s.keshavamurthy@intel.com>
---

 linux-2.6.7-rc2-mm2-root/kernel/workqueue.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff -puN kernel/workqueue.c~andrew2 kernel/workqueue.c
--- linux-2.6.7-rc2-mm2/kernel/workqueue.c~andrew2	2004-06-07 18:45:20.332139681 -0700
+++ linux-2.6.7-rc2-mm2-root/kernel/workqueue.c	2004-06-07 18:46:09.560680511 -0700
@@ -262,7 +262,6 @@ static void flush_cpu_workqueue(struct c
 void fastcall flush_workqueue(struct workqueue_struct *wq)
 {
  	might_sleep();
- 	lock_cpu_hotplug();
 
 	if (is_single_threaded(wq)) {
 		/* Always use cpu 0's area. */
@@ -270,11 +269,12 @@ void fastcall flush_workqueue(struct wor
 	} else {
 		int cpu;
  
+ 		lock_cpu_hotplug();
 		for_each_online_cpu(cpu)
 			flush_cpu_workqueue(wq->cpu_wq + cpu);
+ 		unlock_cpu_hotplug();
  	}
 
- 	unlock_cpu_hotplug();
 }
 
 static struct task_struct *create_workqueue_thread(struct workqueue_struct *wq,

_

 	

>-----Original Message-----
>From: Rusty Russell [mailto:rusty@rustcorp.com.au] 
>Sent: Saturday, June 05, 2004 7:11 PM
>To: Andrew Morton
>Cc: Keshavamurthy, Anil S; lkml - Kernel Mailing List
>Subject: Re: [PATCH] speedup flush_workqueue for singlethread_workqueue
>
>On Sat, 2004-06-05 at 08:30, Andrew Morton wrote:
>> "Anil" <anil.s.keshavamurthy@intel.com> wrote:
>> >
>> > 	In the flush_workqueue function for a 
>single_threaded_worqueue case 
>> > the code seemed to loop the same cpu_workqueue_struct for 
>> > each_online_cpu's. The attached patch checks this 
>condition and bails out of for loop there by speeding up the 
>flush_workqueue for a singlethreaded_workqueue.
>> 
>> 
>> OK, thanks.  I think it's a bit clearer to do it as below.  
>I haven't 
>> tested it though.
>
>Me neither, but agree.
>
>Rusty.
>--
>Anyone who quotes me in their signature is an idiot -- Rusty Russell
>
>
>