On Thu, 7 Dec 2006 08:49:02 -0800 (PST) Linus Torvalds <[EMAIL PROTECTED]> wrote:
> > > On Wed, 6 Dec 2006, Andrew Morton wrote: > > > > But this will return to the caller if the callback is presently running on > > a different CPU. The whole point here is to be able to reliably kill off > > the pending work so that the caller can free resources. > > I mentioned that in one of the emails. > > We do not _have_ the information to not do that. It simply doesn't exist. > We can either wait for _all_ pending entries on the to complete (by > waiting for the workqueue counters for added/removed to be the same), or > we can have the race. Well we'll need to add the infrastructure to be able to do this, won't we? The whole point of calling flush_scheduled_work() (which we're trying to replace/simplify) is to block the caller until it is safe to release resources. It might be a challenge to do this without adding more stuff to work_struct though. umm.. Putting a work_struct* into struct cpu_workqueue_struct and then doing appropriate things with cpu_workqueue_struct.lock might work. <hack, hack> Something along these lines. The keventd-calls-flush_work() case rather sucks though. diff -puN kernel/workqueue.c~a kernel/workqueue.c --- a/kernel/workqueue.c~a +++ a/kernel/workqueue.c @@ -323,6 +323,7 @@ static void run_workqueue(struct cpu_wor work_func_t f = work->func; list_del_init(cwq->worklist.next); + cwq->current_work = work; spin_unlock_irqrestore(&cwq->lock, flags); BUG_ON(get_wq_data(work) != cwq); @@ -342,6 +343,7 @@ static void run_workqueue(struct cpu_wor } spin_lock_irqsave(&cwq->lock, flags); + cwq->current_work = NULL; cwq->remove_sequence++; wake_up(&cwq->work_done); } @@ -425,6 +427,64 @@ static void flush_cpu_workqueue(struct c } } +static void wait_on_work(struct cpu_workqueue_struct *cwq, + struct work_struct *work) +{ + DEFINE_WAIT(wait); + + spin_lock_irq(&cwq->lock); + while (cwq->current_work == work) { + 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); +} + +static void flush_one_work(struct cpu_workqueue_struct *cwq, + struct work_struct *work) +{ + spin_lock_irq(&cwq->lock); + if (test_and_clear_bit(WORK_STRUCT_PENDING, &work->management)) { + list_del_init(&work->entry); + spin_unlock_irq(&cwq->lock); + return; + } + spin_unlock_irq(&cwq->lock); + + /* It's running, or it has completed */ + + if (cwq->thread == current) { + /* This stinks */ + /* + * Probably keventd trying to flush its own queue. So simply run + * it by hand rather than deadlocking. + */ + run_workqueue(cwq); + } else { + wait_on_work(cwq, work); + } +} + +void flush_work(struct work_struct *work) +{ + might_sleep(); + + if (is_single_threaded(wq)) { + /* Always use first cpu's area. */ + flush_one_work(per_cpu_ptr(wq->cpu_wq, singlethread_cpu), work); + } else { + int cpu; + + mutex_lock(&workqueue_mutex); + for_each_online_cpu(cpu) + flush_one_work(per_cpu_ptr(wq->cpu_wq, cpu), work); + mutex_unlock(&workqueue_mutex); + } +} + /** * flush_workqueue - ensure that any scheduled work has run to completion. * @wq: workqueue to flush _ - 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/