lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 21 Dec 2012 17:56:50 -0800
From:	Tejun Heo <tj@...nel.org>
To:	linux-kernel@...r.kernel.org
Subject: [PATCHSET] workqueue: don't use [delayed_]work_pending()

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