public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: jiangshanlai@gmail.com
Cc: torvalds@linux-foundation.org, linux-kernel@vger.kernel.org,
	allen.lkml@gmail.com, kernel-team@meta.com
Subject: [PATCHSET wq/for-6.9,6.10] workqueue: Implement disable/enable_work()
Date: Fri, 16 Feb 2024 08:04:41 -1000	[thread overview]
Message-ID: <20240216180559.208276-1-tj@kernel.org> (raw)

Hello,

4cb1ef64609f ("workqueue: Implement BH workqueues to eventually replace
tasklets") implemented workqueues that execute work items in the BH context
with the goal of eventually replacing tasklet.

While the existing workqueue API covers the basic queueing and canceling
operations, tasklet also has tasklet_disable*() which blocks the execution
of the tasklet until it's re-enabled with tasklet_enable(). The interface if
fairly widely used and workqueue currently doesn't have a counterpart.

This patchset implements disable/enable_work() and the delayed_work
counterparts to address the gap. The ability to block future executions is
something which some users asked for in the past, and, while not essential
as the caller can and often has to shutdown the queuer anyway, it's a nice
convenience to have. Also, timer grew a similar feature recently with
timer_shutdown().

- tasklet_disable() keeps disable depth so that a tasklet can be disabled
  and re-enabled by multiple parties at the same time. Workqueue is updated
  to carry 16bit disable count in work->data while the work item is not
  queued. When non-zero, attempts to queue the work item fail.

- The cancel_work_sync() path used WORK_OFFQ_CANCELING to synchronize
  multiple cancel_sync attempts. This added a completely separate wait
  mechanism which wasn't very congruent with the rest of workqueue. This was
  because the canceling state was carried in a way which couldn't
  accommodate multiple concurrent uses. This mechanism is replaced by
  disable - cancel_sync now simply disables the work item, flushes and
  re-enables it.

- There is a wart in how tasklet_disable/enable() works. When a tasklet is
  disabled, if the tasklet is queued, it keeps the softirq constantly raised
  until the tasklet is re-enabled and executed. This makes disabling
  unnecessarily expensive and brittle. The only busy looping workqueue's
  implementation does is on the party that's trying to cancel_sync or
  disable_sync to wait for the completion of the currently executing
  instance, which should be safe as long as it's from process and BH
  contexts.

- A disabled tasklet remembers whether it was queued while disabled and
  starts executing when re-enabled. It turns out doing this with work items
  is challenging as there are a lot more to remember and the synchronization
  becomes more complicated too. Looking at the use cases and given the
  continuity from how cancel_work_sync() works, it seems better to just
  ignore queueings which happen while a work item is disabled and require
  the users to explicitly queue the work item after re-enabling as
  necessary. Most users should be able to re-enqueue unconditionally after
  enabling.

This patchset is on top of wq/for-6.9 and contains the following 17 patches:

 0001-workqueue-Cosmetic-changes.patch
 0002-workqueue-Use-rcu_read_lock_any_held-instead-of-rcu_.patch
 0003-workqueue-Rename-__cancel_work_timer-to-__cancel_tim.patch
 0004-workqueue-Reorganize-flush-and-cancel-_sync-function.patch
 0005-workqueue-Use-variable-name-irq_flags-for-saving-loc.patch
 0006-workqueue-Introduce-work_cancel_flags.patch
 0007-workqueue-Clean-up-enum-work_bits-and-related-consta.patch
 0008-workqueue-Factor-out-work_grab_pending-from-__cancel.patch
 0009-workqueue-Remove-clear_work_data.patch
 0010-workqueue-Make-flags-handling-consistent-across-set_.patch
 0011-workqueue-Preserve-OFFQ-bits-in-cancel-_sync-paths.patch
 0012-workqueue-Implement-disable-enable-for-delayed-work-.patch
 0013-workqueue-Remove-WORK_OFFQ_CANCELING.patch
 0014-workqueue-Remember-whether-a-work-item-was-on-a-BH-w.patch
 0015-workqueue-Update-how-start_flush_work-is-called.patch
 0016-workqueue-Allow-cancel_work_sync-and-disable_work-fr.patch
 0017-r8152-Convert-from-tasklet-to-BH-workqueue.patch

0001-0010 are cleanup and prep patches with the only functional change being
the use of rcu_read_lock_any_held() instead of rcu_read_lock() in 0002. I'll
apply them to wq/for-6.9 unless there are objections. I thought about making
these a separate patch series but the cleanups make more sense as a part of
this series.

For the rest of the series, given how many invasive workqueue changes are
already queued for v6.9 and the subtle nature of these patches, I think it'd
be best to defer them to the one after that so that we can use v6.9 as an
intermediate verification point.

0011-0012 implement disable_work() and enable_work(). At this stage, all
disable[_sync]_work and enable_work operations might_sleep(). disable_work()
and enable_work() due to CANCELING synchronization described above.
disable_work_sync() also needs to wait for the in-flight work item to finish
which requires blocking.

0013 replaces CANCELING with internal use of disble/enable. This removes one
ugliness from workqueue code and allows disable_work() and enable_work() to
be used from atomic contexts.

0014-0016 implement busy-wait for BH work items when they're being canceled
thus allowing cancel_work_sync() and disable_work_sync() to be called from
atomic contexts for them. This makes workqueue interface a superset of
tasklet and also makes BH workqueues easier to live with.

0017 converts drivers/net/r8152.c from tasklet to BH workqueue as a
demonstration. It seems to work fine.

The patchset is on top of wq/for-6.9 fd0a68a2337b ("workqueue, irq_work:
Build fix for !CONFIG_IRQ_WORK") and also available in the following git
branch:

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git disable_work-v1

diffstat follows. Thanks.

 drivers/net/usb/r8152.c   |   44 ++--
 include/linux/workqueue.h |   69 ++++---
 kernel/workqueue.c        |  623 ++++++++++++++++++++++++++++++++++++++++----------------------------
 3 files changed, 441 insertions(+), 295 deletions(-)

--
tejun

             reply	other threads:[~2024-02-16 18:06 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-16 18:04 Tejun Heo [this message]
2024-02-16 18:04 ` [PATCH 01/17] workqueue: Cosmetic changes Tejun Heo
2024-02-16 18:04 ` [PATCH 02/17] workqueue: Use rcu_read_lock_any_held() instead of rcu_read_lock_held() Tejun Heo
2024-02-16 18:04 ` [PATCH 03/17] workqueue: Rename __cancel_work_timer() to __cancel_timer_sync() Tejun Heo
2024-02-16 18:04 ` [PATCH 04/17] workqueue: Reorganize flush and cancel[_sync] functions Tejun Heo
2024-02-16 18:04 ` [PATCH 05/17] workqueue: Use variable name irq_flags for saving local irq flags Tejun Heo
2024-02-16 18:04 ` [PATCH 06/17] workqueue: Introduce work_cancel_flags Tejun Heo
2024-02-16 18:04 ` [PATCH 07/17] workqueue: Clean up enum work_bits and related constants Tejun Heo
2024-02-16 18:04 ` [PATCH 08/17] workqueue: Factor out work_grab_pending() from __cancel_work_sync() Tejun Heo
2024-02-16 18:04 ` [PATCH 09/17] workqueue: Remove clear_work_data() Tejun Heo
2024-02-16 18:04 ` [PATCH 10/17] workqueue: Make @flags handling consistent across set_work_data() and friends Tejun Heo
2024-02-16 18:04 ` [PATCH 11/17] workqueue: Preserve OFFQ bits in cancel[_sync] paths Tejun Heo
2024-02-16 18:04 ` [PATCH 12/17] workqueue: Implement disable/enable for (delayed) work items Tejun Heo
2024-02-20  7:22   ` Lai Jiangshan
2024-02-20 18:38     ` Tejun Heo
2024-02-21  2:54       ` Lai Jiangshan
2024-02-21  5:47         ` Tejun Heo
2024-02-26  3:42   ` Boqun Feng
2024-02-26 18:30     ` Tejun Heo
2024-02-16 18:04 ` [PATCH 13/17] workqueue: Remove WORK_OFFQ_CANCELING Tejun Heo
2024-02-20  7:23   ` Lai Jiangshan
2024-02-20 18:53     ` Tejun Heo
2024-02-16 18:04 ` [PATCH 14/17] workqueue: Remember whether a work item was on a BH workqueue Tejun Heo
2024-02-16 18:04 ` [PATCH 15/17] workqueue: Update how start_flush_work() is called Tejun Heo
2024-02-16 18:04 ` [PATCH 16/17] workqueue: Allow cancel_work_sync() and disable_work() from atomic contexts on BH work items Tejun Heo
2024-02-20  7:33   ` Lai Jiangshan
2024-02-20 20:01     ` Tejun Heo
2024-02-16 18:04 ` [PATCH 17/17] r8152: Convert from tasklet to BH workqueue Tejun Heo
2024-02-20 17:33 ` [PATCHSET wq/for-6.9,6.10] workqueue: Implement disable/enable_work() Allen
2024-02-20 20:25 ` Tejun Heo
2024-02-20 21:38   ` Allen
2024-02-22 20:26     ` Allen
2024-02-26 18:38       ` Tejun Heo
2024-02-27 17:56         ` Allen
2024-02-21  2:39   ` Lai Jiangshan
2024-02-21  5:39     ` Tejun Heo
2024-02-21 17:03       ` Allen
2024-02-25 17:40         ` Kees Cook
2024-02-26 17:30           ` Allen
2024-02-26 18:30             ` Tejun Heo

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=20240216180559.208276-1-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=allen.lkml@gmail.com \
    --cc=jiangshanlai@gmail.com \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.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