From: Boqun Feng <boqun.feng@gmail.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
Andrea Parri <parri.andrea@gmail.com>
Subject: Re: [RFC tip/locking/lockdep v5 04/17] lockdep: Introduce lock_list::dep
Date: Mon, 26 Feb 2018 18:20:16 +0800 [thread overview]
Message-ID: <20180226102016.4rms4ro6wcwwsw7d@tardis> (raw)
In-Reply-To: <20180226101524.xaarifgmhzdzq7gs@tardis>
[-- Attachment #1: Type: text/plain, Size: 3104 bytes --]
On Mon, Feb 26, 2018 at 06:15:24PM +0800, Boqun Feng wrote:
> On Mon, Feb 26, 2018 at 10:00:50AM +0100, Peter Zijlstra wrote:
> > On Sat, Feb 24, 2018 at 05:26:52PM +0800, Boqun Feng wrote:
> > > > This is for case like:
> > > >
> > > > TASK1:
> > > > read_lock(A);
> > > > read_lock(B);
> > > >
> > > > TASK2:
> > > > write_lock(B);
> > > > read_lock(C);
> > > >
> > > > TASK3:
> > > > read_lock(B);
> > > > write_lock(C);
> > > >
> > > > TASK4:
> > > > read_lock(C);
> > > > write_lock(A);
> > > >
> > > > , which is not a deadlock.
> > > >
> > >
> > > After TASK 1,2,3 have executed, we have A -(RR)-> B, B -(RN/NR)-> C, and
> > > when TASK4 executed, we will try to add C -(RN)-> A into the graph.
> > > Before that we need to check whether we have a A -> ... -(*N)-> C path
> > > in the graph already, so we search from A (@prev is C and @this is A):
> > >
> > > * we set A->have_xr to false, because the dependency we are adding
> > > is a RN.
> > >
> > > * we find A -(RR)-> B, and since have_xr (= A->have_xr) is false,
> > > we can pick this dependency, and since for A -> B, we only have
> > > RR, so we set B->have_xr to true.
> > >
> > > * we then find B -(RN/NR)-> C, and since have_xr (= B->have_xr) is
> > > true, we will pick it only only_rx(C->dep) return false,
> > > otherwise we skip. Because we have RN and NR for B -> C,
> > > therefore we won't skip B -> C.
> > >
> > > * Now we try to set C->have_xr, if we set it to only_xr(C->dep),
> > > we will set it to false, right? Because B -> C has RN.
> > >
> > > * Since we now find a entry equal to @prev, we go into the
> > > hlock_conflict() logic and for expression
> > >
> > > hlock->read != 2 || !entry->have_xr
> > >
> > > @hlock is the C in TASK4, so hlock->read == 2, and @entry is the
> > > C whose ->have_xr we just set, so entry->have_xr is false.
> > > Therefore hlock_conflict() returns true. And that indicates we
> > > find a deadlock in the search. But the above senario can not
> > > introduce a deadlock.
> > >
> > > Could this help you, or I miss something?
> >
> > Yes, although it took me forever to grasp because I have snot for brains
> > atm :-(
> >
>
> Take care!
>
> > Would something like:
> >
> >
> > dep = entry->dep;
> >
> > /* Mask out all *R -> R* relations. */
> > if (have_xr)
> > dep &= ~(RR_MASK | RN_MASK);
> >
> > /* If nothing left, we're done. */
> > if (!dep)
> > continue;
> >
> > /* If there are (only) *R left, set that for the next step. */
> > entry->have_xr = !(dep & (RN_MASK | NN_MASK));
> >
> >
> > Work? I think that reads fairly simple.
>
> I think that works, will apply this simplification to see whether the
> self tests agree.
>
And the self tests agree this works ;-) Thank you!
Regards,
Boqun
> Btw, given the comments in the code, I think it's better to name
> "have_xr" as "only_xr"? I have a feeling that my comment-less code
> somehow misled you to that name ;-(
>
> Regards,
> Boqun
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2018-02-26 10:16 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-22 7:08 [RFC tip/locking/lockdep v5 00/17] lockdep: Support deadlock detection for recursive read locks Boqun Feng
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 01/17] lockdep: Demagic the return value of BFS Boqun Feng
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 02/17] lockdep: Make __bfs() visit every dependency until a match Boqun Feng
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 03/17] lockdep: Redefine LOCK_*_STATE* bits Boqun Feng
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 04/17] lockdep: Introduce lock_list::dep Boqun Feng
2018-02-23 11:55 ` Peter Zijlstra
2018-02-23 12:37 ` Boqun Feng
2018-02-24 5:32 ` Boqun Feng
2018-02-24 6:30 ` Boqun Feng
2018-02-24 8:38 ` Peter Zijlstra
2018-02-24 9:00 ` Boqun Feng
2018-02-24 9:26 ` Boqun Feng
2018-02-26 9:00 ` Peter Zijlstra
2018-02-26 10:15 ` Boqun Feng
2018-02-26 10:20 ` Boqun Feng [this message]
2018-02-24 7:31 ` Boqun Feng
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 05/17] lockdep: Extend __bfs() to work with multiple kinds of dependencies Boqun Feng
2018-02-22 14:26 ` Peter Zijlstra
2018-02-22 15:12 ` Boqun Feng
2018-02-22 15:30 ` Peter Zijlstra
2018-02-22 15:51 ` Peter Zijlstra
2018-02-22 16:31 ` Boqun Feng
2018-02-23 5:02 ` Boqun Feng
2018-02-23 11:15 ` Peter Zijlstra
2018-02-22 16:08 ` Peter Zijlstra
2018-02-22 16:34 ` Boqun Feng
2018-02-22 16:32 ` Peter Zijlstra
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 06/17] lockdep: Support deadlock detection for recursive read in check_noncircular() Boqun Feng
2018-02-22 14:54 ` Peter Zijlstra
2018-02-22 15:16 ` Peter Zijlstra
2018-02-22 15:44 ` Boqun Feng
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 07/17] lockdep: Adjust check_redundant() for recursive read change Boqun Feng
2018-02-22 17:29 ` Peter Zijlstra
2018-03-16 8:20 ` Boqun Feng
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 08/17] lockdep: Fix recursive read lock related safe->unsafe detection Boqun Feng
2018-02-22 17:41 ` Peter Zijlstra
2018-02-22 17:46 ` Peter Zijlstra
2018-02-23 8:21 ` Boqun Feng
2018-02-23 8:58 ` Boqun Feng
2018-02-23 11:36 ` Peter Zijlstra
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 09/17] lockdep: Add recursive read locks into dependency graph Boqun Feng
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 10/17] lockdep/selftest: Add a R-L/L-W test case specific to chain cache behavior Boqun Feng
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 11/17] lockdep: Take read/write status in consideration when generate chainkey Boqun Feng
2018-02-22 7:08 ` [RFC tip/locking/lockdep v5 12/17] lockdep/selftest: Unleash irq_read_recursion2 and add more Boqun Feng
2018-02-22 7:09 ` [RFC tip/locking/lockdep v5 13/17] lockdep/selftest: Add more recursive read related test cases Boqun Feng
2018-02-22 7:09 ` [RFC tip/locking/lockdep v5 14/17] Revert "locking/lockdep/selftests: Fix mixed read-write ABBA tests" Boqun Feng
2018-02-22 7:09 ` [RFC tip/locking/lockdep v5 15/17] lockdep: Reduce the size of lock_list Boqun Feng
2018-02-23 11:38 ` Peter Zijlstra
2018-02-23 12:40 ` Boqun Feng
2018-02-22 7:09 ` [RFC tip/locking/lockdep v5 16/17] lockdep: Documention for recursive read lock detection reasoning Boqun Feng
2018-02-24 22:53 ` Andrea Parri
2018-02-27 2:32 ` Boqun Feng
2018-02-22 7:09 ` [RFC tip/locking/lockdep v5 17/17] MAINTAINERS: Add myself as a LOCKING PRIMITIVES reviewer Boqun Feng
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=20180226102016.4rms4ro6wcwwsw7d@tardis \
--to=boqun.feng@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=parri.andrea@gmail.com \
--cc=peterz@infradead.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).