LKML Archive on
help / color / mirror / Atom feed
From: David Howells <>
To: David Miller <>
Subject: Re: cmpxchg() in kernel/workqueue.c breaks things
Date: Thu, 07 Dec 2006 13:24:27 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

[adding linux-arch to the CC list]

David Miller <> wrote:

> David, you have to fix the locking scheme used in kernel/workqueue.c,
> you absolutely cannot assume that cmpxchg() is available on all
> platforms.  This breaks the build on the platforms that don't
> have such an instruction, and no it cannot emulated.

Yeah, I've figured that one out.  Also, having considered things last night, I
think the use of cmpxchg() is unnecessary.

I was trying to handle against two possibilities:

 (1) The pending flag may have been unset or may be cleared.  However, given
     where it's called, the pending flag is _always_ set.  I don't think it
     can be unset whilst we're in set_wq_data().

     Once the work is enqueued to be actually run, the only way off the queue
     is for it to be actually run.

     If it's a delayed work item, then the bit can't be cleared by the timer
     because we haven't started the timer yet.  Also, the pending bit can't be
     cleared by cancelling the delayed work _until_ the work item has had its
     timer started.

 (2) The workqueue pointer might change.  This can only happen in two cases:

     (a) The work item has just been queued to actually run, and so we're
         protected by the appropriate workqueue spinlock.

     (b) A delayed work item is being queued, and so the timer hasn't been
     	 started yet, and so no one else knows about the work item or can
     	 access it (the pending bit protects us).

     Besides, set_wq_data() _sets_ the workqueue pointer unconditionally, so
     it can be assigned instead.

So, I think replacing the set_wq_data() with a straight assignment would be
okay in most cases.  The problem is where we end up tangling with
test_and_set_bit() emulated using spinlocks, and even then it's not a problem
_provided_ test_and_set_bit() doesn't attempt to modify the word if the bit
was set.


  parent reply	other threads:[~2006-12-07 13:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-07  8:09 David Miller
2006-12-07  8:23 ` Al Viro
2006-12-07 11:03 ` David Howells
2006-12-07 11:28   ` Andrew Morton
2006-12-07 11:46   ` David Howells
2006-12-07 16:35     ` Andrew Morton
2006-12-07 13:24 ` David Howells [this message]
2006-12-07 14:53 ` Russell King

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: cmpxchg() in kernel/workqueue.c breaks things' \

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