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 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/
