Re: [PATCH workqueue/for-4.14-fixes] workqueue: replace pool->manager_arb mutex with a flag

[Tejun Heo <tj@kernel.org>]

Hello, Boqun.

On Mon, Oct 09, 2017 at 10:21:17PM +0800, Boqun Feng wrote:
> > +static DECLARE_WAIT_QUEUE_HEAD(wq_manager_wait); /* wait for manager to go away */
> 
> I think this wait_queue_head better be a per-pool one rather than shared
> among pools?

It should be fine either way.  All the involved operations are pretty
low frequency.

> > @@ -3338,7 +3332,10 @@ static void put_unbound_pool(struct worker_pool *pool)
> >  	if (pool->detach_completion)
> >  		wait_for_completion(pool->detach_completion);
> >  
> > -	mutex_unlock(&pool->manager_arb);
> > +	spin_lock_irq(&pool->lock);
> > +	pool->flags &= ~POOL_MANAGER_ACTIVE;
> > +	wake_up(&wq_manager_wait);
> > +	spin_unlock_irq(&pool->lock);
> >  
> 
> Is the above code necesarry? IIUC, we are going to free the pool
> entirely, so whether manager is active is pointless here and no one is
> waiting for the ->flags of *this* pool to be !POOL_MANAGER_ACTIVE.
> 
> Am I missing something subtle here?

Ah, true.  I'll drop the above chunk.

Thanks.

-- 
tejun