All of lore.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: 84+ 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
     [not found]   ` <1356141435-17340-9-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2012-12-22 23:55     ` Henrique de Moraes Holschuh
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 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.