From: Peter Zijlstra <peterz@infradead.org>
To: Olivier Galibert <galibert@pobox.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
tglx@linutronix.de,
Linus Torvalds <torvalds@linux-foundation.org>,
mingo@elte.hu
Subject: Re: [PATCH] spinlock: __raw_spin_is_locked() should return true for UP
Date: Wed, 19 Aug 2009 11:38:21 +0200 [thread overview]
Message-ID: <1250674701.7583.333.camel@twins> (raw)
In-Reply-To: <20090819093145.GA53298@dspnet.fr.eu.org>
On Wed, 2009-08-19 at 11:31 +0200, Olivier Galibert wrote:
> On Tue, Aug 18, 2009 at 07:40:16PM -0700, Linus Torvalds wrote:
> >
> >
> > On Tue, 18 Aug 2009, Kumar Gala wrote:
> > >
> > > I agree its a little too easy to abuse spin_is_locked. However we should be
> > > consistent between spin_is_locked on UP between with and without
> > > CONFIG_DEBUG_SPINLOCK enabled.
> >
> > No we shouldn't.
> >
> > With CONFIG_DEBUG_SPINLOCK, you have an actual lock variable for debugging
> > purposes, so spin_is_locked() can clearly return a _valid_ answer, and
> > should do so.
> >
> > Without DEBUG_SPINLOCK, there isn't any answer to return.
> >
> > So there's no way we can or should be consistent. In one case an answer
> > exists, in another one the answer is meaningless and doesn't exist.
>
> I always thought behaviour should be consistent between code with
> debugging on and code without. Otherwise you may end up with cases of
> "it starts working when I turn on debugging" which are a pain to fix.
> Has something changed?
>
> Or in other words, do you think lockdep should try solving deadlocks
> instead of just reporting them for instance?
The point is spin_is_locked() is a broken interface in that respect. Its
plain impossible to give the right answer.
Suppose there's code doing:
/*
* Ensure we don't have foo lock taken, because that would cause
* lock inversion under bar lock.
*/
BUG_ON(spin_is_locked(&foo));
spin_lock(&bar);
and other code doing:
/*
* Ensure we've got foo locked because it protects bar
*/
BUG_ON(!spin_is_locked(&foo));
bar = fancy;
What value should you return when locks don't exist (which is the case
for UP)?
There simply is no right answer other than: don't use spin_is_locked().
next prev parent reply other threads:[~2009-08-19 9:38 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-18 22:42 [PATCH] spinlock: __raw_spin_is_locked() should return true for UP Kumar Gala
2009-08-18 23:20 ` Linus Torvalds
2009-08-18 23:36 ` Steven Rostedt
2009-08-18 23:52 ` Linus Torvalds
2009-08-19 0:07 ` Steven Rostedt
2009-08-19 1:17 ` Kumar Gala
2009-08-19 2:40 ` Linus Torvalds
2009-08-19 9:31 ` Olivier Galibert
2009-08-19 9:38 ` Peter Zijlstra [this message]
2009-08-19 18:50 ` Scott Wood
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=1250674701.7583.333.camel@twins \
--to=peterz@infradead.org \
--cc=galibert@pobox.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=mingo@elte.hu \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
/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 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).