On 05/13/2014 05:20 AM, Tejun Heo wrote:
> On Mon, May 12, 2014 at 02:56:15PM +0800, Lai Jiangshan wrote:
>> /**
>> + * worker_detach_from_pool() - detach the worker from the pool
>> + * @worker: worker which is attached to its pool
>> + * @pool: attached pool
>> + *
>> + * Undo the attaching which had been done in create_worker().
>> + * The caller worker shouldn't access to the pool after detached
>> + * except it has other reference to the pool.
>> + */
>> +static void worker_detach_from_pool(struct worker *worker,
>> + struct worker_pool *pool)
>> +{
>> + struct completion *detach_completion = NULL;
>> +
>> + mutex_lock(&pool->manager_mutex);
>> + idr_remove(&pool->worker_idr, worker->id);
>> + if (idr_is_empty(&pool->worker_idr))
>> + detach_completion = pool->detach_completion;
>> + mutex_unlock(&pool->manager_mutex);
>> +
>> + if (detach_completion)
>> + complete(detach_completion);
>> +}
>
> Are we gonna use this function from somewhere else too?
it is called from worker_thread().
I don't want to unfold it into worker_thread(), it is better
readability when it is wrapped and it will be called in patch10
for rescuer.
>
>> @@ -2289,6 +2298,10 @@ woke_up:
>> spin_unlock_irq(&pool->lock);
>> WARN_ON_ONCE(!list_empty(&worker->entry));
>> worker->task->flags &= ~PF_WQ_WORKER;
>> +
>> + set_task_comm(worker->task, "kworker_dying");
>
> Given how other kworkers are named, maybe a better name is
> "kworker/dying" or "kworker/detached"?
>
>> + worker_detach_from_pool(worker, pool);
>> + kfree(worker);
>> return 0;
>> }
>>
>> @@ -3561,6 +3574,7 @@ static void rcu_free_pool(struct rcu_head *rcu)
>> static void put_unbound_pool(struct worker_pool *pool)
>> {
>> struct worker *worker;
>> + DECLARE_COMPLETION_ONSTACK(detach_completion);
>
> I think it's conventional to put initialized ones (especially the ones
> require initializing macros) before uninitialized vars.
>
>> @@ -3579,19 +3593,24 @@ static void put_unbound_pool(struct worker_pool
>> *pool)
>>
>> /*
>> * Become the manager and destroy all workers. Grabbing
>> - * manager_arb prevents @pool's workers from blocking on
>> - * manager_mutex.
>> + * manager_arb ensures manage_workers() finish and enter idle.
>
> I don't follow what the above comment update is trying to say.
If a pool is destroying, the worker will not call manage_workers().
but the existing manage_workers() may be still running.
mutex_lock(&manager_arb) in put_unbound_pool() should wait this manage_workers()
finished due to the manager-worker (non-idle-worker) can't be destroyed.
>
> Thanks.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
