On 01/05, Srivatsa Vaddagiri wrote:
>
> On Thu, Jan 04, 2007 at 10:31:07AM -0800, Andrew Morton wrote:
> > But before we do much more of this we should have a wrapper. Umm
> >
> > static inline void block_cpu_hotplug(void)
> > {
> > preempt_disable();
> > }
>
> Nack.
>
> This will only block cpu down, not cpu_up and hence is a misnomer. I would be
> vary of ignoring cpu_up events totally in writing hotplug safe code.
How about block_cpu_down() ?
These cpu-hotplug races delayed the last workqueue patch I have in my queue.
flush_workqueue() misses an important optimization: we don't need to insert
a barrier and have an extra wake_up + wait_for_completion when cwq has no
pending works. But we need ->current_work (introduced in the next patch) to
implement this correctly.
I'll re-send the patch below later, when we finish with the bug you pointed
out, but it would be nice if you can take a look now.
Oleg.
--- mm-6.20-rc2/kernel/workqueue.c~4_speedup 2006-12-30 18:09:07.000000000
+0300
+++ mm-6.20-rc2/kernel/workqueue.c 2007-01-05 16:32:45.000000000 +0300
@@ -405,12 +405,15 @@ static void wq_barrier_func(struct work_
complete(&barr->done);
}
-static inline void init_wq_barrier(struct wq_barrier *barr)
+static void insert_wq_barrier(struct cpu_workqueue_struct *cwq,
+ struct wq_barrier *barr, int tail)
{
INIT_WORK(&barr->work, wq_barrier_func);
__set_bit(WORK_STRUCT_PENDING, work_data_bits(&barr->work));
init_completion(&barr->done);
+
+ insert_work(cwq, &barr->work, tail);
}
static void flush_cpu_workqueue(struct cpu_workqueue_struct *cwq)
@@ -425,13 +428,20 @@ static void flush_cpu_workqueue(struct c
mutex_lock(&workqueue_mutex);
} else {
struct wq_barrier barr;
+ int active = 0;
- init_wq_barrier(&barr);
- __queue_work(cwq, &barr.work);
+ spin_lock_irq(&cwq->lock);
+ if (!list_empty(&cwq->worklist) || cwq->current_work != NULL) {
+ insert_wq_barrier(cwq, &barr, 1);
+ active = 1;
+ }
+ spin_unlock_irq(&cwq->lock);
- mutex_unlock(&workqueue_mutex);
- wait_for_completion(&barr.done);
- mutex_lock(&workqueue_mutex);
+ if (active) {
+ mutex_unlock(&workqueue_mutex);
+ wait_for_completion(&barr.done);
+ mutex_lock(&workqueue_mutex);
+ }
}
}
@@ -478,8 +488,7 @@ static void wait_on_work(struct cpu_work
spin_lock_irq(&cwq->lock);
if (unlikely(cwq->current_work == work)) {
- init_wq_barrier(&barr);
- insert_work(cwq, &barr.work, 0);
+ insert_wq_barrier(cwq, &barr, 0);
running = 1;
}
spin_unlock_irq(&cwq->lock);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
