LKML Archive on
help / color / mirror / Atom feed
From: Oleg Nesterov <>
To: Andrew Morton <>,
	David Howells <>,
	Dmitry Torokhov <>,
	Jiri Pirko <>,
	"Paul E. McKenney" <>,
	Peter Zijlstra <>
Subject: [RFC,PATCH] workqueues: turn queue_work() into the "barrier" for work->func()
Date: Tue, 11 Nov 2008 21:33:24 +0100	[thread overview]
Message-ID: <> (raw)

To clarify, I will be happy with the "no, we don't need this" comment.

But let's suppose we have

	int VAR;

	void work_func(struct work_struct *work)
		if (VAR)

and we are doing

	VAR = 1;

I think the caller of queue_work() has all rights to expect that
the next invocation of work_func() must see "VAR == 1", but this
is not true if the work is already pending.


			clear_bit(WORK_STRUCT_PENDING) // no mb()
		call work_func()
			if (VAR)

it is possible that CPU reads VAR before before it clears _PENDING,
and queue_work() "infiltrates" in between and fails. So we can miss
an event.

I don't know if we really have such a code in kernel, and even if
we have perhaps we should fix it and do not touch workqueues. But
perhaps the current behaviour is a bit too subtle in this respect.

For example, atkbd_event_work() happens to work correctly, but only
because it does mb() implicitly.

The patch merely adds mb() after work_clear_pending(work), another
side already has the mb semantics implied by test_and_set_bit().
>From now queue_work() always acts as a barrier for work->func().

Signed-off-by: Oleg Nesterov <>

--- K-28/kernel/workqueue.c~WQ_MB	2008-11-06 19:11:02.000000000 +0100
+++ K-28/kernel/workqueue.c	2008-11-11 21:06:20.000000000 +0100
@@ -291,6 +291,12 @@ static void run_workqueue(struct cpu_wor
 		BUG_ON(get_wq_data(work) != cwq);
+		/*
+		 * Ensure that either the concurrent queue_work() succeeds,
+		 * or work->func() sees all the preceding memory changes.
+		 */
+		smp_mb__after_clear_bit();

             reply	other threads:[~2008-11-11 19:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-11 20:33 Oleg Nesterov [this message]
2008-11-11 22:46 ` David Howells
2008-11-12 11:58   ` Oleg Nesterov

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:

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

  git send-email \ \ \ \ \ \ \ \ \ \
    --subject='Re: [RFC,PATCH] workqueues: turn queue_work() into the "barrier" for work->func()' \

* 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).