Skip to contentSkip to conversation

workqueue: make all workqueues non-reentrant

PATCH In review Tejun Heo 3 participants 10 messages Aug 14, 2012 – Aug 20, 2012

Subsystems (25)
Networking Network drivers DRM drivers Documentation +21 more subsystems
lore
Share & tools
lore

Tejun Heo

author maintainer
#0 lore raw

PATCH 1/1 workqueue: make all workqueues non-reentrant

Hello,

Unless explicitly specified, a workqueued is reentrant, which might not even be the correct term. If a work item is executing on a CPU but not pending anywhere, the work item can be queued on a different CPU, and that CPU might start executing the work item whether the previous execution on the first CPU is finished or not. In short, while a work item can be queued on only one CPU, it may be executing concurrently on any subset of the CPUs in the system.

This behavior in itself is already confusing and error-prone; however, this causes much worse complication in flush operations. Because a given work item could be executing on any number of CPUs, even if no further queueing happens after invocation, flush_work() can't guarantee that the work item is idle on return without checking all CPUs. Being a relatively common operation, checking all CPUs on each invocation was deemed too costly. It only verifies the instance queued last and there's a separate version which checks all CPUs - flush_work_sync().

Also, due to the way last CPU is tracked, if there are parallel queueing operations, the following sequence

	schedule_work(A);
	flush_work(A);

can't guarantee that the instance queued right above is finished. If someone else queues A on another CPU inbetween, flush_work() either becomes noop or flushes that instance which may finish earlier than the one from the above.

In the end, workqueue is a pain in the ass to get completely correct and the breakages are very subtle. Depending on queueing pattern, assuming non-reentrancy might work fine most of the time. The effect of using flush_work() where flush_work_sync() should be used could be a lot more subtle. flush_work() becoming noop would happen extremely rarely for most users but it definitely is there.

A tool which is used as widely as workqueue shouldn't be this difficult and error-prone. This is just silly.

Timer does the right thing. It *always* checks whether a timer is still executing on the previous CPU and queues it there if so much like how WQ_NON_REENTRANT makes workqueue behave. WQ_NON_REENTRANT guarantees that any given work item is executing on only one CPU at maximum and if both pending and executing, both are on the same CPU. This makes the behaviors of work item execution and flush_work() much more sane and flush_work_sync() unnecessary.

This patchset enforces WQ_NON_REENTRANT behavior on all workqueues and simplifies workqueue API accordingly.

This adds an additional busy_hash lookup if the work item was previously queued on a different CPU. This shouldn't be noticeable under any sane workload. Work item queueing isn't a very high-frequency operation and they don't jump across CPUs all the time. In a micro benchmark to exaggerate this difference - measuring the time it takes for two work items to repeatedly jump between two CPUs a number (10M) of times with busy_hash table densely populated, the difference was around 3%.

While the overhead is measureable, it is only visible in pathological cases and the difference isn't huge. This change brings much needed sanity to workqueue and makes its behavior consistent with timer. I think this is the right tradeoff to make.

This patchset contains the following six patches.

0001-workqueue-make-all-workqueues-non-reentrant.patch 0002-workqueue-gut-flush-_delayed-_work_sync.patch 0003-workqueue-gut-system_nrt-_freezable-_wq.patch 0004-workqueue-deprecate-flush-_delayed-_work_sync.patch 0005-workqueue-deprecate-system_nrt-_freezable-_wq.patch 0006-workqueue-deprecate-WQ_NON_REENTRANT.patch

0001 makes all workqueues non-reentrant.

0002-0003 simplify workqueue API accordingly.

0004-0006 deprecate now unnecessary parts of workqueue API and convert their users. Most conversions are straight-forward but 0006 contains some non-trivial ones. If you work on NFC and drivers/staging/nvec, please review them.

This patchset is on top of

wq/for-3.7 1265057fa02c7bed3b6d9ddc8a2048065a370364
+ [1] "timer: clean up initializers and implement irqsafe" patchset
+ [2] "workqueue: use irqsafe timer in delayed_work" patchset

and available in the following git branch.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git review-wq-always-nrt

Thanks. diffstat follows.

 Documentation/workqueue.txt                     |   14 --
 arch/arm/mach-pxa/sharpsl_pm.c                  |    4 
 arch/arm/plat-omap/mailbox.c                    |    2 
 arch/sh/drivers/push-switch.c                   |    2 
 block/blk-throttle.c                            |    7 -
 block/genhd.c                                   |   10 -
 drivers/block/xen-blkfront.c                    |    4 
 drivers/cdrom/gdrom.c                           |    2 
 drivers/char/sonypi.c                           |    2 
 drivers/char/tpm/tpm.c                          |    4 
 drivers/firewire/core-transaction.c             |    3 
 drivers/gpu/drm/drm_crtc_helper.c               |    6 
 drivers/gpu/drm/exynos/exynos_drm_g2d.c         |    2 
 drivers/gpu/drm/i915/i915_dma.c                 |    6 
 drivers/gpu/drm/nouveau/nouveau_gpio.c          |    2 
 drivers/gpu/drm/radeon/radeon_irq_kms.c         |    2 
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c              |    2 
 drivers/hid/hid-picolcd.c                       |    2 
 drivers/hid/hid-wiimote-ext.c                   |    2 
 drivers/input/touchscreen/wm831x-ts.c           |    2 
 drivers/isdn/mISDN/hwchannel.c                  |    4 
 drivers/leds/leds-lm3533.c                      |    6 
 drivers/leds/leds-lp8788.c                      |    2 
 drivers/leds/leds-wm8350.c                      |    2 
 drivers/macintosh/ams/ams-core.c                |    2 
 drivers/md/dm-crypt.c                           |   10 -
 drivers/md/dm-kcopyd.c                          |    3 
 drivers/md/dm-mpath.c                           |    2 
 drivers/md/dm-raid1.c                           |    5 
 drivers/md/dm-stripe.c                          |    2 
 drivers/md/dm.c                                 |    3 
 drivers/media/dvb/dvb-core/dvb_net.c            |    4 
 drivers/media/dvb/mantis/mantis_evm.c           |    2 
 drivers/media/dvb/mantis/mantis_uart.c          |    2 
 drivers/media/video/bt8xx/bttv-driver.c         |    2 
 drivers/media/video/cx18/cx18-driver.c          |    2 
 drivers/media/video/cx231xx/cx231xx-cards.c     |    2 
 drivers/media/video/cx23885/cx23885-input.c     |    6 
 drivers/media/video/cx88/cx88-mpeg.c            |    2 
 drivers/media/video/em28xx/em28xx-cards.c       |    2 
 drivers/media/video/omap24xxcam.c               |    6 
 drivers/media/video/saa7134/saa7134-core.c      |    2 
 drivers/media/video/saa7134/saa7134-empress.c   |    2 
 drivers/media/video/tm6000/tm6000-cards.c       |    2 
 drivers/mfd/menelaus.c                          |    4 
 drivers/misc/ioc4.c                             |    2 
 drivers/mmc/core/host.c                         |    4 
 drivers/mmc/host/dw_mmc.c                       |    3 
 drivers/mtd/mtdoops.c                           |    4 
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c |    2 
 drivers/net/ethernet/neterion/vxge/vxge-main.c  |    2 
 drivers/net/ethernet/sun/cassini.c              |    2 
 drivers/net/ethernet/sun/niu.c                  |    2 
 drivers/net/virtio_net.c                        |   12 -
 drivers/net/wireless/hostap/hostap_ap.c         |    4 
 drivers/net/wireless/hostap/hostap_hw.c         |   10 -
 drivers/nfc/pn533.c                             |    5 
 drivers/power/collie_battery.c                  |    2 
 drivers/power/tosa_battery.c                    |    2 
 drivers/power/wm97xx_battery.c                  |    2 
 drivers/power/z2_battery.c                      |    2 
 drivers/regulator/core.c                        |    2 
 drivers/scsi/arcmsr/arcmsr_hba.c                |    4 
 drivers/scsi/ipr.c                              |    2 
 drivers/scsi/pmcraid.c                          |    2 
 drivers/scsi/qla2xxx/qla_target.c               |    2 
 drivers/staging/nvec/nvec.c                     |   10 -
 drivers/staging/nvec/nvec.h                     |    2 
 drivers/staging/omapdrm/omap_drv.c              |    3 
 drivers/tty/hvc/hvsi.c                          |    2 
 drivers/tty/ipwireless/hardware.c               |    2 
 drivers/tty/ipwireless/network.c                |    4 
 drivers/tty/serial/kgdboc.c                     |    2 
 drivers/tty/serial/omap-serial.c                |    2 
 drivers/tty/tty_ldisc.c                         |    6 
 drivers/usb/atm/speedtch.c                      |    2 
 drivers/usb/atm/ueagle-atm.c                    |    2 
 drivers/usb/gadget/u_ether.c                    |    2 
 drivers/usb/host/ohci-hcd.c                     |    2 
 drivers/usb/otg/isp1301_omap.c                  |    2 
 fs/affs/super.c                                 |    2 
 fs/dlm/ast.c                                    |    5 
 fs/gfs2/lock_dlm.c                              |    2 
 fs/gfs2/main.c                                  |    2 
 fs/gfs2/super.c                                 |    2 
 fs/hfs/inode.c                                  |    2 
 fs/ncpfs/inode.c                                |    6 
 fs/ocfs2/cluster/quorum.c                       |    2 
 fs/xfs/xfs_super.c                              |    4 
 fs/xfs/xfs_sync.c                               |    2 
 include/linux/workqueue.h                       |   37 ++++-
 kernel/srcu.c                                   |    4 
 kernel/workqueue.c                              |  150 +++---------------------
 net/9p/trans_fd.c                               |    2 
 net/ceph/messenger.c                            |    2 
 net/dsa/dsa.c                                   |    2 
 net/nfc/core.c                                  |    6 
 net/nfc/hci/core.c                              |    8 -
 net/nfc/hci/shdlc.c                             |    4 
 net/nfc/llcp/llcp.c                             |   18 --
 security/keys/gc.c                              |    8 -
 security/keys/key.c                             |    2 
 sound/i2c/other/ak4113.c                        |    2 
 sound/i2c/other/ak4114.c                        |    2 
 sound/pci/oxygen/oxygen_lib.c                   |    8 -
 sound/soc/codecs/wm8350.c                       |    2 
 sound/soc/codecs/wm8753.c                       |    2 
 sound/soc/soc-core.c                            |    6 
 virt/kvm/eventfd.c                              |    2 
 109 files changed, 221 insertions(+), 351 deletions(-)

