linux-kernel - Re: [PATCH 06/10 V2] workqueue: convert worker_idr to worker

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 13 May 2014 10:17:52 -0400
From:	Tejun Heo <tj@...nel.org>
To:	Lai Jiangshan <laijs@...fujitsu.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 06/10 V2] workqueue: convert worker_idr to worker_ida

Hello,

On Tue, May 13, 2014 at 02:43:06PM +0800, Lai Jiangshan wrote:
> 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.

This is completely arbitrary.  For example, if rescuers needed IDs to
be allocated and deallocated on attach/detach, ID allocation should be
included in the above two functions, right?  This makes sense only
because rescuers don't have IDs and we're gonna use the above
functions for the rescuers too.  There's nothing inherent in
decoupling a worker's attachment to its pool and its pool ID
allocation.  The design developed this way only because there's
certain specific usage for it.  The code is fine but it usually is a
lot more helpful for reviewing and later reference if you actually
explain why things are done in certain specific ways because in
isolation the above decoupling is completely arbitrary.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/