Messages in this thread | | | Subject | Re: [PATCH 2/3] kernel/workqueue: Surround work execution with shared lock annotations | From | Bart Van Assche <> | Date | Thu, 25 Oct 2018 10:22:42 -0700 |
| |
On Thu, 2018-10-25 at 18:53 +0200, Johannes Berg wrote: > On Thu, 2018-10-25 at 15:05 +0000, Bart Van Assche wrote: > > Surround execution of work with a shared lockdep annotation because multiple > > work items associated with a work queue may execute concurrently. > > Hmm. So, I'm not really entirely sure of the semantics here, but I fail > to see how "may execute concurrently" means "can be taken recursively"? > > After all, if they execute concurrently, that's in a different thread, > right? So each thread is really just doing something with this work. It > may not match mutex semantics in how mutexes would lock each other out > and prevent concurrency, but I don't think that matters to lockdep at > all. > > In fact, I'm not sure this actually changes anything, since you can't > really execute a work struct while executing one already? > > What's this intended to change? I currently don't see how lockdep's > behaviour would differ with read==1, unless you actually tried to do > recursive locking, which isn't really possible? > > Or perhaps this is actually the right change for the issue described in > patch 1, where a work struct flushes another work on the same wq, and > that causes recursion of sorts? But that recursion should only happen if > the workqueues is actually marked as ordered, in which case it *is* in > fact wrong?
How about modifying the wq->lockdep_map annotations only and not touching the work->lockdep_map annotations? My comment about concurrency in the patch description refers to a multithreaded workqueue executing multiple different work items concurrently. I am aware that great care has been taken in the workqueue implementation to ensure that each work item is executed by exactly one worker.
Bart.
| |