Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature

From	Peter Zijlstra <peterz@infradead.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature
Date	2017-03-07 19:50 +0100
Message-ID	<tislj-35L-3@gated-at.bofh.it> (permalink)
References	(4 earlier) <tgdJM-4r7-21@gated-at.bofh.it> <tgD1w-5Gf-29@gated-at.bofh.it> <tgJqi-1AZ-23@gated-at.bofh.it> <tgQBs-6J0-19@gated-at.bofh.it> <thuIx-257-7@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Sun, Mar 05, 2017 at 12:08:45PM +0900, Byungchul Park wrote:
> On Fri, Mar 03, 2017 at 09:14:16AM +0100, Peter Zijlstra wrote:

> > 
> > Now the problem with the above condition is that it makes reports
> > harder to decipher, because by avoiding adding redundant links to our
> > graph we loose a possible shorter path.
> 
> Let's see the following example:
> 
>    A -> B -> C
> 
>    where A, B and C are typical lock class.
> 
> Assume the graph above was built and operations happena in the
> following order:
> 
>    CONTEXT X		CONTEXT Y
>    ---------		---------
>    acquire DX
> 			acquire A
> 			acquire B
> 			acquire C
> 
> 			release and commit DX
> 
>    where A, B and C are typical lock class, DX is a crosslock class.
> 
> The graph will grow as following _without_ prev_gen_id.
> 
>         -> A -> B -> C
>        /    /    /
>    DX -----------
> 
>    where A, B and C are typical lock class, DX is a crosslock class.
> 
> The graph will grow as following _with_ prev_gen_id.
> 
>    DX -> A -> B -> C
> 
>    where A, B and C are typical lock class, DX is a crosslock class.
> 
> You said the former is better because it has smaller cost in bfs.

No, I said the former is better because when you report a DX inversion
against C, A and B are not required and the report is easier to
understand by _humans_.

I don't particularly care about the BFS cost itself.

> But it has to use _much_ more memory to keep additional nodes in
> graph. Without exaggeration, every crosslock would get linked with all
> locks in history locks, on commit, unless redundant. It might be
> pretty more than we expect - I will check and let you know how many it
> is. Is it still good?

Dunno, probably not.. but it would be good to have numbers.

Thread

Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-03-01 06:50 +0100
  Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2017-03-01 15:50 +0100
    Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2017-03-02 18:50 +0100
      Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-03-03 01:40 +0100
        Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2017-03-03 09:20 +0100
          Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2017-03-03 10:50 +0100
            Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2017-03-03 10:50 +0100
            Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-03-05 04:40 +0100
          Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-03-05 04:10 +0100
            Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2017-03-07 19:50 +0100
      Re: [PATCH v5 06/13] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2017-03-03 01:50 +0100

csiph-web