Messages in this thread | | | Subject | Re: [PATCH] spinlock: __raw_spin_is_locked() should return true for UP | From | Peter Zijlstra <> | Date | Wed, 19 Aug 2009 11:38:21 +0200 |
| |
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().
| |