--
tejun

[1] http://thread.gmane.org/gmane.linux.kernel/1340224
[2] http://thread.gmane.org/gmane.linux.kernel/1340333

author maintainer
#1 lore raw

PATCH 1/6 workqueue: make all workqueues non-reentrant

By default, each per-cpu part of a bound workqueue operates separately and a work item may be executing concurrently on different CPUs. The behavior avoids some cross-cpu traffic but leads to subtle weirdities and not-so-subtle contortions in the API.

* There's no sane usefulness in allowing a single work item to be executed concurrently on multiple CPUs. People just get the behavior unintentionally and get surprised after learning about it. Most either explicitly synchronize or use non-reentrant/ordered workqueue but this is error-prone.

* flush_work() can't wait for multiple instances of the same work item on different CPUs. If a work item is executing on cpu0 and then queued on cpu1, flush_work() can only wait for the one on cpu1.

Unfortunately, work items can easily cross CPU boundaries unintentionally when the queueing thread gets migrated. This means that if multiple queuers compete, flush_work() can't even guarantee that the instance queued right before it is finished before returning.

* flush_work_sync() was added to work around some of the deficiencies of flush_work(). In addition to the usual flushing, it ensures that all currently executing instances are finished before returning. This operation is expensive as it has to walk all CPUs and at the same time fails to address competing queuer case.

Incorrectly using flush_work() when flush_work_sync() is necessary is an easy error to make and can lead to bugs which are difficult to reproduce.

* Similar problems exist for flush_delayed_work[_sync]().

Other than the cross-cpu access concern, there's no benefit in allowing parallel execution and it's plain silly to have this level of contortion for workqueue which is widely used from core code to extremely obscure drivers.

This patch makes all workqueues non-reentrant. If a work item is executing on a different CPU when queueing is requested, it is always queued to that CPU. This guarantees that any given work item can be executing on one CPU at maximum and if a work item is queued and executing, both are on the same CPU.

The only behavior change which may affect workqueue users negatively is that non-reentrancy overrides the affinity specified by queue_work_on(). On a reentrant workqueue, the affinity specified by queue_work_on() is always followed. Now, if the work item is executing on one of the CPUs, the work item will be queued there regardless of the requested affinity. I've reviewed all workqueue users which request explicit affinity, and, fortunately, none seems to be crazy enough to exploit parallel execution of the same work item.

This adds an additional busy_hash lookup if the work item was previously queued on a different CPU. This shouldn't be noticeable under any sane workload. Work item queueing isn't a very high-frequency operation and they don't jump across CPUs all the time. In a micro benchmark to exaggerate this difference - measuring the time it takes for two work items to repeatedly jump between two CPUs a number (10M) of times with busy_hash table densely populated, the difference was around 3%.

While the overhead is measureable, it is only visible in pathological cases and the difference isn't huge. This change brings much needed sanity to workqueue and makes its behavior consistent with timer. I think this is the right tradeoff to make.

This enables significant simplification of workqueue API. Simplification patches will follow.

Signed-off-by Tejun Heo <[email protected]>
---
 kernel/workqueue.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)
kernel/workqueue.c +7 -6
index 7413242..26f048b 100644
@@ -1216,14 +1216,15 @@ static void __queue_work(unsigned int cpu, struct workqueue_struct *wq,
 			cpu = raw_smp_processor_id();
 
 		/*
-		 * It's multi cpu.  If @wq is non-reentrant and @work
-		 * was previously on a different cpu, it might still
-		 * be running there, in which case the work needs to
-		 * be queued on that cpu to guarantee non-reentrance.
+		 * It's multi cpu.  If @work was previously on a different
+		 * cpu, it might still be running there, in which case the
+		 * work needs to be queued on that cpu to guarantee
+		 * non-reentrancy.
 		 */
 		gcwq = get_gcwq(cpu);
-		if (wq->flags & WQ_NON_REENTRANT &&
-		    (last_gcwq = get_work_gcwq(work)) && last_gcwq != gcwq) {
+		last_gcwq = get_work_gcwq(work);
+
+		if (last_gcwq && last_gcwq != gcwq) {
 			struct worker *worker;
 
 			spin_lock(&last_gcwq->lock);
Signature · 1 line
1.7.7.3
author maintainer
#2 lore raw

PATCH 2/6 workqueue: gut flush[_delayed]_work_sync()

In reply to Tejun Heo

Now that all workqueues are non-reentrant, flush[_delayed]_work_sync() are equivalent to flush[_delayed]_work(). Drop the separate implementation and make them thin wrappers around flush[_delayed]_work().

* start_flush_work() no longer takes @wait_executing as the only left user - flush_work() - always sets it to %true.

* __cancel_work_timer() uses flush_work() instead of wait_on_work().

Signed-off-by Tejun Heo <[email protected]>
---
 include/linux/workqueue.h |   14 +++++-
 kernel/workqueue.c        |  122 ++++-----------------------------------------
 2 files changed, 22 insertions(+), 114 deletions(-)
include/linux/workqueue.h +12 -2
index 6cd8f91..b2716b6 100644
@@ -412,11 +412,9 @@ extern int keventd_up(void);
 int execute_in_process_context(work_func_t fn, struct execute_work *);
 
 extern bool flush_work(struct work_struct *work);
-extern bool flush_work_sync(struct work_struct *work);
 extern bool cancel_work_sync(struct work_struct *work);
 
 extern bool flush_delayed_work(struct delayed_work *dwork);
-extern bool flush_delayed_work_sync(struct delayed_work *work);
 extern bool cancel_delayed_work(struct delayed_work *dwork);
 extern bool cancel_delayed_work_sync(struct delayed_work *dwork);
 

@@ -441,6 +439,18 @@ static inline bool __deprecated __cancel_delayed_work(struct delayed_work *work)
 	return ret;
 }
 
+/* used to be different but now identical to flush_work(), deprecated */
+static inline bool flush_work_sync(struct work_struct *work)
+{
+	return flush_work(work);
+}
+
+/* used to be different but now identical to flush_delayed_work(), deprecated */
+static inline bool flush_delayed_work_sync(struct delayed_work *dwork)
+{
+	return flush_delayed_work(dwork);
+}
+
 #ifndef CONFIG_SMP
 static inline long work_on_cpu(unsigned int cpu, long (*fn)(void *), void *arg)
 {
kernel/workqueue.c +10 -112

174 lines not on this page · raw message

Signature · 1 line
1.7.7.3
author maintainer
#3 lore raw

PATCH 3/6 workqueue: gut system_nrt[_freezable]_wq()

In reply to Tejun Heo

Now that all workqueues are non-reentrant, system[_freezable]_wq() are equivalent to system_nrt[_freezable]_wq(). Replace the latter with wrappers around system[_freezable]_wq(). The wrapping goes through inline functions so that __deprecated can be added easily.

Signed-off-by Tejun Heo <[email protected]>
---
 include/linux/workqueue.h |   23 ++++++++++++++---------
 kernel/workqueue.c        |   12 ++----------
 2 files changed, 16 insertions(+), 19 deletions(-)
include/linux/workqueue.h +14 -9
index b2716b6..b0a678b 100644
@@ -297,10 +297,6 @@ enum {
  * system_long_wq is similar to system_wq but may host long running
  * works.  Queue flushing might take relatively long.
  *
- * system_nrt_wq is non-reentrant and guarantees that any given work
- * item is never executed in parallel by multiple CPUs.  Queue
- * flushing might take relatively long.
- *
  * system_unbound_wq is unbound workqueue.  Workers are not bound to
  * any specific CPU, not concurrency managed, and all queued works are
  * executed immediately as long as max_active limit is not reached and

@@ -308,16 +304,25 @@ enum {
  *
  * system_freezable_wq is equivalent to system_wq except that it's
  * freezable.
- *
- * system_nrt_freezable_wq is equivalent to system_nrt_wq except that
- * it's freezable.
  */
 extern struct workqueue_struct *system_wq;
 extern struct workqueue_struct *system_long_wq;
-extern struct workqueue_struct *system_nrt_wq;
 extern struct workqueue_struct *system_unbound_wq;
 extern struct workqueue_struct *system_freezable_wq;
-extern struct workqueue_struct *system_nrt_freezable_wq;
+
+static inline struct workqueue_struct *__system_nrt_wq(void)
+{
+	return system_wq;
+}
+
+static inline struct workqueue_struct *__system_nrt_freezable_wq(void)
+{
+	return system_freezable_wq;
+}
+
+/* equivlalent to system_wq and system_freezable_wq, deprecated */
+#define system_nrt_wq			__system_nrt_wq()
+#define system_nrt_freezable_wq		__system_nrt_freezable_wq()
 
 extern struct workqueue_struct *
 __alloc_workqueue_key(const char *fmt, unsigned int flags, int max_active,
kernel/workqueue.c +2 -10
index 938adc0..ce39467 100644
@@ -269,16 +269,12 @@ struct workqueue_struct {
 
 struct workqueue_struct *system_wq __read_mostly;
 struct workqueue_struct *system_long_wq __read_mostly;
-struct workqueue_struct *system_nrt_wq __read_mostly;
 struct workqueue_struct *system_unbound_wq __read_mostly;
 struct workqueue_struct *system_freezable_wq __read_mostly;
-struct workqueue_struct *system_nrt_freezable_wq __read_mostly;
 EXPORT_SYMBOL_GPL(system_wq);
 EXPORT_SYMBOL_GPL(system_long_wq);
-EXPORT_SYMBOL_GPL(system_nrt_wq);
 EXPORT_SYMBOL_GPL(system_unbound_wq);
 EXPORT_SYMBOL_GPL(system_freezable_wq);
-EXPORT_SYMBOL_GPL(system_nrt_freezable_wq);
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/workqueue.h>

@@ -3847,16 +3843,12 @@ static int __init init_workqueues(void)
 
 	system_wq = alloc_workqueue("events", 0, 0);
 	system_long_wq = alloc_workqueue("events_long", 0, 0);
-	system_nrt_wq = alloc_workqueue("events_nrt", WQ_NON_REENTRANT, 0);
 	system_unbound_wq = alloc_workqueue("events_unbound", WQ_UNBOUND,
 					    WQ_UNBOUND_MAX_ACTIVE);
 	system_freezable_wq = alloc_workqueue("events_freezable",
 					      WQ_FREEZABLE, 0);
-	system_nrt_freezable_wq = alloc_workqueue("events_nrt_freezable",
-			WQ_NON_REENTRANT | WQ_FREEZABLE, 0);
-	BUG_ON(!system_wq || !system_long_wq || !system_nrt_wq ||
-	       !system_unbound_wq || !system_freezable_wq ||
-		!system_nrt_freezable_wq);
+	BUG_ON(!system_wq || !system_long_wq || !system_unbound_wq ||
+	       !system_freezable_wq);
 	return 0;
 }
 early_initcall(init_workqueues);
Signature · 1 line
1.7.7.3
author maintainer
#4 lore raw

PATCH 5/6 workqueue: deprecate system_nrt[_freezable]_wq

In reply to Tejun Heo

system_nrt[_freezable]_wq are now spurious. Mark them deprecated and convert all users to system[_freezable]_wq.

If you're cc'd and wondering what's going on: Now all workqueues are non-reentrant, so there's no reason to use system_nrt[_freezable]_wq. Please use system[_freezable]_wq instead.

This patch doesn't make any functional difference.

Signed-off-by Tejun Heo <[email protected]>
Cc Jens Axboe <[email protected]>
Cc David Airlie <[email protected]>
Cc Jiri Kosina <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Rusty Russell <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Cc David Howells <[email protected]>
---
 block/blk-throttle.c              |    7 +++----
 block/genhd.c                     |   10 +++++-----
 drivers/gpu/drm/drm_crtc_helper.c |    6 +++---
 drivers/hid/hid-wiimote-ext.c     |    2 +-
 drivers/mmc/core/host.c           |    4 ++--
 drivers/net/virtio_net.c          |   12 ++++++------
 include/linux/workqueue.h         |    4 ++--
 kernel/srcu.c                     |    4 ++--
 security/keys/gc.c                |    8 ++++----
 security/keys/key.c               |    2 +-
 10 files changed, 29 insertions(+), 30 deletions(-)
block/blk-throttle.c +3 -4
index 3d3dcae..a9664fa 100644
@@ -180,7 +180,7 @@ static inline unsigned int total_nr_queued(struct throtl_data *td)
 
 /*
  * Worker for allocating per cpu stat for tgs. This is scheduled on the
- * system_nrt_wq once there are some groups on the alloc_list waiting for
+ * system_wq once there are some groups on the alloc_list waiting for
  * allocation.
  */
 static void tg_stats_alloc_fn(struct work_struct *work)

@@ -194,8 +194,7 @@ alloc_stats:
 		stats_cpu = alloc_percpu(struct tg_stats_cpu);
 		if (!stats_cpu) {
 			/* allocation failed, try again after some time */
-			queue_delayed_work(system_nrt_wq, dwork,
-					   msecs_to_jiffies(10));
+			schedule_delayed_work(dwork, msecs_to_jiffies(10));
 			return;
 		}
 	}

@@ -238,7 +237,7 @@ static void throtl_pd_init(struct blkcg_gq *blkg)
 	 */
 	spin_lock_irqsave(&tg_stats_alloc_lock, flags);
 	list_add(&tg->stats_alloc_node, &tg_stats_alloc_list);
-	queue_delayed_work(system_nrt_wq, &tg_stats_alloc_work, 0);
+	schedule_delayed_work(&tg_stats_alloc_work, 0);
 	spin_unlock_irqrestore(&tg_stats_alloc_lock, flags);
 }
block/genhd.c +5 -5
index 5d8b44a..a2f3d6a 100644
@@ -1490,9 +1490,9 @@ static void __disk_unblock_events(struct gendisk *disk, bool check_now)
 	intv = disk_events_poll_jiffies(disk);
 	set_timer_slack(&ev->dwork.timer, intv / 4);
 	if (check_now)
-		queue_delayed_work(system_nrt_freezable_wq, &ev->dwork, 0);
+		queue_delayed_work(system_freezable_wq, &ev->dwork, 0);
 	else if (intv)
-		queue_delayed_work(system_nrt_freezable_wq, &ev->dwork, intv);
+		queue_delayed_work(system_freezable_wq, &ev->dwork, intv);
 out_unlock:
 	spin_unlock_irqrestore(&ev->lock, flags);
 }

@@ -1535,7 +1535,7 @@ void disk_flush_events(struct gendisk *disk, unsigned int mask)
 	spin_lock_irq(&ev->lock);
 	ev->clearing |= mask;
 	if (!ev->block)
-		mod_delayed_work(system_nrt_freezable_wq, &ev->dwork, 0);
+		mod_delayed_work(system_freezable_wq, &ev->dwork, 0);
 	spin_unlock_irq(&ev->lock);
 }
 

@@ -1571,7 +1571,7 @@ unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask)
 
 	/* uncondtionally schedule event check and wait for it to finish */
 	disk_block_events(disk);
-	queue_delayed_work(system_nrt_freezable_wq, &ev->dwork, 0);
+	queue_delayed_work(system_freezable_wq, &ev->dwork, 0);
 	flush_delayed_work(&ev->dwork);
 	__disk_unblock_events(disk, false);
 

@@ -1608,7 +1608,7 @@ static void disk_events_workfn(struct work_struct *work)
 
 	intv = disk_events_poll_jiffies(disk);
 	if (!ev->block && intv)
-		queue_delayed_work(system_nrt_freezable_wq, &ev->dwork, intv);
+		queue_delayed_work(system_freezable_wq, &ev->dwork, intv);
 
 	spin_unlock_irq(&ev->lock);
drivers/gpu/drm/drm_crtc_helper.c +3 -3
index 3252e70..8fa9d52 100644
@@ -968,7 +968,7 @@ static void output_poll_execute(struct work_struct *work)
 	}
 
 	if (repoll)
