Messages in this thread | | | Date | Tue, 13 May 2014 10:14:18 -0400 | From | Tejun Heo <> | Subject | Re: [PATCH 03/10 V2] workqueue: async worker destruction |
| |
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
| |