From: Andrew Morton <akpm@linux-foundation.org>
To: Valerie Aurora <vaurora@redhat.com>
Cc: npiggin@suse.de, jblunck@suse.de, linux-kernel@vger.kernel.org,
paulmck@us.ibm.com
Subject: Re: [PATCH] atomic: Fix _atomic_dec_and_lock() deadlock on UP
Date: Mon, 15 Jun 2009 12:31:44 -0700 [thread overview]
Message-ID: <20090615123144.fb0a2296.akpm@linux-foundation.org> (raw)
In-Reply-To: <20090615191223.GE352@shell>
On Mon, 15 Jun 2009 15:12:23 -0400
Valerie Aurora <vaurora@redhat.com> wrote:
> On Mon, Jun 15, 2009 at 11:45:43AM -0700, Andrew Morton wrote:
> > On Mon, 15 Jun 2009 14:11:13 -0400
> > Valerie Aurora <vaurora@redhat.com> wrote:
> >
> > > _atomic_dec_and_lock() can deadlock on UP with spinlock debugging
> > > enabled. Currently, on UP we unconditionally spin_lock() first, which
> > > calls __spin_lock_debug(), which takes the lock unconditionally even
> > > on UP. This will deadlock in situations in which we call
> > > atomic_dec_and_lock() knowing that the counter won't go to zero
> > > (because we hold another reference) and that we already hold the lock.
> > > Instead, we should use the SMP code path which only takes the lock if
> > > necessary.
> >
> > Yup, I have this queued for 2.6.31 as
> > atomic-only-take-lock-when-the-counter-drops-to-zero-on-up-as-well.patch,
> > with a different changelog:
> >
> > _atomic_dec_and_lock() should not unconditionally take the lock before
> > calling atomic_dec_and_test() in the UP case. For consistency reasons it
> > should behave exactly like in the SMP case.
> >
> > Besides that this works around the problem that with CONFIG_DEBUG_SPINLOCK
> > this spins in __spin_lock_debug() if the lock is already taken even if the
> > counter doesn't drop to 0.
> >
> > Signed-off-by: Jan Blunck <jblunck@suse.de>
> > Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Acked-by: Nick Piggin <npiggin@suse.de>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> >
> >
> > I can't remember why we decided that 2.6.30 doesn't need this.
>
> Great, last I heard the changelog was still a problem. Thanks,
>
<goes back and checks>
OK, I decided that we didn't need this in 2.6.30 or earlier because
Jan's union mount code is the only known triggerer of the problem.
However the patch is clearly a suitable thing for -stable. Opinions
are sought..
next prev parent reply other threads:[~2009-06-15 19:32 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-06-15 18:11 [PATCH] atomic: Fix _atomic_dec_and_lock() deadlock on UP Valerie Aurora
2009-06-15 18:45 ` Andrew Morton
2009-06-15 19:12 ` Valerie Aurora
2009-06-15 19:31 ` Andrew Morton [this message]
2009-06-15 18:56 ` Paul E. McKenney
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=20090615123144.fb0a2296.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=jblunck@suse.de \
--cc=linux-kernel@vger.kernel.org \
--cc=npiggin@suse.de \
--cc=paulmck@us.ibm.com \
--cc=vaurora@redhat.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.