Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1486270

Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature

From Peter Zijlstra <peterz@infradead.org>
Newsgroups linux.kernel
Subject Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature
Date 2016-09-19 11:00 +0200
Message-ID <sj2AF-3cw-13@gated-at.bofh.it> (permalink)
References (3 earlier) <sgZxg-4RT-37@gated-at.bofh.it> <sh1IJ-6cg-5@gated-at.bofh.it> <sh87w-2fH-3@gated-at.bofh.it> <shdAd-6Rw-1@gated-at.bofh.it> <siWOB-87e-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, Sep 19, 2016 at 11:41:02AM +0900, Byungchul Park wrote:

> > But since these threads are independently scheduled there is no point in
> > transferring the point in time thread A does W to thread B. There is no
> > relation there.
> > 
> > B could have already executed the complete or it could not yet have
> > started execution at all or anything in between, entirely random.
> 
> Of course B could have already executed the complete or it could not yet
> have started execution at all or anything in between. But it's not entirely
> random.
> 
> It might be a random point since they are independently scheduled, but it's
> not entirely random. And it's a random point among valid points which lockdep
> needs to consider. For example,
> 
> 
> CONTEXT 1			CONTEXT 2(forked one)
> =========			=====================
> (a)				acquire F
> acquire A			acquire G
> acquire B			wait_for_completion Z
> acquire C
> (b)				acquire H
> fork 2				acquire I
> acquire D			acquire J
> complete Z			acquire K
> 

I'm hoping you left out the releases for brevity? Because calling fork()
with locks held is _really_ poor form.

> I can provide countless examples with which I can say you're wrong.
> In this case, all acquires between (a) and (b) must be ignored when
> generating dependencies with complete operation of Z.

I still don't get the point. Why does this matter?

Sure, A-C are irrelevant in this example, but I don't see how they're
differently irrelevant from a whole bunch of other prior state action.


Earlier you said the algorithm for selecting the dependency is the first
acquire observed in the completing thread after the
wait_for_completion(). Is this correct?


				W z

	A a
	for (i<0;i<many;i++) {
	  A x[i]
	  R x[i]
	}
	R a

	<IRQ>
	  A b
	  R b
	  C z
	</IRQ>

That would be 'a' in this case, but that isn't at all related. Its just
as irrelevant as your A-C. And we can pick @many as big as needed to
flush the prev held cyclic buffer (although I've no idea how that
matters either).

What we want here is to link z to b, no? That is the last, not the first
acquire, it also is independent of when W happened.

At the same time, picking the last is no guarantee either, since that
can equally miss dependencies. Suppose the IRQ handler did:

	<IRQ>
	  A c
	  R c
	  A b
	  R b
	  C z
	</IRQ>

instead. We'd miss the z depends on c relation, and since they're
independent lock sections, lockdep wouldn't make a b-c relation either.


Clearly I'm still missing stuff...

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v3 07/15] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2016-09-13 12:00 +0200
  Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2016-09-13 12:10 +0200
    Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2016-09-13 14:20 +0200
    Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Byungchul Park <max.byungchul.park@gmail.com> - 2016-09-13 17:20 +0200
  Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2016-09-13 17:10 +0200
    Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Byungchul Park <max.byungchul.park@gmail.com> - 2016-09-13 19:20 +0200
      Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2016-09-13 21:40 +0200
        Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2016-09-13 23:50 +0200
          Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Byungchul Park <max.byungchul.park@gmail.com> - 2016-09-14 03:10 +0200
        Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Byungchul Park <max.byungchul.park@gmail.com> - 2016-09-14 04:30 +0200
          Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Byungchul Park <max.byungchul.park@gmail.com> - 2016-09-14 06:50 +0200
          Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2016-09-14 10:20 +0200
            Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2016-09-19 04:50 +0200
              Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Peter Zijlstra <peterz@infradead.org> - 2016-09-19 11:00 +0200
                Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2016-09-20 08:10 +0200
                Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2016-09-20 08:40 +0200
                Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2016-09-21 03:50 +0200
                Re: [PATCH v3 07/15] lockdep: Implement crossrelease feature Byungchul Park <byungchul.park@lge.com> - 2016-09-22 05:10 +0200

csiph-web