From: Peter Zijlstra <peterz@infradead.org>
To: Boqun Feng <boqun.feng@gmail.com>
Cc: rcu@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
Will Deacon <will@kernel.org>, Waiman Long <longman@redhat.com>,
Lai Jiangshan <jiangshanlai@gmail.com>,
"Paul E. McKenney" <paulmck@kernel.org>,
Josh Triplett <josh@joshtriplett.org>,
Steven Rostedt <rostedt@goodmis.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Davidlohr Bueso <dave@stgolabs.net>,
Frederic Weisbecker <frederic@kernel.org>,
Neeraj Upadhyay <quic_neeraju@quicinc.com>,
Joel Fernandes <joel@joelfernandes.org>,
Shuah Khan <shuah@kernel.org>,
David Woodhouse <dwmw2@infradead.org>,
Paolo Bonzini <pbonzini@redhat.com>,
kvm@vger.kernel.org, seanjc@google.com,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH rcu 4/7] locking/lockdep: Improve the deadlock scenario print for sync and read lock
Date: Mon, 20 Mar 2023 13:13:05 +0100 [thread overview]
Message-ID: <20230320121305.GK2194297@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20230317031339.10277-5-boqun.feng@gmail.com>
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).
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
next prev parent reply other threads:[~2023-03-20 12:13 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-17 3:13 [PATCH rcu 0/7] RCU-related lockdep changes for v6.4 Boqun Feng
2023-03-17 3:13 ` [PATCH rcu 1/7] locking/lockdep: Introduce lock_sync() Boqun Feng
2023-03-20 17:06 ` Davidlohr Bueso
2023-03-20 17:50 ` Boqun Feng
2023-03-17 3:13 ` [PATCH rcu 2/7] rcu: Annotate SRCU's update-side lockdep dependencies Boqun Feng
2023-03-17 3:13 ` [PATCH rcu 3/7] locking: Reduce the number of locks in ww_mutex stress tests Boqun Feng
2023-03-17 18:38 ` Paul E. McKenney
2023-03-17 21:26 ` Boqun Feng
2023-03-17 3:13 ` [PATCH rcu 4/7] locking/lockdep: Improve the deadlock scenario print for sync and read lock Boqun Feng
2023-03-20 12:13 ` Peter Zijlstra [this message]
2023-03-20 17:50 ` Boqun Feng
2023-03-17 3:13 ` [PATCH rcu 5/7] rcutorture: Add SRCU deadlock scenarios Boqun Feng
2023-03-17 3:13 ` [PATCH rcu 6/7] rcutorture: Add RCU Tasks Trace and " Boqun Feng
2023-03-17 3:13 ` [PATCH rcu 7/7] rcutorture: Add srcu_lockdep.sh Boqun Feng
2023-03-20 18:19 ` Boqun Feng
2023-03-20 19:09 ` Paul E. McKenney
2023-03-20 19:28 ` Boqun Feng
2023-03-20 20:22 ` Paul E. McKenney
2023-03-20 20:26 ` Boqun Feng
2023-03-20 20:36 ` 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=20230320121305.GK2194297@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=boqun.feng@gmail.com \
--cc=dave@stgolabs.net \
--cc=dwmw2@infradead.org \
--cc=frederic@kernel.org \
--cc=jiangshanlai@gmail.com \
--cc=joel@joelfernandes.org \
--cc=josh@joshtriplett.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=longman@redhat.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@redhat.com \
--cc=paulmck@kernel.org \
--cc=pbonzini@redhat.com \
--cc=quic_neeraju@quicinc.com \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=seanjc@google.com \
--cc=shuah@kernel.org \
--cc=will@kernel.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).