-		queue_delayed_work(system_nrt_wq, delayed_work, DRM_OUTPUT_POLL_PERIOD);
+		schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
 }
 
 void drm_kms_helper_poll_disable(struct drm_device *dev)

@@ -993,7 +993,7 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
 	}
 
 	if (poll)
-		queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, DRM_OUTPUT_POLL_PERIOD);
+		schedule_delayed_work(&dev->mode_config.output_poll_work, DRM_OUTPUT_POLL_PERIOD);
 }
 EXPORT_SYMBOL(drm_kms_helper_poll_enable);
 

@@ -1020,6 +1020,6 @@ void drm_helper_hpd_irq_event(struct drm_device *dev)
 	/* kill timer and schedule immediate execution, this doesn't block */
 	cancel_delayed_work(&dev->mode_config.output_poll_work);
 	if (drm_kms_helper_poll)
-		queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0);
+		schedule_delayed_work(&dev->mode_config.output_poll_work, 0);
 }
 EXPORT_SYMBOL(drm_helper_hpd_irq_event);
drivers/hid/hid-wiimote-ext.c +1 -1
index 0a1805c..d37cd09 100644
@@ -204,7 +204,7 @@ static void wiiext_worker(struct work_struct *work)
 /* schedule work only once, otherwise mark for reschedule */
 static void wiiext_schedule(struct wiimote_ext *ext)
 {
-	queue_work(system_nrt_wq, &ext->worker);
+	schedule_work(&ext->worker);
 }
 
 /*
drivers/mmc/core/host.c +2 -2
index 597f189..ee2e16b 100644
@@ -204,8 +204,8 @@ void mmc_host_clk_release(struct mmc_host *host)
 	host->clk_requests--;
 	if (mmc_host_may_gate_card(host->card) &&
 	    !host->clk_requests)
-		queue_delayed_work(system_nrt_wq, &host->clk_gate_work,
-				msecs_to_jiffies(host->clkgate_delay));
+		schedule_delayed_work(&host->clk_gate_work,
+				      msecs_to_jiffies(host->clkgate_delay));
 	spin_unlock_irqrestore(&host->clk_lock, flags);
 }
drivers/net/virtio_net.c +6 -6
index 83d2b0c..9650c41 100644
@@ -521,7 +521,7 @@ static void refill_work(struct work_struct *work)
 	/* In theory, this can happen: if we don't get any buffers in
 	 * we will *never* try to fill again. */
 	if (still_empty)
-		queue_delayed_work(system_nrt_wq, &vi->refill, HZ/2);
+		schedule_delayed_work(&vi->refill, HZ/2);
 }
 
 static int virtnet_poll(struct napi_struct *napi, int budget)

@@ -540,7 +540,7 @@ again:
 
 	if (vi->num < vi->max / 2) {
 		if (!try_fill_recv(vi, GFP_ATOMIC))
-			queue_delayed_work(system_nrt_wq, &vi->refill, 0);
+			schedule_delayed_work(&vi->refill, 0);
 	}
 
 	/* Out of packets? */

@@ -745,7 +745,7 @@ static int virtnet_open(struct net_device *dev)
 
 	/* Make sure we have some buffers: if oom use wq. */
 	if (!try_fill_recv(vi, GFP_KERNEL))
-		queue_delayed_work(system_nrt_wq, &vi->refill, 0);
+		schedule_delayed_work(&vi->refill, 0);
 
 	virtnet_napi_enable(vi);
 	return 0;

@@ -1020,7 +1020,7 @@ static void virtnet_config_changed(struct virtio_device *vdev)
 {
 	struct virtnet_info *vi = vdev->priv;
 
-	queue_work(system_nrt_wq, &vi->config_work);
+	schedule_work(&vi->config_work);
 }
 
 static int init_vqs(struct virtnet_info *vi)

@@ -1152,7 +1152,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 	   otherwise get link status from config. */
 	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
 		netif_carrier_off(dev);
