Re: [PATCH] Export current_is_keventd() for libphy

Andrew Morton
2006-12-07T17:53:36Z

On Thu, 7 Dec 2006 08:49:02 -0800 (PST)
Linus Torvalds <torvalds@osdl.org> 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
_