All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: linux-kernel@vger.kernel.org, Lai Jiangshan <laijs@linux.alibaba.com>
Subject: Re: [RFC PATCH 7/7] workqueue: Replace pool lock with preemption disabling in wq_worker_sleeping()
Date: Thu, 9 Dec 2021 12:14:33 -1000	[thread overview]
Message-ID: <YbJ/yR6KMjhv9EfS@slm.duckdns.org> (raw)
In-Reply-To: <20211207073543.61092-8-jiangshanlai@gmail.com>

Hello,

On Tue, Dec 07, 2021 at 03:35:43PM +0800, Lai Jiangshan wrote:
> From: Lai Jiangshan <laijs@linux.alibaba.com>
> 
> Once upon a time,  wq_worker_sleeping() was called with rq lock held,
> so wq_worker_sleeping() can not use pool lock.  Instead it used "X:"
> protection: preemption disabled on local cpu and wq_worker_sleeping()
> didn't depend on rq lock to work even with it held.
> 
> Now, wq_worker_sleeping() isn't called with rq lock held and is using
> pool lock.  But the functionality of "X:" protection isn't removed and
> wq_worker_running() is still using it.
> 
> So we can also use "X:" protection in wq_worker_sleeping() and avoid
> locking the pool lock.
> 
> This patch also documents that only idle_list.next is under "X:"
> protection, not the whole idle_list because destroy_worker() in idle
> timer can remove non-first idle workers.  Idle timer can be possible
> strayed in different CPU, or even in the same CPU, it can interrupt
> preemption disabled context.

It's nice to go back to not needing to grab pool lock in the worker sleeping
path but I'm not sure it actually matters. This isn't in a red-hot path and
we're touching a bunch of stuff in the pool anyway, so the overhead of
grabbing a lock which likely isn't too contended shouldn't matter all that
much. So, maybe it'd be better to just keep things simple?

Thanks.

-- 
tejun

      reply	other threads:[~2021-12-09 22:14 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-07  7:35 [PATCH 0/7] workqueue: cleanups for schedule callbacks Lai Jiangshan
2021-12-07  7:35 ` [PATCH 1/7] workqueue: Remove the outdated comment before wq_worker_sleeping() Lai Jiangshan
2021-12-07  7:35 ` [PATCH 2/7] workqueue: Remove the advanced kicking of the idle workers in rebind_workers() Lai Jiangshan
2021-12-07  7:35 ` [PATCH 3/7] workqueue: Remove outdated comment about exceptional workers in unbind_workers() Lai Jiangshan
2021-12-09 22:16   ` Tejun Heo
2021-12-07  7:35 ` [PATCH 4/7] workqueue: Remove schedule() " Lai Jiangshan
2021-12-07  7:35 ` [PATCH 5/7] workqueue: Move the code of waking a worker up " Lai Jiangshan
2021-12-07  7:35 ` [PATCH 6/7] workqueue: Remove the cacheline_aligned for nr_running Lai Jiangshan
2021-12-09 22:07   ` Tejun Heo
2021-12-09 23:31     ` Lai Jiangshan
2021-12-09 22:27   ` Tejun Heo
2021-12-07  7:35 ` [RFC PATCH 7/7] workqueue: Replace pool lock with preemption disabling in wq_worker_sleeping() Lai Jiangshan
2021-12-09 22:14   ` Tejun Heo [this message]

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=YbJ/yR6KMjhv9EfS@slm.duckdns.org \
    --to=tj@kernel.org \
    --cc=jiangshanlai@gmail.com \
    --cc=laijs@linux.alibaba.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.