Messages in this thread | | | Date | Sun, 9 Sep 2012 12:09:58 +0800 | Subject | Re: [PATCH 4/7 V6] workqueue: fix idle worker depletion | From | Lai Jiangshan <> |
| |
On Sun, Sep 9, 2012 at 3:02 AM, Tejun Heo <tj@kernel.org> wrote: > Hello, Lai. > > On Sun, Sep 09, 2012 at 02:34:02AM +0800, Lai Jiangshan wrote: >> in 3.6 busy_worker_rebind() handle WORKER_REBIND bit, >> not WORKER_UNBOUND bit. >> >> busy_worker_rebind() takes struct work_struct *work argument, we have to >> add new patch to add a helper and restruct it at first. > > What's wrong with just treating manager as busy. Factor out, > rebind_work scheduling from rebind_workers() and call it for busy > workers and the manager if it exists. manage_workers() only need to > call process_scheduled_works(). Wouldn't that work? > >> worker_maybe_bind_and_lock() 's mean is very clear >> here. busy_worker_rebind() seems for busy workers, manager is not >> busy workers. > > I don't know. It just seems unnecessarily wordy. If you don't like > reusing the busy worker path, how about just calling > maybe_bind_and_lock() unconditionally after locking manager_mutex? I > mean, can't it just do the following? > > spin_unlock_irq(&gcwq->lock); > > /* > * Explain what's going on. > */ > mutex_lock(&pool->manager_mutex); > if (worker_maybe_bind_and_lock(worker)) > worker_clr_flags(worker, WORKER_UNBOUND); > ret = true; >
This code is correct. worker_maybe_bind_and_lock() can be called any time. but I like to call it only when it is really needed.
Thanks. Lai
| |