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

Groups > linux.kernel > #1722180 > unrolled thread

Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation

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.

Contents

  Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <byungchul.park@lge.com> - 2017-08-29 08:50 +0200
    Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Peter Zijlstra <peterz@infradead.org> - 2017-08-29 11:10 +0200
      Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation Byungchul Park <max.byungchul.park@gmail.com> - 2017-08-29 18:20 +0200

#1722180 — Re: [PATCH 4/4] lockdep: Fix workqueue crossrelease annotation

On Wed, Aug 23, 2017 at 01:58:47PM +0200, Peter Zijlstra wrote:
> The new completion/crossrelease annotations interact unfavourable with
> the extant flush_work()/flush_workqueue() annotations.
> 
> The problem is that when a single work class does:
> 
>   wait_for_completion(&C)
> 
> and
> 
>   complete(&C)
> 
> in different executions, we'll build dependencies like:
> 
>   lock_map_acquire(W)
>   complete_acquire(C)
> 
> and
> 
>   lock_map_acquire(W)
>   complete_release(C)
> 
> which results in the dependency chain: W->C->W, which lockdep thinks
> spells deadlock, even though there is no deadlock potential since
> works are ran concurrently.
> 
> One possibility would be to change the work 'lock' to recursive-read,
> but that would mean hitting a lockdep limitation on recursive locks.
> Also, unconditinoally switching to recursive-read here would fail to
> detect the actual deadlock on single-threaded workqueues, which do
> have a problem with this.
> 
> For now, forcefully disregard these locks for crossrelease.

Eventually, you pushed this patch to tip tree without any comment.

I don't really understand you.

How does a maintainer choose a very work-around method and avoid
problems rather than fix a root cause? I am very disappointed.

But, I have nothing to do against your will.

[toc] | [next] | [standalone]

#1722253

On Tue, Aug 29, 2017 at 03:46:38PM +0900, Byungchul Park wrote:

> How does a maintainer choose a very work-around method and avoid
> problems rather than fix a root cause? I am very disappointed.

Time.. we need this sorted before we push the whole lot to Linus in the
next window. Fixing the recursive-read thing is far more work (although
Boqun did post some patches for that, which I still have to look at).

[toc] | [prev] | [next] | [standalone]

#1722596

On Tue, Aug 29, 2017 at 6:01 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Aug 29, 2017 at 03:46:38PM +0900, Byungchul Park wrote:
>
>> How does a maintainer choose a very work-around method and avoid
>> problems rather than fix a root cause? I am very disappointed.
>
> Time.. we need this sorted before we push the whole lot to Linus in the
> next window. Fixing the recursive-read thing is far more work (although
> Boqun did post some patches for that, which I still have to look at).

As I said, it's not a problem of whether read-acquire should be used or not, of
course I thought so, for the first time. But now I see what problems are, as I
already answered to TJ. The work which makes read-acquire work well is
worth itself. But, problems we should focus on now is that manual acquire(work,
wq) should be replaced with others or removed.

-- 
Thanks,
Byungchul

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web