Re: [PATCH] Revert "workqueue: re-add lockdep dependencies for flushing"

[Johannes Berg <johannes@sipsolutions.net>]

On Tue, 2018-10-23 at 21:44 +0200, Johannes Berg wrote:

> > There is
> > no agreement however that the kind of checking implemented by the "crosslock"
> > code made sense. My understanding is that you are trying to reintroduce
> > through a backdoor some of the crosslock code. 
> 
> Not at all.

Perhaps I should elaborate on this, although I'm not really entirely
sure of it.

As I understand it, crosslock was trying to solve an entirely different
problem, namely that of tracking locks that can be acquired in one
thread, and released in another. This obviously still causes deadlocks,
but doesn't lend itself to actual tracking in the lockdep chain, since
you don't really know how directly long the lock was held.

With a classic mutex (spinlock, ...), you always have

lock(A)
do_something()
unlock(A)

in the same thread, so if do_something() contains another lock(B), you
know that you've got a dependency A->B.

If A instead is something that might be released in another thread, e.g.
a completion or semaphore, it's really hard to tell whether you have

down(A)
do_something()
up(A)

in a single thread, or if the up(A) happens elsewhere entirely.
Therefore, things like this aren't tracked by lockdep at all.

Crosslock tried to address this.

The workqueue annotations, on the other hand, *are* within the same
thread. You're either executing from the work struct, and the same
thread will obviously end up going out of the work struct again, or
you're flushing (and waiting) for the workqueue, so all of that also
happens in the same thread (apart from the actual work that you wait
for, but that's unrelated).

johannes