From: Lai Jiangshan <jiangshanlai@gmail.com>
To: Boqun Feng <boqun.feng@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>, Tejun Heo <tj@kernel.org>,
"Paul E . McKenney" <paulmck@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Peter Zijlstra <peterz@infradead.org>,
Frederic Weisbecker <frederic@kernel.org>
Subject: Re: [RFC 2/2] workqueue: Fix work re-entrance when requeue to a different workqueue
Date: Sat, 9 Oct 2021 10:06:23 +0800 [thread overview]
Message-ID: <CAJhGHyDudet_xyNk=8xnuO2==o-u06s0E0GZVP4Q67nmQ84Ceg@mail.gmail.com> (raw)
In-Reply-To: <20211008100454.2802393-3-boqun.feng@gmail.com>
On Fri, Oct 8, 2021 at 6:06 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> When requeuing a work to a different workqueue while it's still getting
> processed, re-entrace as the follow can happen:
>
> { both WQ1 and WQ2 are bounded workqueue, and a work W has been
> queued on CPU0 for WQ1}
>
> CPU 0 CPU 1
> ===== ====
> <In worker on CPU 0>
> process_one_work():
> ...
> // pick up W
> worker->current_work = W;
> worker->current_func = W->func;
> ...
> set_work_pool_and_clear_pending(...);
> // W can be requeued afterwards
> queue_work_on(1, WQ2, W):
> if (!test_and_set_bit(...)) {
> // this branch is taken, as CPU 0
> // just clears pending bit.
> __queue_work(...):
> pwq = <pool for CPU1 of WQ2>;
> last_pool = <pool for CPU 0 of WQ1>;
> if (last_pool != pwq->pool) { // true
> if (.. && worker->current_pwq->wq == wq) {
> // false, since @worker is a
> // a worker of @last_pool (for
> // WQ1), and @wq is WQ2.
> }
> ...
> insert_work(pwq, W, ...);
> }
> // W queued.
> <schedule to worker on CPU 1>
> process_one_work():
> collision = find_worker_executing_work(..);
> // NULL, because we're searching the
> // worker pool of CPU 1, while W is
> // the current work on worker pool of
> // CPU 0.
> worker->current_work = W;
> worker->current_func = W->func;
> worker->current_func(...);
> ...
> worker->current_func(...); // Re-entrance
Concurrent or parallel executions on the same work item aren't
considered as "Re-entrance" if the workqueue is changed.
It allows the work function to free itself(the item) and another
subsystem allocates the same item and reuses it.
"Re-entrance" is defined as:
work function has not been changed
wq has not been changed
the item has not been reinitiated.
(To reduce the check complication, the workqueue subsystem often
considers it "Re-entrance" if the condition is changed and has changed
back. But the wq users should not depend on this behavior and should avoid
it)
>
> This issue is already partially fixed because in queue_work_on(),
> last_pool can be used to queue the work, as a result the requeued work
> processing will find the collision and wait for the existing one to
> finish. However, currently the last_pool is only used when two
> workqueues are the same one, which causes the issue. Therefore extend
> the behavior to allow last_pool to requeue the work W even if the
> workqueues are different. It's safe to do this since the work W has been
> proved safe to queue and run on the last_pool.
>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
> kernel/workqueue.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 1418710bffcd..410141cc5f88 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1465,7 +1465,7 @@ static void __queue_work(int cpu, struct workqueue_struct *wq,
>
> worker = find_worker_executing_work(last_pool, work);
>
> - if (worker && worker->current_pwq->wq == wq) {
> + if (worker) {
> pwq = worker->current_pwq;
> } else {
> /* meh... not running there, queue here */
> --
> 2.32.0
>
next prev parent reply other threads:[~2021-10-09 2:06 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-08 10:04 [RFC 0/2] Re-entrace of a work when requeued to a different workqueue Boqun Feng
2021-10-08 10:04 ` [RFC 1/2] NOT FOR MERGE: A selftest shows that re-entrance can happen Boqun Feng
2021-10-08 10:04 ` [RFC 2/2] workqueue: Fix work re-entrance when requeue to a different workqueue Boqun Feng
2021-10-09 2:06 ` Lai Jiangshan [this message]
2021-10-09 3:21 ` Boqun Feng
2021-10-09 15:06 ` Boqun Feng
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='CAJhGHyDudet_xyNk=8xnuO2==o-u06s0E0GZVP4Q67nmQ84Ceg@mail.gmail.com' \
--to=jiangshanlai@gmail.com \
--cc=boqun.feng@gmail.com \
--cc=frederic@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).