From: Tejun Heo <tj@kernel.org>
To: linux-kernel@vger.kernel.org
Subject: [PATCHSET] workqueue: don't use [delayed_]work_pending()
Date: Fri, 21 Dec 2012 17:56:50 -0800 [thread overview]
Message-ID: <1356141435-17340-1-git-send-email-tj@kernel.org> (raw)
Hello,
Given the current set of workqueue APIs, there are very few cases
where [delayed_]work_pending() are actually necessary; however, it's
seemingly somewhat popular for a few purposes including skipping
queue/flush/cancel depending on the current state for optimization.
work_pending() could be slightly cheaper than performing the actual
operation because it can skip atomic bitops assuming that the user is
synchronizing against other workqueue operations properly; however,
most paths with this type of optimization are siberia-cold for this
level of optimization to matter - e.g. driver detach path or parameter
update via sysfs - and it's easy to get it subtly wrong and introduce
difficult-to-trigger race conditions. It just isn't worth it.
Other use cases include using work_pending() state to decide the state
of previously scheduled async action. This too, unfortunately, seems
easy to get wrong. Several users forgot that work_pending() becomes
false *before* the work item starts execution and fails to synchronize
with on-going execution. Unless one is specifically looking for
those, they can be tricky to spot.
Overall, [delayed_]work_pending() seem to bring more troubles than
benefits and not using them usually results in better code. This
patchset removes [delayed_]work_pending() usages from various
subsystems. A lot are straight-forward removal of unnecessary
optimizations. Some fix bugs around work item handling. Others
restructure code so that [delayed_]work_pending() isn't necessary.
After this patchset, there remain a handful of
[delayed_]work_pending() users. Some of them legit. Some quite
broken. Hopefully, they can be converted too and we can unexport
these easy-to-misuse interface.
This patchset contains the following 25 patches.
0001-charger_manager-don-t-use-delayed_-work_pending.patch
0002-ab8500_charger-don-t-use-delayed_-work_pending.patch
0003-sja1000-don-t-use-delayed_-work_pending.patch
0004-ipw2x00-simplify-scan_event-handling.patch
0005-devfreq-don-t-use-delayed_-work_pending.patch
0006-libertas-don-t-use-delayed_-work_pending.patch
0007-mwifiex-don-t-use-delayed_-work_pending.patch
0008-thinkpad_acpi-don-t-use-delayed_-work_pending.patch
0009-wl1251-don-t-use-delayed_-work_pending.patch
0010-kprobes-fix-wait_for_kprobe_optimizer.patch
0011-pm-don-t-use-delayed_-work_pending.patch
0012-bluetooth-l2cap-don-t-use-delayed_-work_pending.patch
0013-sound-wm8350-don-t-use-delayed_-work_pending.patch
0014-rfkill-don-t-use-delayed_-work_pending.patch
0015-x86-mce-don-t-use-delayed_-work_pending.patch
0016-PM-Domains-don-t-use-delayed_-work_pending.patch
0017-wm97xx-don-t-use-delayed_-work_pending.patch
0018-TMIO-MMC-don-t-use-delayed_-work_pending.patch
0019-net-caif-don-t-use-delayed_-work_pending.patch
0020-wimax-i2400m-fix-i2400m-wake_tx_skb-handling.patch
0021-tty-max3100-don-t-use-delayed_-work_pending.patch
0022-usb-at91_udc-don-t-use-delayed_-work_pending.patch
0023-video-exynos-don-t-use-delayed_-work_pending.patch
0024-debugobjects-don-t-use-delayed_-work_pending.patch
0025-ipc-don-t-use-delayed_-work_pending.patch
And available in the following git branch.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git review-work_pending-cleanup
diffstat follows.
arch/x86/kernel/cpu/mcheck/mce.c | 10 ++--------
drivers/base/power/domain.c | 3 +--
drivers/devfreq/devfreq.c | 3 +--
drivers/input/touchscreen/wm97xx-core.c | 4 +---
drivers/mmc/host/tmio_mmc_pio.c | 3 +--
drivers/net/caif/caif_shmcore.c | 15 +++++----------
drivers/net/can/sja1000/peak_pci.c | 3 +--
drivers/net/wimax/i2400m/netdev.c | 31 +++++++++++++++++--------------
drivers/net/wireless/ipw2x00/ipw2100.c | 31 ++++++++-----------------------
drivers/net/wireless/ipw2x00/ipw2100.h | 3 +--
drivers/net/wireless/ipw2x00/ipw2200.c | 13 +++----------
drivers/net/wireless/libertas/cfg.c | 2 +-
drivers/net/wireless/libertas/if_sdio.c | 9 ++++-----
drivers/net/wireless/mwifiex/sdio.c | 9 ++++-----
drivers/net/wireless/ti/wl1251/ps.c | 3 +--
drivers/platform/x86/thinkpad_acpi.c | 3 +--
drivers/power/ab8500_charger.c | 13 ++++---------
drivers/power/charger-manager.c | 31 ++++++++++++++++---------------
drivers/tty/serial/max3100.c | 3 +--
drivers/usb/gadget/at91_udc.c | 3 +--
drivers/video/exynos/exynos_dp_core.c | 6 ++----
include/net/bluetooth/l2cap.h | 24 ++++++++++++++++--------
ipc/util.c | 3 +--
kernel/kprobes.c | 23 +++++++++++++++--------
kernel/power/autosleep.c | 2 +-
kernel/power/qos.c | 9 +++------
lib/debugobjects.c | 7 +++----
net/bluetooth/l2cap_core.c | 7 +++----
net/rfkill/input.c | 8 +++-----
sound/soc/codecs/wm8350.c | 10 ++++------
30 files changed, 125 insertions(+), 169 deletions(-)
Thanks.
--
tejun
next reply other threads:[~2012-12-22 1:57 UTC|newest]
Thread overview: 83+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-22 1:56 Tejun Heo [this message]
2012-12-22 1:56 ` [PATCH 01/25] charger_manager: don't use [delayed_]work_pending() Tejun Heo
2013-01-05 22:11 ` Anton Vorontsov
2012-12-22 1:56 ` [PATCH 02/25] ab8500_charger: " Tejun Heo
[not found] ` <CACRpkdYE3F22rnW+ZRhOWT4VGRhG2CSnb7Rj3gdCosTK8wLQmA@mail.gmail.com>
[not found] ` <50EAA53F.1000304@stericsson.com>
2013-01-07 10:48 ` Arun MURTHY
2013-01-08 14:30 ` Linus Walleij
2012-12-22 1:56 ` [PATCH 03/25] sja1000: " Tejun Heo
2012-12-22 8:01 ` David Miller
2012-12-28 21:40 ` Tejun Heo
2012-12-22 1:56 ` [PATCH 04/25] ipw2x00: simplify scan_event handling Tejun Heo
2013-01-27 21:02 ` Stanislav Yakovlev
2013-02-09 19:31 ` Tejun Heo
2012-12-22 1:56 ` [PATCH 05/25] devfreq: don't use [delayed_]work_pending() Tejun Heo
2012-12-22 1:56 ` [PATCH 06/25] libertas: " Tejun Heo
2012-12-22 1:56 ` [PATCH 07/25] mwifiex: " Tejun Heo
2012-12-22 22:29 ` Bing Zhao
2012-12-28 21:41 ` Tejun Heo
2012-12-22 1:56 ` [PATCH 08/25] thinkpad_acpi: " Tejun Heo
2012-12-22 23:55 ` Henrique de Moraes Holschuh
2012-12-28 21:41 ` Tejun Heo
2012-12-22 1:56 ` [PATCH 09/25] wl1251: " Tejun Heo
2012-12-22 14:14 ` Luciano Coelho
2012-12-28 21:42 ` Tejun Heo
2012-12-22 1:57 ` [PATCH 10/25] kprobes: fix wait_for_kprobe_optimizer() Tejun Heo
2012-12-25 3:51 ` Masami Hiramatsu
2013-01-28 19:49 ` Tejun Heo
2013-01-29 11:53 ` Masami Hiramatsu
2013-02-09 19:33 ` Tejun Heo
2012-12-22 1:57 ` [PATCH 11/25] pm: don't use [delayed_]work_pending() Tejun Heo
2012-12-22 11:53 ` Rafael J. Wysocki
2012-12-25 16:44 ` Tejun Heo
2012-12-22 1:57 ` [PATCH 12/25] bluetooth/l2cap: " Tejun Heo
2013-01-03 22:27 ` Gustavo Padovan
2012-12-22 1:57 ` [PATCH 13/25] sound/wm8350: " Tejun Heo
2012-12-24 16:11 ` Mark Brown
2012-12-22 1:57 ` [PATCH 14/25] rfkill: " Tejun Heo
2012-12-22 20:22 ` Johannes Berg
2012-12-28 21:42 ` Tejun Heo
2012-12-22 1:57 ` [PATCH 15/25] x86/mce: " Tejun Heo
2012-12-25 11:07 ` Borislav Petkov
2012-12-28 21:44 ` [PATCH v2 " Tejun Heo
2012-12-22 1:57 ` [PATCH 16/25] PM / Domains: " Tejun Heo
2012-12-22 11:57 ` Rafael J. Wysocki
2012-12-25 17:03 ` Tejun Heo
2012-12-25 20:33 ` Rafael J. Wysocki
2012-12-26 1:23 ` Tejun Heo
2012-12-22 1:57 ` [PATCH 17/25] wm97xx: " Tejun Heo
2012-12-23 9:54 ` Dmitry Torokhov
2012-12-24 16:18 ` Mark Brown
2013-03-09 23:53 ` Dmitry Torokhov
2013-03-12 18:49 ` Mark Brown
2012-12-24 18:25 ` Tejun Heo
2012-12-22 1:57 ` [PATCH 18/25] TMIO MMC: " Tejun Heo
2012-12-24 22:31 ` Guennadi Liakhovetski
2012-12-22 1:57 ` [PATCH 19/25] net/caif: " Tejun Heo
2012-12-22 1:57 ` [PATCH 20/25] wimax/i2400m: fix i2400m->wake_tx_skb handling Tejun Heo
2012-12-22 15:28 ` Perez-Gonzalez, Inaky
2013-01-04 21:19 ` Dan Williams
2013-02-09 19:35 ` Tejun Heo
2012-12-22 1:57 ` [PATCH 21/25] tty/max3100: don't use [delayed_]work_pending() Tejun Heo
2012-12-22 4:21 ` Greg Kroah-Hartman
2012-12-28 21:44 ` Tejun Heo
2012-12-22 1:57 ` [PATCH 22/25] usb/at91_udc: " Tejun Heo
2013-01-07 16:25 ` Nicolas Ferre
2012-12-22 1:57 ` [PATCH 23/25] video/exynos: " Tejun Heo
2012-12-22 3:05 ` Kukjin Kim
2012-12-26 4:04 ` Jingoo Han
2012-12-28 21:44 ` 'Tejun Heo'
2012-12-22 1:57 ` [PATCH 24/25] debugobjects: " Tejun Heo
2012-12-22 1:57 ` [PATCH 25/25] ipc: " Tejun Heo
2012-12-22 2:15 ` Andrew Morton
2012-12-22 2:22 ` Tejun Heo
2012-12-22 11:09 ` Borislav Petkov
2012-12-24 18:33 ` Tejun Heo
2012-12-24 18:45 ` Tejun Heo
2012-12-24 19:41 ` Borislav Petkov
2012-12-25 3:29 ` Tejun Heo
2012-12-25 10:46 ` Borislav Petkov
2012-12-25 16:35 ` Tejun Heo
2012-12-24 18:55 ` Borislav Petkov
2012-12-24 19:07 ` Tejun Heo
2012-12-24 19:32 ` Borislav Petkov
2012-12-25 3:18 ` 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=1356141435-17340-1-git-send-email-tj@kernel.org \
--to=tj@kernel.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox