All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lai Jiangshan <laijs@cn.fujitsu.com>
To: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/9 V3] workqueue: add non_manager_role_manager_mutex_unlock()
Date: Thu, 30 Aug 2012 17:16:01 +0800	[thread overview]
Message-ID: <503F2F51.8000301@cn.fujitsu.com> (raw)
In-Reply-To: <20120829182510.GB2258@dhcp-172-17-108-109.mtv.corp.google.com>

On 08/30/2012 02:25 AM, Tejun Heo wrote:
> On Thu, Aug 30, 2012 at 12:51:55AM +0800, Lai Jiangshan wrote:
>> If hotplug code grabbed the manager_mutex and worker_thread try to create
>> a worker, the manage_worker() will return false and worker_thread go to
>> process work items. Now, on the CPU, all workers are processing work items,
>> no idle_worker left/ready for managing. It breaks the concept of workqueue
>> and it is bug.
>>
>> So when this case happens, the last idle should not go to process work,
>> it should go to sleep as usual and wait normal events. but it should
>> also be notified by the event that hotplug code release the manager_mutex.
>>
>> So we add non_manager_role_manager_mutex_unlock() to do this notify.
> 
> Hmmm... how about just running rebind_workers() from a work item?
> That way, it would be guaranteed that there alwyas will be an extra
> worker available on rebind completion.
> 
> Thanks.
> 

gcwq_unbind_fn() is unsafe even it is called from a work item.
so we need non_manager_role_manager_mutex_unlock().

If rebind_workers() is called from a work item, it is safe when there is
no CPU_INTENSIVE items. but we can't disable CPU_INTENSIVE items,
so it is still unsafe, we need non_manager_role_manager_mutex_unlock() too.

non_manager_role_manager_mutex_unlock() approach is good to fix it.
I'm writing V4 patch/approach to fix it too, it is a little more complicated,
but it has some benefit over non_manager_role_manager_mutex_unlock() approach.

Thanks.
Lai

  reply	other threads:[~2012-08-30  9:14 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-29 16:51 [PATCH 0/9 V3] workqueue: fix and cleanup hotplug/rebind_workers() Lai Jiangshan
2012-08-29 16:51 ` [PATCH 1/9 V3] workqueue: ensure the wq_worker_sleeping() see the right flags Lai Jiangshan
2012-08-29 16:51 ` [PATCH 2/9 V3] workqueue: fix deadlock in rebind_workers() Lai Jiangshan
2012-08-29 16:51 ` [PATCH 3/9 V3] workqueue: add POOL_MANAGING_WORKERS Lai Jiangshan
2012-08-29 18:21   ` Tejun Heo
2012-08-30  2:38     ` Lai Jiangshan
2012-08-29 16:51 ` [PATCH 4/9 V3] workqueue: add non_manager_role_manager_mutex_unlock() Lai Jiangshan
2012-08-29 18:25   ` Tejun Heo
2012-08-30  9:16     ` Lai Jiangshan [this message]
2012-08-30  9:17       ` Tejun Heo
2012-08-31  1:08         ` Lai Jiangshan
2012-08-29 16:51 ` [PATCH 5/9 V3] workqueue: move rebind_hold to idle_rebind Lai Jiangshan
2012-08-29 16:51 ` [PATCH 6/9 V3] workqueue: simple clear WORKER_REBIND Lai Jiangshan
2012-08-29 16:51 ` [PATCH 7/9 V3] workqueue: explicit way to wait for idles workers to finish Lai Jiangshan
2012-08-29 18:34   ` Tejun Heo
2012-08-29 16:51 ` [PATCH 8/9 V3] workqueue: single pass rebind_workers Lai Jiangshan
2012-08-29 18:40   ` Tejun Heo
2012-08-29 16:52 ` [PATCH 9/9 V3] workqueue: merge the role of rebind_hold to idle_done 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=503F2F51.8000301@cn.fujitsu.com \
    --to=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@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.