On Mon, Mar 20, 2023 at 01:13:05PM +0100, Peter Zijlstra wrote:
On Thu, Mar 16, 2023 at 08:13:36PM -0700, Boqun Feng wrote:
Lock scenario print is always a weak spot of lockdep splats. Improvement can be made if we rework the dependency search and the error printing.
However without touching the graph search, we can improve a little for the circular deadlock case, since we have the to-be-added lock dependency, and know whether these two locks are read/write/sync.
In order to know whether a held_lock is sync or not, a bit was "stolen" from ->references, which reduce our limit for the same lock class nesting from 2^12 to 2^11, and it should still be good enough.
Besides, since we now have bit in held_lock for sync, we don't need the "hardirqoffs being 1" trick, and also we can avoid the __lock_release() if we jump out of __lock_acquire() before the held_lock stored.
With these changes, a deadlock case evolved with read lock and sync gets a better print-out from:
[...] Possible unsafe locking scenario: [...] [...] CPU0 CPU1 [...] ---- ---- [...] lock(srcuA); [...] lock(srcuB); [...] lock(srcuA); [...] lock(srcuB);
to
[...] Possible unsafe locking scenario: [...] [...] CPU0 CPU1 [...] ---- ---- [...] rlock(srcuA); [...] lock(srcuB); [...] lock(srcuA); [...] sync(srcuB);
Signed-off-by: Boqun Feng boqun.feng@gmail.com Signed-off-by: Paul E. McKenney paulmck@kernel.org Signed-off-by: Boqun Feng boqun.feng@gmail.com
include/linux/lockdep.h | 3 ++- kernel/locking/lockdep.c | 48 ++++++++++++++++++++++++++-------------- 2 files changed, 34 insertions(+), 17 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index 14d9dbedc6c1..b32256e9e944 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -134,7 +134,8 @@ struct held_lock { unsigned int read:2; /* see lock_acquire() comment */ unsigned int check:1; /* see lock_acquire() comment */ unsigned int hardirqs_off:1;
- unsigned int references:12; /* 32 bits */
- unsigned int sync:1;
- unsigned int references:11; /* 32 bits */ unsigned int pin_count;
};
Yeah, I suppose we can do that -- another option is to steal some bits from pin_count, but whatever (references used to be 11 a long while ago, no problem going back to that).
Thanks!
Acked-by: Peter Zijlstra (Intel) peterz@infradead.org
Applied locally.
Regards, Boqun