On 09/05/2012 08:54 AM, Tejun Heo wrote:
> How about something like the following? This is more consistent with
> the existing code and as the fixes need to go separately through
> for-3.6-fixes, it's best to stay consistent regardless of the end
> result after all the restructuring. It's not tested yet. If you
> don't object, I'll split it into two patches, test and route them
> through for-3.6-fixes w/ your Original-patch-by.
>
I see that this patch's idea is same as mine but reuses
@idle_rebind.cnt and @idle_rebind.done.
I don't think it is consistent to avoid adding new field
and to reuse old field for different purpose
Thanks
Lai
> Thanks.
> ---
> kernel/workqueue.c | 51 ++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 38 insertions(+), 13 deletions(-)
>
> Index: work/kernel/workqueue.c
> ===================================================================
> --- work.orig/kernel/workqueue.c
> +++ work/kernel/workqueue.c
> @@ -1326,6 +1326,15 @@ static void idle_worker_rebind(struct wo
>
> /* we did our part, wait for rebind_workers() to finish up */
> wait_event(gcwq->rebind_hold, !(worker->flags & WORKER_REBIND));
> +
> + /*
> + * rebind_workers() shouldn't finish until all workers passed the
> + * above WORKER_REBIND wait. Tell it when done.
> + */
> + spin_lock_irq(&worker->pool->gcwq->lock);
> + if (!--worker->idle_rebind->cnt)
> + complete(&worker->idle_rebind->done);
> + spin_unlock_irq(&worker->pool->gcwq->lock);
> }
>
> /*
> @@ -1422,19 +1431,7 @@ retry:
> goto retry;
> }
>
> - /*
> - * All idle workers are rebound and waiting for %WORKER_REBIND to
> - * be cleared inside idle_worker_rebind(). Clear and release.
> - * Clearing %WORKER_REBIND from this foreign context is safe
> - * because these workers are still guaranteed to be idle.
> - */
> - for_each_worker_pool(pool, gcwq)
> - list_for_each_entry(worker, &pool->idle_list, entry)
> - worker->flags &= ~WORKER_REBIND;
> -
> - wake_up_all(&gcwq->rebind_hold);
don't need to move down.
> -
> - /* rebind busy workers */
> + /* all idle workers are rebound, rebind busy workers */
> for_each_busy_worker(worker, i, pos, gcwq) {
> struct work_struct *rebind_work = &worker->rebind_work;
> unsigned long worker_flags = worker->flags;
> @@ -1454,6 +1451,34 @@ retry:
> worker->scheduled.next,
> work_color_to_flags(WORK_NO_COLOR));
> }
> +
> + /*
> + * All idle workers are rebound and waiting for %WORKER_REBIND to
> + * be cleared inside idle_worker_rebind(). Clear and release.
> + * Clearing %WORKER_REBIND from this foreign context is safe
> + * because these workers are still guaranteed to be idle.
> + *
> + * We need to make sure all idle workers passed WORKER_REBIND wait
> + * in idle_worker_rebind() before returning; otherwise, workers can
> + * get stuck at the wait if hotplug cycle repeats.
> + */
> + idle_rebind.cnt = 1;
> + INIT_COMPLETION(idle_rebind.done);
> +
> + for_each_worker_pool(pool, gcwq) {
> + list_for_each_entry(worker, &pool->idle_list, entry) {
> + worker->flags &= ~WORKER_REBIND;
> + idle_rebind.cnt++;
> + }
> + }
> +
> + wake_up_all(&gcwq->rebind_hold);
> +
> + if (--idle_rebind.cnt) {
> + spin_unlock_irq(&gcwq->lock);
> + wait_for_completion(&idle_rebind.done);
> + spin_lock_irq(&gcwq->lock);
> + }
> }
>
> static struct worker *alloc_worker(void)
>
--
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/