-		queue_work(system_nrt_wq, &vi->config_work);
+		schedule_work(&vi->config_work);
 	} else {
 		vi->status = VIRTIO_NET_S_LINK_UP;
 		netif_carrier_on(dev);

@@ -1264,7 +1264,7 @@ static int virtnet_restore(struct virtio_device *vdev)
 	netif_device_attach(vi->dev);
 
 	if (!try_fill_recv(vi, GFP_KERNEL))
-		queue_delayed_work(system_nrt_wq, &vi->refill, 0);
+		schedule_delayed_work(&vi->refill, 0);
 
 	mutex_lock(&vi->config_lock);
 	vi->config_enable = true;
include/linux/workqueue.h +2 -2
index ffaaec0..2b58905 100644
@@ -310,12 +310,12 @@ extern struct workqueue_struct *system_long_wq;
 extern struct workqueue_struct *system_unbound_wq;
 extern struct workqueue_struct *system_freezable_wq;
 
-static inline struct workqueue_struct *__system_nrt_wq(void)
+static inline struct workqueue_struct * __deprecated __system_nrt_wq(void)
 {
 	return system_wq;
 }
 
-static inline struct workqueue_struct *__system_nrt_freezable_wq(void)
+static inline struct workqueue_struct * __deprecated __system_nrt_freezable_wq(void)
 {
 	return system_freezable_wq;
 }
kernel/srcu.c +2 -2
index 2095be3..97c465e 100644
@@ -379,7 +379,7 @@ void call_srcu(struct srcu_struct *sp, struct rcu_head *head,
 	rcu_batch_queue(&sp->batch_queue, head);
 	if (!sp->running) {
 		sp->running = true;
-		queue_delayed_work(system_nrt_wq, &sp->work, 0);
+		schedule_delayed_work(&sp->work, 0);
 	}
 	spin_unlock_irqrestore(&sp->queue_lock, flags);
 }

@@ -631,7 +631,7 @@ static void srcu_reschedule(struct srcu_struct *sp)
 	}
 
 	if (pending)
-		queue_delayed_work(system_nrt_wq, &sp->work, SRCU_INTERVAL);
+		schedule_delayed_work(&sp->work, SRCU_INTERVAL);
 }
 
 /*
security/keys/gc.c +4 -4
index 61ab7c8..d67c97b 100644
@@ -62,7 +62,7 @@ void key_schedule_gc(time_t gc_at)
 
 	if (gc_at <= now || test_bit(KEY_GC_REAP_KEYTYPE, &key_gc_flags)) {
 		kdebug("IMMEDIATE");
-		queue_work(system_nrt_wq, &key_gc_work);
+		schedule_work(&key_gc_work);
 	} else if (gc_at < key_gc_next_run) {
 		kdebug("DEFERRED");
 		key_gc_next_run = gc_at;

@@ -77,7 +77,7 @@ void key_schedule_gc(time_t gc_at)
 void key_schedule_gc_links(void)
 {
 	set_bit(KEY_GC_KEY_EXPIRED, &key_gc_flags);
-	queue_work(system_nrt_wq, &key_gc_work);
+	schedule_work(&key_gc_work);
 }
 
 /*

@@ -120,7 +120,7 @@ void key_gc_keytype(struct key_type *ktype)
 	set_bit(KEY_GC_REAP_KEYTYPE, &key_gc_flags);
 
 	kdebug("schedule");
-	queue_work(system_nrt_wq, &key_gc_work);
+	schedule_work(&key_gc_work);
 
 	kdebug("sleep");
 	wait_on_bit(&key_gc_flags, KEY_GC_REAPING_KEYTYPE, key_gc_wait_bit,

@@ -369,7 +369,7 @@ maybe_resched:
 	}
 
 	if (gc_state & KEY_GC_REAP_AGAIN)
-		queue_work(system_nrt_wq, &key_gc_work);
+		schedule_work(&key_gc_work);
 	kleave(" [end %x]", gc_state);
 	return;
security/keys/key.c +1 -1
index 50d96d4..3cbe352 100644
@@ -598,7 +598,7 @@ void key_put(struct key *key)
 		key_check(key);
 
 		if (atomic_dec_and_test(&key->usage))
-			queue_work(system_nrt_wq, &key_gc_work);
+			schedule_work(&key_gc_work);
 	}
 }
 EXPORT_SYMBOL(key_put);
Signature · 1 line
1.7.7.3
Acked-by on v1 5/6
#5 lore raw

Re: [PATCH 5/6] workqueue: deprecate system_nrt[_freezable]_wq

Quoted 46 lines · Tejun Heo

system_nrt[_freezable]_wq are now spurious. Mark them deprecated and convert all users to system[_freezable]_wq.

If you're cc'd and wondering what's going on: Now all workqueues are non-reentrant, so there's no reason to use system_nrt[_freezable]_wq. Please use system[_freezable]_wq instead.

This patch doesn't make any functional difference.

Signed-off-by Tejun Heo <[email protected]>
Cc Jens Axboe <[email protected]>
Cc David Airlie <[email protected]>
Cc Jiri Kosina <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc Rusty Russell <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
Cc David Howells <[email protected]>
---
 block/blk-throttle.c              |    7 +++----
 block/genhd.c                     |   10 +++++-----
 drivers/gpu/drm/drm_crtc_helper.c |    6 +++---
 drivers/hid/hid-wiimote-ext.c     |    2 +-
 drivers/mmc/core/host.c           |    4 ++--
 drivers/net/virtio_net.c          |   12 ++++++------
 include/linux/workqueue.h         |    4 ++--
 kernel/srcu.c                     |    4 ++--
 security/keys/gc.c                |    8 ++++----
 security/keys/key.c               |    2 +-
 10 files changed, 29 insertions(+), 30 deletions(-)
kernel/srcu.c +1 -1
index 2095be3..97c465e 100644
@@ -379,7 +379,7 @@ void call_srcu(struct srcu_struct *sp, struct rcu_head *head,
 	rcu_batch_queue(&sp->batch_queue, head);
 	if (!sp->running) {
 		sp->running = true;
-		queue_delayed_work(system_nrt_wq, &sp->work, 0);
+		schedule_delayed_work(&sp->work, 0);
 	}
 	spin_unlock_irqrestore(&sp->queue_lock, flags);
 }

kernel/srcu.c static void srcu_reschedule(struct srcu_struct *sp) · line 637

Quoted 8 lines
kernel/srcu.c +1 -1
@@ -631,7 +631,7 @@ static void srcu_reschedule(struct srcu_struct *sp)
 	}
 
 	if (pending)
-		queue_delayed_work(system_nrt_wq, &sp->work, SRCU_INTERVAL);
+		schedule_delayed_work(&sp->work, SRCU_INTERVAL);
 }
 
 /*
Acked-by Lai Jiangshan <[email protected]>
author maintainer
#6 lore raw

PATCH 6/6 workqueue: deprecate WQ_NON_REENTRANT

In reply to Tejun Heo

WQ_NON_REENTRANT is now spurious. Make alloc_workqueue() trigger WARN_ON() if the flag is specified and drop it from all in-kernel users.

If you're cc'd and wondering what's going on: Now all workqueues are non-reentrant, so there's no reason to use WQ_NON_REENTRANT.

Some of the conversions in this patch aren't trivial.

* WQ_UNBOUND | WQ_NON_REENTRANT workqueues w/ max_active == 1 are converted to alloc_ordered_workqueue().

* NFC was using a lot of WQ_UNBOUND | WQ_NON_REENTRANT | WQ_MEM_RECLAIM workqueues w/ max_active == 1. I converted them to alloc_ordered_workqueue() but can't understand why NFC would need the reclaim flag at all. I added comments above all such occurrences. If I'm missing something, please let me know; otherwise, please drop the flag. Better, please try to use system_wq instead.

* drivers/staging/nvec doesn't seem to need its own workqueue. Converted to use system_wq and cancel work items on unload. Plesae scream if it's wrong.

Signed-off-by Tejun Heo <[email protected]>
Cc Stefan Richter <[email protected]>
Cc Daniel Vetter <[email protected]>
Cc Alasdair Kergon <[email protected]>
Cc Lauro Ramos Venancio <[email protected]>
Cc Aloisio Almeida Jr <[email protected]>
Cc Samuel Ortiz <[email protected]>
Cc Julian Andres Klode <[email protected]>
Cc Marc Dietrich <[email protected]>
Cc Rob Clark <[email protected]>
Cc Christine Caulfield <[email protected]>
Cc Steven Whitehouse <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Sage Weil <[email protected]>
---
 Documentation/workqueue.txt         |   14 ++------------
 drivers/firewire/core-transaction.c |    3 +--
 drivers/gpu/drm/i915/i915_dma.c     |    6 ++----
 drivers/md/dm-crypt.c               |   10 ++--------
 drivers/md/dm-kcopyd.c              |    3 +--
 drivers/md/dm-raid1.c               |    3 +--
 drivers/md/dm.c                     |    3 +--
 drivers/mmc/host/dw_mmc.c           |    3 +--
 drivers/nfc/pn533.c                 |    5 ++---
 drivers/staging/nvec/nvec.c         |   10 ++++------
 drivers/staging/nvec/nvec.h         |    2 --
 drivers/staging/omapdrm/omap_drv.c  |    3 +--
 fs/dlm/ast.c                        |    5 +----
 fs/gfs2/main.c                      |    2 +-
 fs/xfs/xfs_super.c                  |    2 +-
 kernel/workqueue.c                  |    3 +++
 net/ceph/messenger.c                |    2 +-
 net/nfc/core.c                      |    6 +++---
 net/nfc/hci/core.c                  |    8 ++++----
 net/nfc/hci/shdlc.c                 |    4 ++--
 net/nfc/llcp/llcp.c                 |   18 ++++++------------
 21 files changed, 40 insertions(+), 75 deletions(-)
Documentation/workqueue.txt +2 -12
index a6ab4b6..1fb8813 100644
@@ -100,8 +100,8 @@ Subsystems and drivers can create and queue work items through special
 workqueue API functions as they see fit. They can influence some
 aspects of the way the work items are executed by setting flags on the
 workqueue they are putting the work item on. These flags include
-things like CPU locality, reentrancy, concurrency limits, priority and
-more.  To get a detailed overview refer to the API description of
+things like CPU locality, concurrency limits, priority and more.  To
+get a detailed overview refer to the API description of
 alloc_workqueue() below.
 
 When a work item is queued to a workqueue, the target gcwq and

@@ -166,16 +166,6 @@ resources, scheduled and executed.
 
 @flags:
 
-  WQ_NON_REENTRANT
-
-	By default, a wq guarantees non-reentrance only on the same
-	CPU.  A work item may not be executed concurrently on the same
-	CPU by multiple workers but is allowed to be executed
-	concurrently on multiple CPUs.  This flag makes sure
-	non-reentrance is enforced across all CPUs.  Work items queued
-	to a non-reentrant wq are guaranteed to be executed by at most
-	one worker system-wide at any given time.
-
   WQ_UNBOUND
 
 	Work items queued to an unbound wq are served by a special
drivers/firewire/core-transaction.c +1 -2
index 87d6f2d..45acc0f 100644
@@ -1257,8 +1257,7 @@ static int __init fw_core_init(void)
 {
 	int ret;
 
-	fw_workqueue = alloc_workqueue("firewire",
-				       WQ_NON_REENTRANT | WQ_MEM_RECLAIM, 0);
+	fw_workqueue = alloc_workqueue("firewire", WQ_MEM_RECLAIM, 0);
 	if (!fw_workqueue)
 		return -ENOMEM;
drivers/gpu/drm/i915/i915_dma.c +2 -4
index 9cf7dfe..a55ca7a 100644
@@ -1536,11 +1536,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	 *
 	 * All tasks on the workqueue are expected to acquire the dev mutex
 	 * so there is no point in running more than one instance of the
-	 * workqueue at any time: max_active = 1 and NON_REENTRANT.
+	 * workqueue at any time.  Use an ordered one.
 	 */
