Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1722249 > unrolled thread
| Started by | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| First post | 2017-08-29 11:10 +0200 |
| Last post | 2017-08-31 10:10 +0200 |
| Articles | 6 on this page of 46 — 7 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-08-29 11:10 +0200
[tip:locking/core] locking/lockdep: Untangle xhlock history save/restore from task independence tip-bot for Peter Zijlstra <tipbot@zytor.com> - 2017-08-29 16:30 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <max.byungchul.park@gmail.com> - 2017-08-29 18:10 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-08-29 20:50 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-08-30 04:20 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-08-30 09:50 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-08-30 11:00 +0200
RE: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation "Byungchul Park" <byungchul.park@lge.com> - 2017-08-30 11:10 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-08-30 11:20 +0200
RE: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation "Byungchul Park" <byungchul.park@lge.com> - 2017-08-30 11:40 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-08-30 11:20 +0200
RE: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation "Byungchul Park" <byungchul.park@lge.com> - 2017-08-30 11:30 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-08-30 13:30 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <max.byungchul.park@gmail.com> - 2017-08-30 15:00 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-08-31 09:30 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-08-31 10:10 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-08-31 10:20 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-08-31 10:40 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-01 04:10 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-09-01 11:50 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-01 12:20 +0200
RE: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com) <byungchul.park@lge.com> - 2017-09-01 14:10 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-09-01 14:40 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <max.byungchul.park@gmail.com> - 2017-09-01 16:00 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-09-01 18:40 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-04 03:40 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-04 04:10 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-09-04 13:50 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-05 02:40 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-09-05 09:10 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-09-05 09:20 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-05 11:00 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-09-05 11:40 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-05 12:40 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-05 13:00 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-09-05 15:50 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-06 02:00 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Boqun Feng <boqun.feng@gmail.com> - 2017-09-06 02:50 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-06 03:40 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-07 02:00 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-07 02:20 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-06 02:50 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-09-05 13:00 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-05 13:30 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-09-05 10:40 +0200
Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-08-31 10:10 +0200
Page 3 of 3 — ← Prev page 1 2 [3]
| From | Byungchul Park <byungchul.park@lge.com> |
|---|---|
| Date | 2017-09-07 02:20 +0200 |
| Message-ID | <umSI1-EN-3@gated-at.bofh.it> |
| In reply to | #1727110 |
On Wed, Sep 06, 2017 at 10:32:54AM +0900, Byungchul Park wrote: > > What do you mean by "false dependencies"? AFAICT, recursive-read could > > All locks used in every work->func() generate false dependencies with > 'work' and 'wq', while any flush works are not involved. It's inevitable. > > Moreover, it's also possible to generate more false ones between the > pseudo acquisitions, if real acquisitions are used for that speculative > purpose e.i. recursive-read here, which are anyway real ones. > > Moreover, it's also possible to generate more false ones between holding > locks and the pseudo ones, of course, the workqueue code is not the case > for now. > > Moreover, it's also possible to generate more false ones between the > pseudo ones and crosslocks on commit, once making crossrelease work even > for recursive-read things. Hi Peter, What do you think about the above? Just let me know please. It's ok if you think it's not needed yet. Or, if you think it's necessary from now on, I'm going ahead for that work, starting from renaming the 'might' thing.
[toc] | [prev] | [next] | [standalone]
| From | Byungchul Park <byungchul.park@lge.com> |
|---|---|
| Date | 2017-09-06 02:50 +0200 |
| Message-ID | <umwHw-1VB-15@gated-at.bofh.it> |
| In reply to | #1726742 |
On Tue, Sep 05, 2017 at 03:46:43PM +0200, Peter Zijlstra wrote: > On Tue, Sep 05, 2017 at 07:58:38PM +0900, Byungchul Park wrote: > > On Tue, Sep 05, 2017 at 07:31:44PM +0900, Byungchul Park wrote: > > > Recursive-read and the hint I proposed(a.k.a. might) should be used for > > > their different specific applications. Both meaning and constraints of > > > them are totally different. > > > > > > Using a right function semantically is more important than making it > > > just work, as you know. Wrong? > > > Of course, in the following cases, the results are same: > > > > recursive-read(A) -> recursive-read(A), is like nothing, and also > > might(A) -> might(A) , is like nothing. > > > > recursive-read(A) -> lock(A), end in a deadlock, and also > > might(A) -> lock(A), end in a deadlock. > > And these are exactly the cases we need. > > > Futhermore, recursive-read-might() can be used if needed, since their > > semantics are orthogonal so they can be used in mixed forms. > > > > I really hope you accept the new semantics... I think current workqueue > > code exactly needs the semantics. > > I really don't want to introduce this extra state if we don't have to. > And as you already noted, this 'might' thing of yours doesn't belong in > the .read argument, since as you say its orthogonal. Right. Of course, it can be changed to be a proper form if allowed. I was afraid to introduce another new function instead of using an arg. > recursive-read > wait_for_completion() > recursive-read > complete() > > is fundamentally not a deadlock, we don't need anything extra. It might be ok wrt the workqueue. But, I think generally the recursive-read is not a good option for that purpose.
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-09-05 13:00 +0200 |
| Message-ID | <umjKi-1Bt-23@gated-at.bofh.it> |
| In reply to | #1726602 |
On Tue, Sep 05, 2017 at 07:31:44PM +0900, Byungchul Park wrote: > Let me show you a possible scenario with a leaf lock: > > lock(A) > lock(A) wait_for_completion(B) > unlock(A) ... > ... unlock(A) > process_one_work() > work->func() > complete(B) > > It's a deadlock by a lead lock A and completion B. By having wait_for_completion() in it, A is not a leaf lock.
[toc] | [prev] | [next] | [standalone]
| From | Byungchul Park <byungchul.park@lge.com> |
|---|---|
| Date | 2017-09-05 13:30 +0200 |
| Message-ID | <umkdj-21X-13@gated-at.bofh.it> |
| In reply to | #1726615 |
On Tue, Sep 05, 2017 at 12:52:36PM +0200, Peter Zijlstra wrote: > On Tue, Sep 05, 2017 at 07:31:44PM +0900, Byungchul Park wrote: > > Let me show you a possible scenario with a leaf lock: > > > > lock(A) > > lock(A) wait_for_completion(B) > > unlock(A) ... > > ... unlock(A) > > process_one_work() > > work->func() > > complete(B) > > > > It's a deadlock by a lead lock A and completion B. > > By having wait_for_completion() in it, A is not a leaf lock. I see. After all, you want to force to use only leaf locks in (1), (3) and (5) forever in future. I really don't understand why you want to force it and use them carefully always in head, *only* for that locks, though original code even makes it unnecessary. But ok... Let's discuss the issue later if necessary. Again, I think your patches are worth nothing but avoiding the workqueue issue. I really hope you think it more, but it's ok if you don't want...
[toc] | [prev] | [next] | [standalone]
| From | Byungchul Park <byungchul.park@lge.com> |
|---|---|
| Date | 2017-09-05 10:40 +0200 |
| Message-ID | <umhyO-kE-5@gated-at.bofh.it> |
| In reply to | #1726384 |
On Tue, Sep 05, 2017 at 09:08:25AM +0200, Peter Zijlstra wrote: > > Your patches only do avoiding the wq issue now we focus on. > > > > Look at: > > > > worker thread another context > > ------------- --------------- > > wait_for_completion() > > | > > | (1) > > v > > +---------+ > > | Work A | (2) > > +---------+ > > | > > | (3) > > v > > +---------+ > > | Work B | (4) > > +---------+ > > | > > | (5) > > v > > +---------+ > > | Work C | (6) > > +---------+ > > | > > v > > > > We have to consider whole context of the worker to build dependencies > > with a crosslock e.g. wait_for_commplete(). > > > > Only thing we have to care here is to make all works e.g. (2), (4) and > > (6) independent, because workqueue does _concurrency control_. As I said > > last year at the very beginning, for works not applied the control e.g. > > max_active == 1, we don't need that isolation. I said, it's a future work. > > > > It would have been much easier to communicate with each other if you > > *tried* to understand my examples like now or you *tried* to give me one > > example at least. You didn't even *try*. Only thing I want to ask you > > for is to *try* to understand my opinions on conflicts. > > > > Now, understand what I intended? Still unsufficient? > > So you worry about max_active==1 ? Or you worry about pool->lock or > about the thread setup? I'm still not sure. It's close to the letter. Precisely, I worry about (1), (3), (5) and so on, since they certainly create dependencies with crosslocks e.g. completion in my example.
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-08-31 10:10 +0200 |
| Message-ID | <uksI1-38N-9@gated-at.bofh.it> |
| In reply to | #1723146 |
On Wed, Aug 30, 2017 at 06:24:39PM +0900, Byungchul Park wrote: > > And there obviously _should_ not be any dependencies between those. A > > 100% right. Since there obviously should not be any, it would be better > to check them. So I've endlessly asked you 'do you have any reason removing > the opportunity for that check?'. Overhead? Logical problem? Or want to > believe workqueue setup code perfect forever? I mean, is it a problem if we > check them? Nothing is perfect. And cross-release less that others. Covering that case for the first few works really isn't worth it.
[toc] | [prev] | [standalone]
Page 3 of 3 — ← Prev page 1 2 [3]
Back to top | Article view | linux.kernel
csiph-web