All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lai Jiangshan <laijs@cn.fujitsu.com>
To: Tejun Heo <tj@kernel.org>
Cc: <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 06/10 V2] workqueue: convert worker_idr to worker_ida
Date: Tue, 13 May 2014 14:43:06 +0800	[thread overview]
Message-ID: <5371BEFA.9060902@cn.fujitsu.com> (raw)
In-Reply-To: <20140512214030.GG18959@mtj.dyndns.org>

On 05/13/2014 05:40 AM, Tejun Heo wrote:
> On Mon, May 12, 2014 at 02:56:18PM +0800, Lai Jiangshan wrote:
>> @@ -1681,7 +1682,6 @@ static void worker_detach_from_pool(struct worker *worker,
>>  	struct completion *detach_completion = NULL;
>>  
>>  	mutex_lock(&pool->manager_mutex);
>> -	idr_remove(&pool->worker_idr, worker->id);
>>  	list_del(&worker->node);
>>  	if (list_empty(&pool->workers))
>>  		detach_completion = pool->detach_completion;
> 
> Why are we moving ida removal to the caller here?  Does

ida							is for worker ID
pool->workers list and worker_detach_from_pool()	are for attaching/detaching

moving ida removal to the caller removes the unneeded coupling.

> worker_detach_from_pool() get used for something else later?  Up until
> this point, the distinction seems pretty arbitrary.
> 
>> @@ -1757,8 +1754,6 @@ static struct worker *create_worker(struct worker_pool *pool)
>>  	if (pool->flags & POOL_DISASSOCIATED)
>>  		worker->flags |= WORKER_UNBOUND;
>>  
>> -	/* successful, commit the pointer to idr */
>> -	idr_replace(&pool->worker_idr, worker, worker->id);
> 
> Ah, the comment is removed altogether.  Please disregard my previous
> review on the comment.
> 
> Thanks.
> 


  reply	other threads:[~2014-05-13  6:38 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-27  4:08 [PATCH 00/10] workqueue: async worker destruction and pool-binding synchronization Lai Jiangshan
2014-04-27  4:08 ` [PATCH 01/10] workqueue: use manager lock only to protect worker_idr Lai Jiangshan
2014-05-05 13:01   ` Tejun Heo
2014-05-06 15:43     ` Lai Jiangshan
2014-04-27  4:08 ` [PATCH 02/10] workqueue: destroy_worker() should destroy idle workers only Lai Jiangshan
2014-05-05 13:13   ` Tejun Heo
2014-05-06 15:58     ` Lai Jiangshan
2014-04-27  4:08 ` [PATCH 03/10] workqueue: async worker destruction Lai Jiangshan
2014-05-05 14:31   ` Tejun Heo
2014-05-05 15:02   ` Tejun Heo
2014-05-06 16:27     ` Lai Jiangshan
2014-05-06 16:31       ` Tejun Heo
2014-05-07  7:30   ` Lai Jiangshan
2014-05-07 13:15     ` Tejun Heo
2014-04-27  4:08 ` [PATCH 04/10] workqueue: destroy worker directly in the idle timeout handler Lai Jiangshan
2014-05-05 14:36   ` Tejun Heo
2014-05-07  7:10     ` Lai Jiangshan
2014-05-07 13:12       ` Tejun Heo
2014-05-07 13:38         ` Lai Jiangshan
2014-05-07 13:41           ` Tejun Heo
2014-05-07 15:30             ` Lai Jiangshan
2014-05-07 15:37               ` Tejun Heo
2014-04-27  4:09 ` [PATCH 05/10] workqueue: separate iteration role from worker_idr Lai Jiangshan
2014-05-05 14:57   ` Tejun Heo
2014-04-27  4:09 ` [PATCH 06/10] workqueue: convert worker_idr to worker_ida Lai Jiangshan
2014-05-05 14:59   ` Tejun Heo
2014-05-06 16:33     ` Lai Jiangshan
2014-05-06 16:35       ` Tejun Heo
2014-05-06 16:38         ` Tejun Heo
2014-04-27  4:09 ` [PATCH 07/10] workqueue: narrow the protection range of manager_mutex Lai Jiangshan
2014-04-27  4:09 ` [PATCH 08/10] workqueue: rename manager_mutex to bind_mutex Lai Jiangshan
2014-04-27  4:09 ` [PATCH 09/10] workqueue: separate pool-binding code out from create_worker() Lai Jiangshan
2014-04-27  4:09 ` [PATCH 10/10] workqueue: use generic pool-bind/unbind routine for rescuers Lai Jiangshan
2014-05-05 14:54   ` Tejun Heo
2014-05-05 15:05 ` [PATCH 00/10] workqueue: async worker destruction and pool-binding synchronization Tejun Heo
2014-05-12  6:56   ` [PATCH 00/10 V2] workqueue: async worker destruction and worker attaching/detaching Lai Jiangshan
2014-05-12  6:56     ` [PATCH 01/10 V2] workqueue: use manager lock only to protect worker_idr Lai Jiangshan
2014-05-12  6:56     ` [PATCH 02/10 V2] workqueue: destroy_worker() should destroy idle workers only Lai Jiangshan
2014-05-12 21:08       ` Tejun Heo
2014-05-13  6:08         ` Lai Jiangshan
2014-05-12  6:56     ` [PATCH 03/10 V2] workqueue: async worker destruction Lai Jiangshan
2014-05-12 21:20       ` Tejun Heo
2014-05-13  6:32         ` Lai Jiangshan
2014-05-13 14:14           ` Tejun Heo
2014-05-20  9:54             ` Lai Jiangshan
2014-05-12  6:56     ` [PATCH 04/10 V2] workqueue: destroy worker directly in the idle timeout handler Lai Jiangshan
2014-05-12 21:26       ` Tejun Heo
2014-05-12  6:56     ` [PATCH 05/10 V2] workqueue: separate iteration role from worker_idr Lai Jiangshan
2014-05-12 21:35       ` Tejun Heo
2014-05-12 21:37       ` Tejun Heo
2014-05-12  6:56     ` [PATCH 06/10 V2] workqueue: convert worker_idr to worker_ida Lai Jiangshan
2014-05-12 21:40       ` Tejun Heo
2014-05-13  6:43         ` Lai Jiangshan [this message]
2014-05-13 14:17           ` Tejun Heo
2014-05-12  6:56     ` [PATCH 07/10 V2] workqueue: narrow the protection range of manager_mutex Lai Jiangshan
2014-05-12  6:56     ` [PATCH 08/10 V2] workqueue: rename manager_mutex to attach_mutex Lai Jiangshan
2014-05-12 22:01       ` Tejun Heo
2014-05-13  6:45         ` Lai Jiangshan
2014-05-13 14:19           ` Tejun Heo
2014-05-12  6:56     ` [PATCH 09/10 V2] workqueue: separate pool-attaching code out from create_worker() Lai Jiangshan
2014-05-12  6:56     ` [PATCH 10/10 V2] workqueue: use generic attach/detach routine for rescuers Lai Jiangshan
2014-05-12 22:05       ` Tejun Heo
2014-05-13  6:55         ` Lai Jiangshan
2014-05-20  9:46     ` [PATCH V3 00/10] workqueue: async worker destruction and worker attaching/detaching Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 01/10] workqueue: use manager lock only to protect worker_idr Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 02/10] workqueue: destroy_worker() should destroy idle workers only Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 03/10] workqueue: async worker destruction Lai Jiangshan
2014-05-20 14:22         ` Tejun Heo
2014-05-20 14:23         ` Tejun Heo
2014-05-20 14:24         ` Tejun Heo
2014-05-20  9:46       ` [PATCH V3 04/10] workqueue: destroy worker directly in the idle timeout handler Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 05/10] workqueue: separate iteration role from worker_idr Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 06/10] workqueue: convert worker_idr to worker_ida Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 07/10] workqueue: narrow the protection range of manager_mutex Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 08/10] workqueue: rename manager_mutex to attach_mutex Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 09/10] workqueue: separate pool-attaching code out from create_worker() Lai Jiangshan
2014-05-20  9:46       ` [PATCH V3 10/10] workqueue: use generic attach/detach routine for rescuers Lai Jiangshan
2014-05-20 15:00       ` [PATCH V3 00/10] workqueue: async worker destruction and worker attaching/detaching Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5371BEFA.9060902@cn.fujitsu.com \
    --to=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.