All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 02/11 V5] workqueue: async idle rebinding
Date: Wed, 5 Sep 2012 11:06:23 -0700	[thread overview]
Message-ID: <20120905180623.GA13737@google.com> (raw)
In-Reply-To: <1346841475-4422-3-git-send-email-laijs@cn.fujitsu.com>

Hello, Lai.

Ooh, I like the approach.  That said, I think it's a bit too invasive
for 3.6-fixes.  I'll merge the two patches I posted yesterday in
3.6-fixes.  Let's do this restructuring in for-3.7.

On Wed, Sep 05, 2012 at 06:37:39PM +0800, Lai Jiangshan wrote:
>  static void idle_worker_rebind(struct worker *worker)
>  {
>  	struct global_cwq *gcwq = worker->pool->gcwq;
>  
> -	/* CPU must be online at this point */
> -	WARN_ON(!worker_maybe_bind_and_lock(worker));
> -	if (!--worker->idle_rebind->cnt)
> -		complete(&worker->idle_rebind->done);
> -	spin_unlock_irq(&worker->pool->gcwq->lock);
> +	if (worker_maybe_bind_and_lock(worker))
> +		worker_clr_flags(worker, WORKER_UNBOUND);
>  
> -	/* we did our part, wait for rebind_workers() to finish up */
> -	wait_event(gcwq->rebind_hold, !(worker->flags & WORKER_REBIND));
> +	worker_clr_flags(worker, WORKER_REBIND);
> +	list_add(&worker->entry, &worker->pool->idle_list);
> +	spin_unlock_irq(&gcwq->lock);

This looks correct to me but it's still a bit scary.  Some comments
explaining why the above is correct would be nice.

Yeah, other than that, looks good to me.  I'll prepare new for-3.7
branch this can be based on, so please wait a bit.  Also, I think I'll
probably update commit description / comments while committing.

Thanks!

-- 
tejun

  reply	other threads:[~2012-09-05 18:06 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-05 10:37 [PATCH 00/11 V5] workqueue: reimplement unbind/rebind Lai Jiangshan
2012-09-05 10:37 ` [PATCH 01/11 V5] workqueue: ensure the wq_worker_sleeping() see the right flags Lai Jiangshan
2012-09-05 10:37 ` [PATCH 02/11 V5] workqueue: async idle rebinding Lai Jiangshan
2012-09-05 18:06   ` Tejun Heo [this message]
2012-09-06  1:28     ` Lai Jiangshan
2012-09-05 10:37 ` [PATCH 03/11 V5] workqueue: new day don't need WORKER_REBIND for busy rebinding Lai Jiangshan
2012-09-05 18:31   ` Tejun Heo
2012-09-06  2:10     ` Lai Jiangshan
2012-09-05 10:37 ` [PATCH 04/11 V5] workqueue: remove WORKER_REBIND Lai Jiangshan
2012-09-05 10:37 ` [PATCH 05/11 V5] workqueue: Add @bind arguement back without change any thing Lai Jiangshan
2012-09-05 19:49   ` Tejun Heo
2012-09-06  1:04     ` Lai Jiangshan
2012-09-06 16:51       ` Tejun Heo
2012-09-07  2:11         ` Lai Jiangshan
2012-09-07 19:37           ` Tejun Heo
2012-09-05 10:37 ` [PATCH 06/11 V5] workqueue: unbind manager Lai Jiangshan
2012-09-05 10:37 ` [PATCH 07/11 V5] workqueue: rebind manager Lai Jiangshan
2012-09-05 10:37 ` [PATCH 08/11 V5] workqueue: unbind newly created worker Lai Jiangshan
2012-09-05 10:37 ` [PATCH 09/11 V5] workqueue: rebind " Lai Jiangshan
2012-09-05 16:19   ` Lai Jiangshan
2012-09-05 10:37 ` [PATCH 10/11 V5] workqueue: unbind/rebind without manager_mutex Lai Jiangshan
2012-09-05 20:04   ` Tejun Heo
2012-09-06 10:44     ` Lai Jiangshan
2012-09-06 17:00       ` Tejun Heo
2012-09-05 10:37 ` [PATCH 11/11 V5] workqueue: remove manager_mutex Lai Jiangshan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120905180623.GA13737@google.com \
    --to=tj@kernel.org \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.