| Message ID | 20160818173707.GA1240@lst.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thu, Aug 18, 2016 at 07:37:07PM +0200, Christoph Hellwig wrote: > On Thu, Aug 11, 2016 at 11:54:44PM +0200, Peter Zijlstra wrote: > > Sure, find below. Not been near a compiler. > > It has now. Below are the additions I need, and things seem to be > passing fine with that. > > Note that to fit the existing XFS lock asserts I'm using > _lockdep_is_held directly instead of lockdep_assert_held_exclusive. > All the exports are there, but I'm just trying to make sure you're > not going to shout at me for that later :) Hurm, if you're going to directly use that maybe we should pick a better name ;-) Also, be sure to check the debug_locks variable, if that's cleared the result of _lockdep_is_held() isn't reliable -- we stop tracking lock state when there's an error. Thanks!
On Fri, Aug 19, 2016 at 03:27:36PM +0200, Peter Zijlstra wrote: > Hurm, if you're going to directly use that maybe we should pick a better > name ;-) Fine with that. > Also, be sure to check the debug_locks variable, if that's cleared the > result of _lockdep_is_held() isn't reliable -- we stop tracking lock > state when there's an error. I already do. But I'm wondering if we can't simply move the debug_locks check into lockdep_is_held? It's already used directly in a few places, and that would also solve the whole naming issue.
On Sat, Aug 20, 2016 at 08:37:23AM +0200, Christoph Hellwig wrote: > On Fri, Aug 19, 2016 at 03:27:36PM +0200, Peter Zijlstra wrote: > > Hurm, if you're going to directly use that maybe we should pick a better > > name ;-) > > Fine with that. > > > Also, be sure to check the debug_locks variable, if that's cleared the > > result of _lockdep_is_held() isn't reliable -- we stop tracking lock > > state when there's an error. > > I already do. But I'm wondering if we can't simply move the > debug_locks check into lockdep_is_held? It's already used directly > in a few places, and that would also solve the whole naming issue. Reason I didn't do that initially was that I used lock_is_held() for both positive and negative tests (ie. assert a lock is held and a lock is not held). Given that, you cannot pick a right return value when !debug_locks. Not sure we still do that, but I distinctly remember running into that when I did as you suggest now. But that was many years ago.
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index 7f0098d..4cef9f8 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -345,7 +345,8 @@ static inline int lock_is_held(struct lockdep_map *lock) return _lock_is_held(lock, -1); } -#define lockdep_is_held(lock) lock_is_held(&(lock)->dep_map) +#define lockdep_is_held(lock) lock_is_held(&(lock)->dep_map) +#define _lockdep_is_held(lock, r) _lock_is_held(&(lock)->dep_map, (r)) extern void lock_set_class(struct lockdep_map *lock, const char *name, struct lock_class_key *key, unsigned int subclass, diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index abec578..f39573b 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -3778,7 +3778,7 @@ void lock_release(struct lockdep_map *lock, int nested, } EXPORT_SYMBOL_GPL(lock_release); -int lock_is_held(struct lockdep_map *lock, int read) +int _lock_is_held(struct lockdep_map *lock, int read) { unsigned long flags; int ret = 0; @@ -3796,7 +3796,7 @@ int lock_is_held(struct lockdep_map *lock, int read) return ret; } -EXPORT_SYMBOL_GPL(lock_is_held); +EXPORT_SYMBOL_GPL(_lock_is_held); struct pin_cookie lock_pin_lock(struct lockdep_map *lock) {
On Thu, Aug 11, 2016 at 11:54:44PM +0200, Peter Zijlstra wrote: > Sure, find below. Not been near a compiler. It has now. Below are the additions I need, and things seem to be passing fine with that. Note that to fit the existing XFS lock asserts I'm using _lockdep_is_held directly instead of lockdep_assert_held_exclusive. All the exports are there, but I'm just trying to make sure you're not going to shout at me for that later :)