Re: [PATCH] Export current_is_keventd() for libphy

[Andrew Morton <akpm@osdl.org>]

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
_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/