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) {
_
"Anil" <[email protected]> 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" <[email protected]>
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 <[email protected]>
---
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();
}
_
On Sat, 2004-06-05 at 08:30, Andrew Morton wrote:
> "Anil" <[email protected]> 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
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 <[email protected]>
---
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:[email protected]]
>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" <[email protected]> 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
>
>
>