LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: linux-kernel@vger.kernel.org
Cc: Daniel Walker <dwalker@mvista.com>,
	Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@elte.hu>,
	Thomas Gleixner <tglx@linutronix.de>,
	Gregory Haskins <ghaskins@novell.com>,
	Oleg Nesterov <oleg@tv-sign.ru>
Subject: [RFC/PATCH 6/5] rt: PI-workqueue: wait_on_work() fixup
Date: Tue, 23 Oct 2007 21:22:24 +0200	[thread overview]
Message-ID: <1193167344.5648.23.camel@lappy> (raw)
In-Reply-To: <20071023120357.782284000@chello.nl>

Subject: rt: PI-workqueue: wait_on_work() fixup

Oleg noticed that the new wait_on_work() barrier does not properly interact
with the nesting barrier.

The problem is that a wait_on_work() targeted at a worklet in a nested list
will complete too late.

Fix this by using a wait_queue instead.

[ will be folded into the previous patch on next posting ]

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 kernel/workqueue.c |   74 ++++++++++++++++++++---------------------------------
 1 file changed, 29 insertions(+), 45 deletions(-)

Index: linux-2.6/kernel/workqueue.c
===================================================================
--- linux-2.6.orig/kernel/workqueue.c
+++ linux-2.6/kernel/workqueue.c
@@ -33,10 +33,11 @@
 #include <linux/freezer.h>
 #include <linux/kallsyms.h>
 #include <linux/debug_locks.h>
+#include <linux/wait.h>
 
 #include <asm/uaccess.h>
 
-struct wq_full_barrier;
+struct wq_barrier;
 
 /*
  * The per-CPU workqueue (if single thread, we always use the first
@@ -55,7 +56,8 @@ struct cpu_workqueue_struct {
 
 	int run_depth;		/* Detect run_workqueue() recursion depth */
 
