Hello, On Tue, May 13, 2014 at 02:32:52PM +0800, Lai Jiangshan wrote: > >> + 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.
Yeah, it's shared by rescuer later, so it's fine but, again, it probably helps to mention that it's planned to do so; otherwise, the rationale is kinda weak and what belongs to that function is rather arbitrary. > >> /* > >> * 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. Hmmm... I think it'd be better to spell it out then. The single sentence is kinda cryptic especially because the two verbs in the sentence don't have the same subject (managee_workers() can't enter idle). 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/
