On Wed, Aug 23, 2017 at 12:46:48PM +0200, Peter Zijlstra wrote: > On Wed, Aug 23, 2017 at 11:31:18AM +0900, Byungchul Park wrote: > > On Tue, Aug 22, 2017 at 11:06:03AM +0200, Peter Zijlstra wrote: > > > Currently, we do the following in process_one_work(), > > > > lockdep_map_acquire for a workqueue > > lockdep_map_acquire for a work > > > > But IMHO it should be, > > > > lockdep_map_acquire for a pair of workqueue and work. > > > > Right? > > No, it is right. We need the two 'locks'. > > The work one is for flush_work(), the workqueue one is for > flush_workqueue(). > > Just like how flush_work() must not depend on any lock taken inside the > work, flush_workqueue() callers must not hold any lock acquired inside > any work ran inside the workqueue. This cannot be done with a single > 'lock'.
Thank you for explanation. > The reason flush_work() also depends on the wq 'lock' is because doing > flush_work() from inside work is a possible problem for single threaded > workqueues and workqueues with a rescuer. > > > > Agreed. The interaction with workqueues is buggered. > > > > I think original uses of lockdep_map were already wrong. I mean it's > > not a problem of newly introduced code. > > Not wrong per-se, the new code certainly places more constraints on it. "the new code places more constraints on it" is just the right expression.