-	struct wq_full_barrier *barrier;
+	wait_queue_head_t work_done;
+	struct wq_barrier *barrier;
 } ____cacheline_aligned;
 
 /*
@@ -259,10 +261,10 @@ static void leak_check(void *func)
 	dump_stack();
 }
 
-struct wq_full_barrier {
+struct wq_barrier {
 	struct work_struct		work;
 	struct plist_head		worklist;
-	struct wq_full_barrier 		*prev_barrier;
+	struct wq_barrier 		*prev_barrier;
 	int				prev_prio;
 	int				waiter_prio;
 	struct cpu_workqueue_struct 	*cwq;
@@ -314,13 +316,13 @@ again:
 
 		spin_lock_irq(&cwq->lock);
 		cwq->current_work = NULL;
-
+		wake_up_all(&cwq->work_done);
 		if (unlikely(cwq->barrier))
 			worklist = &cwq->barrier->worklist;
 	}
 
 	if (unlikely(worklist != &cwq->worklist)) {
-		struct wq_full_barrier *barrier = cwq->barrier;
+		struct wq_barrier *barrier = cwq->barrier;
 
 		BUG_ON(!barrier);
 		cwq->barrier = barrier->prev_barrier;
@@ -369,32 +371,10 @@ static int worker_thread(void *__cwq)
 	return 0;
 }
 
-struct wq_barrier {
-	struct work_struct	work;
-	struct completion	done;
-};
-
 static void wq_barrier_func(struct work_struct *work)
 {
-	struct wq_barrier *barr = container_of(work, struct wq_barrier, work);
-	complete(&barr->done);
-}
-
-static void insert_wq_barrier(struct cpu_workqueue_struct *cwq,
-					struct wq_barrier *barr, int prio)
-{
-	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, prio, current->prio);
-}
-
-static void wq_full_barrier_func(struct work_struct *work)
-{
-	struct wq_full_barrier *barrier =
-		container_of(work, struct wq_full_barrier, work);
+	struct wq_barrier *barrier =
+		container_of(work, struct wq_barrier, work);
 	struct cpu_workqueue_struct *cwq = barrier->cwq;
 	int prio = MAX_PRIO;
 
@@ -409,10 +389,10 @@ static void wq_full_barrier_func(struct 
 	spin_unlock_irq(&cwq->lock);
 }
 
-static void insert_wq_full_barrier(struct cpu_workqueue_struct *cwq,
-		struct wq_full_barrier *barr)
+static void insert_wq_barrier(struct cpu_workqueue_struct *cwq,
+		struct wq_barrier *barr)
 {
-	INIT_WORK(&barr->work, wq_full_barrier_func);
+	INIT_WORK(&barr->work, wq_barrier_func);
 	__set_bit(WORK_STRUCT_PENDING, work_data_bits(&barr->work));
 
 	plist_head_init(&barr->worklist, NULL);
@@ -436,13 +416,13 @@ static int flush_cpu_workqueue(struct cp
 		run_workqueue(cwq);
 		active = 1;
 	} else {
-		struct wq_full_barrier barr;
+		struct wq_barrier barr;
 
 		active = 0;
 		spin_lock_irq(&cwq->lock);
 		if (!plist_head_empty(&cwq->worklist) ||
 			cwq->current_work != NULL) {
-			insert_wq_full_barrier(cwq, &barr);
+			insert_wq_barrier(cwq, &barr);
 			active = 1;
 		}
 		spin_unlock_irq(&cwq->lock);
@@ -518,21 +498,24 @@ static int try_to_grab_pending(struct wo
 	return ret;
 }
 
-static void wait_on_cpu_work(struct cpu_workqueue_struct *cwq,
-				struct work_struct *work)
+static inline
+int is_current_work(struct cpu_workqueue_struct *cwq, struct work_struct *work)
 {
-	struct wq_barrier barr;
-	int running = 0;
+	int ret;
 
 	spin_lock_irq(&cwq->lock);
-	if (unlikely(cwq->current_work == work)) {
-		insert_wq_barrier(cwq, &barr, -1);
-		running = 1;
-	}
+	ret = (cwq->current_work == work);
 	spin_unlock_irq(&cwq->lock);
 
-	if (unlikely(running))
-		wait_for_completion(&barr.done);
+	return ret;
+}
+
+static void wait_on_cpu_work(struct cpu_workqueue_struct *cwq,
+				struct work_struct *work)
+{
+	DEFINE_WAIT(wait);
+
+	wait_event(cwq->work_done, is_current_work(cwq, work));
 }
 
 static void wait_on_work(struct work_struct *work)
@@ -838,6 +821,7 @@ init_cpu_workqueue(struct workqueue_stru
 	plist_head_init(&cwq->worklist, NULL);
 	init_waitqueue_head(&cwq->more_work);
 	cwq->barrier = NULL;
+	init_waitqueue_head(&cwq->work_done);
 
 	return cwq;
 }



  parent reply	other threads:[~2007-10-23 19:23 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-23 12:03 [RFC/PATCH 0/5] rt: workqueue PI support -v2 Peter Zijlstra
2007-10-23 12:03 ` [RFC/PATCH 1/5] rt: rename rt_mutex_setprio to task_setprio Peter Zijlstra
2007-10-23 12:03 ` [RFC/PATCH 2/5] rt: list_splice2 Peter Zijlstra
2007-10-23 14:08   ` Steven Rostedt
2007-10-23 12:04 ` [RFC/PATCH 3/5] rt: plist_head_splice Peter Zijlstra
2007-10-23 15:10   ` Steven Rostedt
2007-10-23 16:26     ` Peter Zijlstra
2007-10-23 17:45     ` Peter Zijlstra
2007-10-23 12:04 ` [RFC/PATCH 4/5] rt: PI-workqueue support Peter Zijlstra
2007-10-23 12:04 ` [RFC/PATCH 5/5] rt: PI-workqueue: fix barriers Peter Zijlstra
2007-10-23 19:22 ` Peter Zijlstra [this message]
2007-10-23 19:22 ` [RFC/PATCH 7/5] rt: PI-workqueue: propagate prio for delayed work Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1193167344.5648.23.camel@lappy \
    --to=a.p.zijlstra@chello.nl \
    --cc=dwalker@mvista.com \
    --cc=ghaskins@novell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=oleg@tv-sign.ru \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --subject='Re: [RFC/PATCH 6/5] rt: PI-workqueue: wait_on_work() fixup' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).