-	dev_priv->wq = alloc_workqueue("i915",
-				       WQ_UNBOUND | WQ_NON_REENTRANT,
-				       1);
+	dev_priv->wq = alloc_ordered_workqueue("i915", 0);
 	if (dev_priv->wq == NULL) {
 		DRM_ERROR("Failed to create our workqueue.\n");
 		ret = -ENOMEM;
drivers/md/dm-crypt.c +2 -8
index 664743d..a7472d6 100644
@@ -1674,20 +1674,14 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	}
 
 	ret = -ENOMEM;
-	cc->io_queue = alloc_workqueue("kcryptd_io",
-				       WQ_NON_REENTRANT|
-				       WQ_MEM_RECLAIM,
-				       1);
+	cc->io_queue = alloc_workqueue("kcryptd_io", WQ_MEM_RECLAIM, 1);
 	if (!cc->io_queue) {
 		ti->error = "Couldn't create kcryptd io queue";
 		goto bad;
 	}
 
 	cc->crypt_queue = alloc_workqueue("kcryptd",
-					  WQ_NON_REENTRANT|
-					  WQ_CPU_INTENSIVE|
-					  WQ_MEM_RECLAIM,
-					  1);
+					  WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM, 1);
 	if (!cc->crypt_queue) {
 		ti->error = "Couldn't create kcryptd queue";
 		goto bad;
drivers/md/dm-kcopyd.c +1 -2
index bed444c..7234f71 100644
@@ -704,8 +704,7 @@ struct dm_kcopyd_client *dm_kcopyd_client_create(void)
 		goto bad_slab;
 
 	INIT_WORK(&kc->kcopyd_work, do_work);
-	kc->kcopyd_wq = alloc_workqueue("kcopyd",
-					WQ_NON_REENTRANT | WQ_MEM_RECLAIM, 0);
+	kc->kcopyd_wq = alloc_workqueue("kcopyd", WQ_MEM_RECLAIM, 0);
 	if (!kc->kcopyd_wq)
 		goto bad_workqueue;
drivers/md/dm-raid1.c +1 -2
index fd61f98..c282fc7 100644
@@ -1090,8 +1090,7 @@ static int mirror_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	ti->num_discard_requests = 1;
 	ti->discard_zeroes_data_unsupported = true;
 
-	ms->kmirrord_wq = alloc_workqueue("kmirrord",
-					  WQ_NON_REENTRANT | WQ_MEM_RECLAIM, 0);
+	ms->kmirrord_wq = alloc_workqueue("kmirrord", WQ_MEM_RECLAIM, 0);
 	if (!ms->kmirrord_wq) {
 		DMERR("couldn't start kmirrord");
 		r = -ENOMEM;
drivers/md/dm.c +1 -2
index 4e09b6f..7c5ef85 100644
@@ -1897,8 +1897,7 @@ static struct mapped_device *alloc_dev(int minor)
 	add_disk(md->disk);
 	format_dev_t(md->name, MKDEV(_major, minor));
 
-	md->wq = alloc_workqueue("kdmflush",
-				 WQ_NON_REENTRANT | WQ_MEM_RECLAIM, 0);
+	md->wq = alloc_workqueue("kdmflush", WQ_MEM_RECLAIM, 0);
 	if (!md->wq)
 		goto bad_thread;
drivers/mmc/host/dw_mmc.c +1 -2
index 72dc3cd..119e441 100644
@@ -2030,8 +2030,7 @@ int dw_mci_probe(struct dw_mci *host)
 	mci_writel(host, CLKSRC, 0);
 
 	tasklet_init(&host->tasklet, dw_mci_tasklet_func, (unsigned long)host);
-	host->card_workqueue = alloc_workqueue("dw-mci-card",
-			WQ_MEM_RECLAIM | WQ_NON_REENTRANT, 1);
+	host->card_workqueue = alloc_workqueue("dw-mci-card", WQ_MEM_RECLAIM, 1);
 	if (!host->card_workqueue)
 		goto err_dmaunmap;
 	INIT_WORK(&host->card_work, dw_mci_work_routine_card);
drivers/nfc/pn533.c +2 -3
index d606f52..7dadcb8 100644
@@ -2334,9 +2334,8 @@ static int pn533_probe(struct usb_interface *interface,
 	INIT_WORK(&dev->mi_work, pn533_wq_mi_recv);
 	INIT_WORK(&dev->tg_work, pn533_wq_tg_get_data);
 	INIT_WORK(&dev->poll_work, pn533_wq_poll);
-	dev->wq = alloc_workqueue("pn533",
-				  WQ_NON_REENTRANT | WQ_UNBOUND | WQ_MEM_RECLAIM,
-				  1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	dev->wq = alloc_ordered_workqueue("pn533", WQ_MEM_RECLAIM);
 	if (dev->wq == NULL)
 		goto error;
drivers/staging/nvec/nvec.c +4 -6
index 695ea35..9c39d53 100644
@@ -264,7 +264,7 @@ int nvec_write_async(struct nvec_chip *nvec, const unsigned char *data,
 	list_add_tail(&msg->node, &nvec->tx_data);
 	spin_unlock_irqrestore(&nvec->tx_lock, flags);
 
-	queue_work(nvec->wq, &nvec->tx_work);
+	schedule_work(&nvec->tx_work);
 
 	return 0;
 }

@@ -470,7 +470,7 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
 	if (!nvec_msg_is_event(nvec->rx))
 		complete(&nvec->ec_transfer);
 
-	queue_work(nvec->wq, &nvec->rx_work);
+	schedule_work(&nvec->rx_work);
 }
 
 /**

@@ -791,13 +791,11 @@ static int __devinit tegra_nvec_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&nvec->tx_data);
 	INIT_WORK(&nvec->rx_work, nvec_dispatch);
 	INIT_WORK(&nvec->tx_work, nvec_request_master);
-	nvec->wq = alloc_workqueue("nvec", WQ_NON_REENTRANT, 2);
 
 	err = devm_gpio_request_one(&pdev->dev, nvec->gpio, GPIOF_OUT_INIT_HIGH,
 					"nvec gpio");
 	if (err < 0) {
 		dev_err(nvec->dev, "couldn't request gpio\n");
-		destroy_workqueue(nvec->wq);
 		return -ENODEV;
 	}
 

@@ -805,7 +803,6 @@ static int __devinit tegra_nvec_probe(struct platform_device *pdev)
 				"nvec", nvec);
 	if (err) {
 		dev_err(nvec->dev, "couldn't request irq\n");
-		destroy_workqueue(nvec->wq);
 		return -ENODEV;
 	}
 	disable_irq(nvec->irq);

@@ -859,7 +856,8 @@ static int __devexit tegra_nvec_remove(struct platform_device *pdev)
 
 	nvec_write_async(nvec, EC_DISABLE_EVENT_REPORTING, 3);
 	mfd_remove_devices(nvec->dev);
-	destroy_workqueue(nvec->wq);
+	cancel_work_sync(&nvec->rx_work);
+	cancel_work_sync(&nvec->tx_work);
 
 	return 0;
 }
drivers/staging/nvec/nvec.h +0 -2
index ba6ed8f..f522dbd 100644
@@ -139,7 +139,6 @@ struct nvec_platform_data {
  * @nvec_status_notifier: Internal notifier (see nvec_status_notifier())
  * @rx_work: A work structure for the RX worker nvec_dispatch()
  * @tx_work: A work structure for the TX worker nvec_request_master()
- * @wq: The work queue in which @rx_work and @tx_work are executed
  * @rx: The message currently being retrieved or %NULL
  * @msg_pool: A pool of messages for allocation
  * @tx: The message currently being transferred

@@ -165,7 +164,6 @@ struct nvec_chip {
 	struct list_head rx_data, tx_data;
 	struct notifier_block nvec_status_notifier;
 	struct work_struct rx_work, tx_work;
-	struct workqueue_struct *wq;
 	struct nvec_msg msg_pool[NVEC_POOL_SIZE];
 	struct nvec_msg *rx;
drivers/staging/omapdrm/omap_drv.c +1 -2
index 4beab94..672cb3a 100644
@@ -571,8 +571,7 @@ static int dev_load(struct drm_device *dev, unsigned long flags)
 
 	dev->dev_private = priv;
 
-	priv->wq = alloc_workqueue("omapdrm",
-			WQ_UNBOUND | WQ_NON_REENTRANT, 1);
+	priv->wq = alloc_ordered_workqueue("omapdrm", 0);
 
 	INIT_LIST_HEAD(&priv->obj_list);
fs/dlm/ast.c +1 -4
index 63dc19c..cd24afd 100644
@@ -267,10 +267,7 @@ void dlm_callback_work(struct work_struct *work)
 int dlm_callback_start(struct dlm_ls *ls)
 {
 	ls->ls_callback_wq = alloc_workqueue("dlm_callback",
-					     WQ_UNBOUND |
-					     WQ_MEM_RECLAIM |
-					     WQ_NON_REENTRANT,
-					     0);
+					     WQ_UNBOUND | WQ_MEM_RECLAIM, 0);
 	if (!ls->ls_callback_wq) {
 		log_print("can't start dlm_callback workqueue");
 		return -ENOMEM;
fs/gfs2/main.c +1 -1
index e04d0e0..7b0f504 100644
@@ -155,7 +155,7 @@ static int __init init_gfs2_fs(void)
 		goto fail_wq;
 
 	gfs2_control_wq = alloc_workqueue("gfs2_control",
-			       WQ_NON_REENTRANT | WQ_UNBOUND | WQ_FREEZABLE, 0);
+					  WQ_UNBOUND | WQ_FREEZABLE, 0);
 	if (!gfs2_control_wq)
 		goto fail_recovery;
fs/xfs/xfs_super.c +1 -1
index e8e6be4..c7199a4 100644
@@ -1531,7 +1531,7 @@ xfs_init_workqueues(void)
 	 * competing for ressources.  Use the default large max_active value
 	 * so that even lots of filesystems can perform these task in parallel.
 	 */
-	xfs_syncd_wq = alloc_workqueue("xfssyncd", WQ_NON_REENTRANT, 0);
+	xfs_syncd_wq = alloc_workqueue("xfssyncd", 0, 0);
 	if (!xfs_syncd_wq)
 		return -ENOMEM;
kernel/workqueue.c +3 -0
index ce39467..910b1ee 100644
@@ -3195,6 +3195,9 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
 	unsigned int cpu;
 	size_t namelen;
 
+	WARN(flags & WQ_NON_REENTRANT,
+	     "workqueue: WQ_NON_REENTRANT is deprecated, all workqueues are non-reentrant\n");
+
 	/* determine namelen, allocate wq and format name */
 	va_start(args, lock_name);
 	va_copy(args1, args);
net/ceph/messenger.c +1 -1
index b979675..fe1bd47 100644
@@ -190,7 +190,7 @@ int ceph_msgr_init(void)
 	zero_page = ZERO_PAGE(0);
 	page_cache_get(zero_page);
 
-	ceph_msgr_wq = alloc_workqueue("ceph-msgr", WQ_NON_REENTRANT, 0);
+	ceph_msgr_wq = alloc_workqueue("ceph-msgr", 0, 0);
 	if (ceph_msgr_wq)
 		return 0;
net/nfc/core.c +3 -3
index ff74979..b071f97 100644
@@ -791,9 +791,9 @@ struct nfc_dev *nfc_allocate_device(struct nfc_ops *ops,
 
 		INIT_WORK(&dev->check_pres_work, nfc_check_pres_work);
 		snprintf(name, sizeof(name), "nfc%d_check_pres_wq", dev->idx);
-		dev->check_pres_wq = alloc_workqueue(name, WQ_NON_REENTRANT |
-						     WQ_UNBOUND |
-						     WQ_MEM_RECLAIM, 1);
+		/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+		dev->check_pres_wq = alloc_ordered_workqueue(name,
+							     WQ_MEM_RECLAIM);
 		if (dev->check_pres_wq == NULL) {
 			kfree(dev);
 			return NULL;
net/nfc/hci/core.c +4 -4
index 1ac7b3f..5e48792 100644
@@ -670,8 +670,8 @@ int nfc_hci_register_device(struct nfc_hci_dev *hdev)
 
 	INIT_WORK(&hdev->msg_tx_work, nfc_hci_msg_tx_work);
 	snprintf(name, sizeof(name), "%s_hci_msg_tx_wq", devname);
-	hdev->msg_tx_wq = alloc_workqueue(name, WQ_NON_REENTRANT | WQ_UNBOUND |
-					  WQ_MEM_RECLAIM, 1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	hdev->msg_tx_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (hdev->msg_tx_wq == NULL) {
 		r = -ENOMEM;
 		goto exit;

@@ -685,8 +685,8 @@ int nfc_hci_register_device(struct nfc_hci_dev *hdev)
 
 	INIT_WORK(&hdev->msg_rx_work, nfc_hci_msg_rx_work);
 	snprintf(name, sizeof(name), "%s_hci_msg_rx_wq", devname);
-	hdev->msg_rx_wq = alloc_workqueue(name, WQ_NON_REENTRANT | WQ_UNBOUND |
-					  WQ_MEM_RECLAIM, 1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	hdev->msg_rx_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (hdev->msg_rx_wq == NULL) {
 		r = -ENOMEM;
 		goto exit;
net/nfc/hci/shdlc.c +2 -2
index 6f840c1..04405101 100644
@@ -877,8 +877,8 @@ struct nfc_shdlc *nfc_shdlc_allocate(struct nfc_shdlc_ops *ops,
 
 	INIT_WORK(&shdlc->sm_work, nfc_shdlc_sm_work);
 	snprintf(name, sizeof(name), "%s_shdlc_sm_wq", devname);
-	shdlc->sm_wq = alloc_workqueue(name, WQ_NON_REENTRANT | WQ_UNBOUND |
-				       WQ_MEM_RECLAIM, 1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	shdlc->sm_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (shdlc->sm_wq == NULL)
 		goto err_allocwq;
net/nfc/llcp/llcp.c +6 -12
index 82f0f75..57d5df2 100644
@@ -1150,10 +1150,8 @@ int nfc_llcp_register_device(struct nfc_dev *ndev)
 	skb_queue_head_init(&local->tx_queue);
 	INIT_WORK(&local->tx_work, nfc_llcp_tx_work);
 	snprintf(name, sizeof(name), "%s_llcp_tx_wq", dev_name(dev));
-	local->tx_wq =
-		alloc_workqueue(name,
-				WQ_NON_REENTRANT | WQ_UNBOUND | WQ_MEM_RECLAIM,
-				1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	local->tx_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (local->tx_wq == NULL) {
 		err = -ENOMEM;
 		goto err_local;

@@ -1162,10 +1160,8 @@ int nfc_llcp_register_device(struct nfc_dev *ndev)
 	local->rx_pending = NULL;
 	INIT_WORK(&local->rx_work, nfc_llcp_rx_work);
 	snprintf(name, sizeof(name), "%s_llcp_rx_wq", dev_name(dev));
-	local->rx_wq =
-		alloc_workqueue(name,
-				WQ_NON_REENTRANT | WQ_UNBOUND | WQ_MEM_RECLAIM,
-				1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	local->rx_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (local->rx_wq == NULL) {
 		err = -ENOMEM;
 		goto err_tx_wq;

@@ -1173,10 +1169,8 @@ int nfc_llcp_register_device(struct nfc_dev *ndev)
 
 	INIT_WORK(&local->timeout_work, nfc_llcp_timeout_work);
 	snprintf(name, sizeof(name), "%s_llcp_timeout_wq", dev_name(dev));
-	local->timeout_wq =
-		alloc_workqueue(name,
-				WQ_NON_REENTRANT | WQ_UNBOUND | WQ_MEM_RECLAIM,
-				1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	local->timeout_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (local->timeout_wq == NULL) {
 		err = -ENOMEM;
 		goto err_rx_wq;
Signature · 1 line
1.7.7.3
maintainer Acked-by on v1 6/6
#7 lore raw

Re: [PATCH 6/6] workqueue: deprecate WQ_NON_REENTRANT

Hi Tejun,

Quoted 20 lines · On Monday 13 August 2012 18:49:46 Tejun Heo wrote:

WQ_NON_REENTRANT is now spurious. Make alloc_workqueue() trigger WARN_ON() if the flag is specified and drop it from all in-kernel users.

If you're cc'd and wondering what's going on: Now all workqueues are non-reentrant, so there's no reason to use WQ_NON_REENTRANT.

Some of the conversions in this patch aren't trivial.

* WQ_UNBOUND | WQ_NON_REENTRANT workqueues w/ max_active == 1 are converted to alloc_ordered_workqueue().

* NFC was using a lot of WQ_UNBOUND | WQ_NON_REENTRANT | WQ_MEM_RECLAIM workqueues w/ max_active == 1. I converted them to alloc_ordered_workqueue() but can't understand why NFC would need the reclaim flag at all. I added comments above all such occurrences. If I'm missing something, please let me know; otherwise, please drop the flag. Better, please try to use system_wq instead.

Quoted 3 lines

* drivers/staging/nvec doesn't seem to need its own workqueue. Converted to use system_wq and cancel work items on unload. Plesae scream if it's wrong.

the nvec driver had its own workqueue because we found that it reduced some timimg induced protocol errors. I just tested your patch which uses the system workqueue and I failed to reproduce these protocol errors under stress testing, so I think it's ok.

Because this is a single patch affecting many drivers, is this going through linux-next or some subsystem tree? Maybe it would have been better to split it in one patch per driver. Otherwise ...

Acked-by Marc Dietrich <[email protected]>
Quoted 463 lines
Signed-off-by Tejun Heo <[email protected]>
Cc Stefan Richter <[email protected]>
Cc Daniel Vetter <[email protected]>
Cc Alasdair Kergon <[email protected]>
Cc Lauro Ramos Venancio <[email protected]>
Cc Aloisio Almeida Jr <[email protected]>
Cc Samuel Ortiz <[email protected]>
Cc Julian Andres Klode <[email protected]>
Cc Marc Dietrich <[email protected]>
Cc Rob Clark <[email protected]>
Cc Christine Caulfield <[email protected]>
Cc Steven Whitehouse <[email protected]>
Cc Christoph Hellwig <[email protected]>
Cc Sage Weil <[email protected]>
---
 Documentation/workqueue.txt         |   14 ++------------
 drivers/firewire/core-transaction.c |    3 +--
 drivers/gpu/drm/i915/i915_dma.c     |    6 ++----
 drivers/md/dm-crypt.c               |   10 ++--------
 drivers/md/dm-kcopyd.c              |    3 +--
 drivers/md/dm-raid1.c               |    3 +--
 drivers/md/dm.c                     |    3 +--
 drivers/mmc/host/dw_mmc.c           |    3 +--
 drivers/nfc/pn533.c                 |    5 ++---
 drivers/staging/nvec/nvec.c         |   10 ++++------
 drivers/staging/nvec/nvec.h         |    2 --
 drivers/staging/omapdrm/omap_drv.c  |    3 +--
 fs/dlm/ast.c                        |    5 +----
 fs/gfs2/main.c                      |    2 +-
 fs/xfs/xfs_super.c                  |    2 +-
 kernel/workqueue.c                  |    3 +++
 net/ceph/messenger.c                |    2 +-
 net/nfc/core.c                      |    6 +++---
 net/nfc/hci/core.c                  |    8 ++++----
 net/nfc/hci/shdlc.c                 |    4 ++--
 net/nfc/llcp/llcp.c                 |   18 ++++++------------
 21 files changed, 40 insertions(+), 75 deletions(-)
Documentation/workqueue.txt +0 -0
index a6ab4b6..1fb8813 100644
@@ -100,8 +100,8 @@ Subsystems and drivers can create and queue work items

through special workqueue API functions as they see fit. They can influence some
aspects of the way the work items are executed by setting flags on the workqueue they are putting the work item on. These flags include

Documentation/workqueue.txt +2 -12
-things like CPU locality, reentrancy, concurrency limits, priority and
-more.  To get a detailed overview refer to the API description of
+things like CPU locality, concurrency limits, priority and more.  To
+get a detailed overview refer to the API description of
 alloc_workqueue() below.
 
 When a work item is queued to a workqueue, the target gcwq and

@@ -166,16 +166,6 @@ resources, scheduled and executed.
 
 @flags:
 
-  WQ_NON_REENTRANT
-
-	By default, a wq guarantees non-reentrance only on the same
-	CPU.  A work item may not be executed concurrently on the same
-	CPU by multiple workers but is allowed to be executed
-	concurrently on multiple CPUs.  This flag makes sure
-	non-reentrance is enforced across all CPUs.  Work items queued
-	to a non-reentrant wq are guaranteed to be executed by at most
-	one worker system-wide at any given time.
-
   WQ_UNBOUND
 
 	Work items queued to an unbound wq are served by a special
(quoted diff) +0 -0

b/drivers/firewire/core-transaction.c index 87d6f2d..45acc0f 100644

drivers/firewire/core-transaction.c +1 -2
@@ -1257,8 +1257,7 @@ static int __init fw_core_init(void)
 {
 	int ret;
 
-	fw_workqueue = alloc_workqueue("firewire",
-				       WQ_NON_REENTRANT | WQ_MEM_RECLAIM, 0);
+	fw_workqueue = alloc_workqueue("firewire", WQ_MEM_RECLAIM, 0);
 	if (!fw_workqueue)
 		return -ENOMEM;
(quoted diff) +0 -0

b/drivers/gpu/drm/i915/i915_dma.c index 9cf7dfe..a55ca7a 100644

drivers/gpu/drm/i915/i915_dma.c +0 -0
@@ -1536,11 +1536,9 @@ int i915_driver_load(struct drm_device *dev, unsigned

long flags) *

drivers/gpu/drm/i915/i915_dma.c +2 -4
 	 * All tasks on the workqueue are expected to acquire the dev mutex
 	 * so there is no point in running more than one instance of the
-	 * workqueue at any time: max_active = 1 and NON_REENTRANT.
+	 * workqueue at any time.  Use an ordered one.
 	 */
-	dev_priv->wq = alloc_workqueue("i915",
-				       WQ_UNBOUND | WQ_NON_REENTRANT,
-				       1);
+	dev_priv->wq = alloc_ordered_workqueue("i915", 0);
 	if (dev_priv->wq == NULL) {
 		DRM_ERROR("Failed to create our workqueue.\n");
 		ret = -ENOMEM;
drivers/md/dm-crypt.c +2 -8
index 664743d..a7472d6 100644
@@ -1674,20 +1674,14 @@ static int crypt_ctr(struct dm_target *ti, unsigned
 int argc, char **argv) }
 
 	ret = -ENOMEM;
-	cc->io_queue = alloc_workqueue("kcryptd_io",
-				       WQ_NON_REENTRANT|
-				       WQ_MEM_RECLAIM,
-				       1);
+	cc->io_queue = alloc_workqueue("kcryptd_io", WQ_MEM_RECLAIM, 1);
 	if (!cc->io_queue) {
 		ti->error = "Couldn't create kcryptd io queue";
 		goto bad;
 	}
 
 	cc->crypt_queue = alloc_workqueue("kcryptd",
-					  WQ_NON_REENTRANT|
-					  WQ_CPU_INTENSIVE|
-					  WQ_MEM_RECLAIM,
-					  1);
+					  WQ_CPU_INTENSIVE | WQ_MEM_RECLAIM, 1);
 	if (!cc->crypt_queue) {
 		ti->error = "Couldn't create kcryptd queue";
 		goto bad;
drivers/md/dm-kcopyd.c +1 -2
index bed444c..7234f71 100644
@@ -704,8 +704,7 @@ struct dm_kcopyd_client *dm_kcopyd_client_create(void)
 		goto bad_slab;
 
 	INIT_WORK(&kc->kcopyd_work, do_work);
-	kc->kcopyd_wq = alloc_workqueue("kcopyd",
-					WQ_NON_REENTRANT | WQ_MEM_RECLAIM, 0);
+	kc->kcopyd_wq = alloc_workqueue("kcopyd", WQ_MEM_RECLAIM, 0);
 	if (!kc->kcopyd_wq)
 		goto bad_workqueue;
drivers/md/dm-raid1.c +1 -2
index fd61f98..c282fc7 100644
@@ -1090,8 +1090,7 @@ static int mirror_ctr(struct dm_target *ti, unsigned
 int argc, char **argv) ti->num_discard_requests = 1;
 	ti->discard_zeroes_data_unsupported = true;
 
-	ms->kmirrord_wq = alloc_workqueue("kmirrord",
-					  WQ_NON_REENTRANT | WQ_MEM_RECLAIM, 0);
+	ms->kmirrord_wq = alloc_workqueue("kmirrord", WQ_MEM_RECLAIM, 0);
 	if (!ms->kmirrord_wq) {
 		DMERR("couldn't start kmirrord");
 		r = -ENOMEM;
drivers/md/dm.c +1 -2
index 4e09b6f..7c5ef85 100644
@@ -1897,8 +1897,7 @@ static struct mapped_device *alloc_dev(int minor)
 	add_disk(md->disk);
 	format_dev_t(md->name, MKDEV(_major, minor));
 
-	md->wq = alloc_workqueue("kdmflush",
-				 WQ_NON_REENTRANT | WQ_MEM_RECLAIM, 0);
+	md->wq = alloc_workqueue("kdmflush", WQ_MEM_RECLAIM, 0);
 	if (!md->wq)
 		goto bad_thread;
drivers/mmc/host/dw_mmc.c +1 -2
index 72dc3cd..119e441 100644
@@ -2030,8 +2030,7 @@ int dw_mci_probe(struct dw_mci *host)
 	mci_writel(host, CLKSRC, 0);
 
 	tasklet_init(&host->tasklet, dw_mci_tasklet_func, (unsigned long)host);
-	host->card_workqueue = alloc_workqueue("dw-mci-card",
-			WQ_MEM_RECLAIM | WQ_NON_REENTRANT, 1);
+	host->card_workqueue = alloc_workqueue("dw-mci-card", WQ_MEM_RECLAIM, 1);
 	if (!host->card_workqueue)
 		goto err_dmaunmap;
 	INIT_WORK(&host->card_work, dw_mci_work_routine_card);
drivers/nfc/pn533.c +2 -3
index d606f52..7dadcb8 100644
@@ -2334,9 +2334,8 @@ static int pn533_probe(struct usb_interface
 *interface, INIT_WORK(&dev->mi_work, pn533_wq_mi_recv);
 	INIT_WORK(&dev->tg_work, pn533_wq_tg_get_data);
 	INIT_WORK(&dev->poll_work, pn533_wq_poll);
-	dev->wq = alloc_workqueue("pn533",
-				  WQ_NON_REENTRANT | WQ_UNBOUND | WQ_MEM_RECLAIM,
-				  1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	dev->wq = alloc_ordered_workqueue("pn533", WQ_MEM_RECLAIM);
 	if (dev->wq == NULL)
 		goto error;
drivers/staging/nvec/nvec.c +4 -6
index 695ea35..9c39d53 100644
@@ -264,7 +264,7 @@ int nvec_write_async(struct nvec_chip *nvec, const
 unsigned char *data, list_add_tail(&msg->node, &nvec->tx_data);
 	spin_unlock_irqrestore(&nvec->tx_lock, flags);
 
-	queue_work(nvec->wq, &nvec->tx_work);
+	schedule_work(&nvec->tx_work);
 
 	return 0;
 }

@@ -470,7 +470,7 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
 	if (!nvec_msg_is_event(nvec->rx))
 		complete(&nvec->ec_transfer);
 
-	queue_work(nvec->wq, &nvec->rx_work);
+	schedule_work(&nvec->rx_work);
 }
 
 /**

@@ -791,13 +791,11 @@ static int __devinit tegra_nvec_probe(struct
 platform_device *pdev) INIT_LIST_HEAD(&nvec->tx_data);
 	INIT_WORK(&nvec->rx_work, nvec_dispatch);
 	INIT_WORK(&nvec->tx_work, nvec_request_master);
-	nvec->wq = alloc_workqueue("nvec", WQ_NON_REENTRANT, 2);
 
 	err = devm_gpio_request_one(&pdev->dev, nvec->gpio, GPIOF_OUT_INIT_HIGH,
 					"nvec gpio");
 	if (err < 0) {
 		dev_err(nvec->dev, "couldn't request gpio\n");
-		destroy_workqueue(nvec->wq);
 		return -ENODEV;
 	}
 

@@ -805,7 +803,6 @@ static int __devinit tegra_nvec_probe(struct
 platform_device *pdev) "nvec", nvec);
 	if (err) {
 		dev_err(nvec->dev, "couldn't request irq\n");
-		destroy_workqueue(nvec->wq);
 		return -ENODEV;
 	}
 	disable_irq(nvec->irq);

@@ -859,7 +856,8 @@ static int __devexit tegra_nvec_remove(struct
 platform_device *pdev)
 
 	nvec_write_async(nvec, EC_DISABLE_EVENT_REPORTING, 3);
 	mfd_remove_devices(nvec->dev);
-	destroy_workqueue(nvec->wq);
+	cancel_work_sync(&nvec->rx_work);
+	cancel_work_sync(&nvec->tx_work);
 
 	return 0;

}

drivers/staging/nvec/nvec.h +0 -2
index ba6ed8f..f522dbd 100644
@@ -139,7 +139,6 @@ struct nvec_platform_data {
  * @nvec_status_notifier: Internal notifier (see nvec_status_notifier())
  * @rx_work: A work structure for the RX worker nvec_dispatch()
  * @tx_work: A work structure for the TX worker nvec_request_master()
- * @wq: The work queue in which @rx_work and @tx_work are executed
  * @rx: The message currently being retrieved or %NULL
  * @msg_pool: A pool of messages for allocation
  * @tx: The message currently being transferred

@@ -165,7 +164,6 @@ struct nvec_chip {
 	struct list_head rx_data, tx_data;
 	struct notifier_block nvec_status_notifier;
 	struct work_struct rx_work, tx_work;
-	struct workqueue_struct *wq;
 	struct nvec_msg msg_pool[NVEC_POOL_SIZE];
 	struct nvec_msg *rx;
(quoted diff) +0 -0

b/drivers/staging/omapdrm/omap_drv.c index 4beab94..672cb3a 100644

drivers/staging/omapdrm/omap_drv.c +1 -2
@@ -571,8 +571,7 @@ static int dev_load(struct drm_device *dev, unsigned
 long flags)
 
 	dev->dev_private = priv;
 
-	priv->wq = alloc_workqueue("omapdrm",
-			WQ_UNBOUND | WQ_NON_REENTRANT, 1);
+	priv->wq = alloc_ordered_workqueue("omapdrm", 0);
 
 	INIT_LIST_HEAD(&priv->obj_list);
fs/dlm/ast.c +1 -4
index 63dc19c..cd24afd 100644
@@ -267,10 +267,7 @@ void dlm_callback_work(struct work_struct *work)
 int dlm_callback_start(struct dlm_ls *ls)
 {
 	ls->ls_callback_wq = alloc_workqueue("dlm_callback",
-					     WQ_UNBOUND |
-					     WQ_MEM_RECLAIM |
-					     WQ_NON_REENTRANT,
-					     0);
+					     WQ_UNBOUND | WQ_MEM_RECLAIM, 0);
 	if (!ls->ls_callback_wq) {
 		log_print("can't start dlm_callback workqueue");
 		return -ENOMEM;
fs/gfs2/main.c +1 -1
index e04d0e0..7b0f504 100644
@@ -155,7 +155,7 @@ static int __init init_gfs2_fs(void)
 		goto fail_wq;
 
 	gfs2_control_wq = alloc_workqueue("gfs2_control",
-			       WQ_NON_REENTRANT | WQ_UNBOUND | WQ_FREEZABLE, 0);
+					  WQ_UNBOUND | WQ_FREEZABLE, 0);
 	if (!gfs2_control_wq)
 		goto fail_recovery;
fs/xfs/xfs_super.c +1 -1
index e8e6be4..c7199a4 100644
@@ -1531,7 +1531,7 @@ xfs_init_workqueues(void)
 	 * competing for ressources.  Use the default large max_active value
 	 * so that even lots of filesystems can perform these task in parallel.
 	 */
-	xfs_syncd_wq = alloc_workqueue("xfssyncd", WQ_NON_REENTRANT, 0);
+	xfs_syncd_wq = alloc_workqueue("xfssyncd", 0, 0);
 	if (!xfs_syncd_wq)
 		return -ENOMEM;
kernel/workqueue.c +2 -0
index ce39467..910b1ee 100644
@@ -3195,6 +3195,9 @@ struct workqueue_struct *__alloc_workqueue_key(const
 char *fmt, unsigned int cpu;
 	size_t namelen;
 
+	WARN(flags & WQ_NON_REENTRANT,
+	     "workqueue: WQ_NON_REENTRANT is deprecated, all workqueues are

non-reentrant\n"); +

kernel/workqueue.c +0 -0
 	/* determine namelen, allocate wq and format name */
 	va_start(args, lock_name);
 	va_copy(args1, args);
net/ceph/messenger.c +1 -1
index b979675..fe1bd47 100644
@@ -190,7 +190,7 @@ int ceph_msgr_init(void)
 	zero_page = ZERO_PAGE(0);
 	page_cache_get(zero_page);
 
-	ceph_msgr_wq = alloc_workqueue("ceph-msgr", WQ_NON_REENTRANT, 0);
+	ceph_msgr_wq = alloc_workqueue("ceph-msgr", 0, 0);
 	if (ceph_msgr_wq)
 		return 0;
net/nfc/core.c +3 -3
index ff74979..b071f97 100644
@@ -791,9 +791,9 @@ struct nfc_dev *nfc_allocate_device(struct nfc_ops *ops,
 
 		INIT_WORK(&dev->check_pres_work, nfc_check_pres_work);
 		snprintf(name, sizeof(name), "nfc%d_check_pres_wq", dev->idx);
-		dev->check_pres_wq = alloc_workqueue(name, WQ_NON_REENTRANT |
-						     WQ_UNBOUND |
-						     WQ_MEM_RECLAIM, 1);
+		/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+		dev->check_pres_wq = alloc_ordered_workqueue(name,
+							     WQ_MEM_RECLAIM);
 		if (dev->check_pres_wq == NULL) {
 			kfree(dev);
 			return NULL;
net/nfc/hci/core.c +4 -4
index 1ac7b3f..5e48792 100644
@@ -670,8 +670,8 @@ int nfc_hci_register_device(struct nfc_hci_dev *hdev)
 
 	INIT_WORK(&hdev->msg_tx_work, nfc_hci_msg_tx_work);
 	snprintf(name, sizeof(name), "%s_hci_msg_tx_wq", devname);
-	hdev->msg_tx_wq = alloc_workqueue(name, WQ_NON_REENTRANT | WQ_UNBOUND |
-					  WQ_MEM_RECLAIM, 1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	hdev->msg_tx_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (hdev->msg_tx_wq == NULL) {
 		r = -ENOMEM;
 		goto exit;

@@ -685,8 +685,8 @@ int nfc_hci_register_device(struct nfc_hci_dev *hdev)
 
 	INIT_WORK(&hdev->msg_rx_work, nfc_hci_msg_rx_work);
 	snprintf(name, sizeof(name), "%s_hci_msg_rx_wq", devname);
-	hdev->msg_rx_wq = alloc_workqueue(name, WQ_NON_REENTRANT | WQ_UNBOUND |
-					  WQ_MEM_RECLAIM, 1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	hdev->msg_rx_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (hdev->msg_rx_wq == NULL) {
 		r = -ENOMEM;
 		goto exit;
net/nfc/hci/shdlc.c +2 -2
index 6f840c1..04405101 100644
@@ -877,8 +877,8 @@ struct nfc_shdlc *nfc_shdlc_allocate(struct
 nfc_shdlc_ops *ops,
 
 	INIT_WORK(&shdlc->sm_work, nfc_shdlc_sm_work);
 	snprintf(name, sizeof(name), "%s_shdlc_sm_wq", devname);
-	shdlc->sm_wq = alloc_workqueue(name, WQ_NON_REENTRANT | WQ_UNBOUND |
-				       WQ_MEM_RECLAIM, 1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	shdlc->sm_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (shdlc->sm_wq == NULL)
 		goto err_allocwq;
net/nfc/llcp/llcp.c +6 -12
index 82f0f75..57d5df2 100644
@@ -1150,10 +1150,8 @@ int nfc_llcp_register_device(struct nfc_dev *ndev)
 	skb_queue_head_init(&local->tx_queue);
 	INIT_WORK(&local->tx_work, nfc_llcp_tx_work);
 	snprintf(name, sizeof(name), "%s_llcp_tx_wq", dev_name(dev));
-	local->tx_wq =
-		alloc_workqueue(name,
-				WQ_NON_REENTRANT | WQ_UNBOUND | WQ_MEM_RECLAIM,
-				1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	local->tx_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (local->tx_wq == NULL) {
 		err = -ENOMEM;
 		goto err_local;

@@ -1162,10 +1160,8 @@ int nfc_llcp_register_device(struct nfc_dev *ndev)
 	local->rx_pending = NULL;
 	INIT_WORK(&local->rx_work, nfc_llcp_rx_work);
 	snprintf(name, sizeof(name), "%s_llcp_rx_wq", dev_name(dev));
-	local->rx_wq =
-		alloc_workqueue(name,
-				WQ_NON_REENTRANT | WQ_UNBOUND | WQ_MEM_RECLAIM,
-				1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	local->rx_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (local->rx_wq == NULL) {
 		err = -ENOMEM;
 		goto err_tx_wq;

@@ -1173,10 +1169,8 @@ int nfc_llcp_register_device(struct nfc_dev *ndev)
 
 	INIT_WORK(&local->timeout_work, nfc_llcp_timeout_work);
 	snprintf(name, sizeof(name), "%s_llcp_timeout_wq", dev_name(dev));
-	local->timeout_wq =
-		alloc_workqueue(name,
-				WQ_NON_REENTRANT | WQ_UNBOUND | WQ_MEM_RECLAIM,
-				1);
+	/* XXX tj - why does NFC need WQ_MEM_RECLAIM? */
+	local->timeout_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
 	if (local->timeout_wq == NULL) {
 		err = -ENOMEM;
 		goto err_rx_wq;
author maintainer
#8 lore raw

Re: [PATCH 6/6] workqueue: deprecate WQ_NON_REENTRANT

Hello,

Quoted 4 lines · Marc Dietrich

the nvec driver had its own workqueue because we found that it reduced some timimg induced protocol errors. I just tested your patch which uses the system workqueue and I failed to reproduce these protocol errors under stress testing, so I think it's ok.

Thanks, there shouldn't be any difference whether you use system_wq or your own.

Quoted 3 lines

Because this is a single patch affecting many drivers, is this going through linux-next or some subsystem tree? Maybe it would have been better to split it in one patch per driver. Otherwise ...

I generally think it's better to do quick one time conversion for things like this, but this patch is rather borderline. It might be better to separate out the non trivialones and route them separately. I'll think more about it.

Thanks.

Signature · 1 line
tejun
author maintainer
#9 lore raw
In reply to Tejun Heo
Quoted 2 lines · Tejun Heo

This patchset enforces WQ_NON_REENTRANT behavior on all workqueues and simplifies workqueue API accordingly.

Applied 1-5 to wq/for-3.7. I'll break up the last patch further and route it separately.

Thanks.

Signature · 1 line
tejun