public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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

             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