LinuxLists.cc - [PATCHSET wq/for-3.12] workqueue: WQ_NON_REENTRANT is meaningless and going away

2013-07-30 12:40:36

Subject: [PATCHSET wq/for-3.12] workqueue: WQ_NON_REENTRANT is meaningless and going away

Hello,

dbf2576e37 ("workqueue: make all workqueues non-reentrant") made
WQ_NON_REENTRANT no-op but the following patches didn't remove the
flag or update the documentation. Let's mark the flag deprecated and
update the documentation accordingly and remove the existing usages.
We can remove the flag after the next merge window.

This patchset contains the following nine patches.

0001-workqueue-mark-WQ_NON_REENTRANT-deprecated.patch
0002-dm-WQ_NON_REENTRANT-is-meaningless-and-going-away.patch
0003-mmc-WQ_NON_REENTRANT-is-meaningless-and-going-away.patch
0004-firewire-WQ_NON_REENTRANT-is-meaningless-and-going-a.patch
0005-dlm-WQ_NON_REENTRANT-is-meaningless-and-going-away.patch
0006-gfs2-WQ_NON_REENTRANT-is-meaningless-and-going-away.patch
0007-xfs-WQ_NON_REENTRANT-is-meaningless-and-going-away.patch
0008-ceph-WQ_NON_REENTRANT-is-meaningless-and-going-away.patch
0009-l2tp-WQ_NON_REENTRANT-is-meaningless-and-going-away.patch

0001 marks the flag deprecated. The rest remove the exising usages.
This patchset as a whole doesn't introduce any behavior changes.

0001 is already applied to wq/for-3.12. The rest will be routed
through the respective trees. The patches are also available in the
following git branch.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git review-WQ_NON_REENTRANT-removal

Documentation/workqueue.txt | 18 ++++++------------
drivers/firewire/core-transaction.c | 3 +--
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 | 2 +-
fs/dlm/ast.c | 5 +----
fs/gfs2/main.c | 2 +-
fs/xfs/xfs_super.c | 6 +++---
include/linux/workqueue.h | 7 ++++++-
net/ceph/messenger.c | 2 +-
net/l2tp/l2tp_core.c | 2 +-
13 files changed, 26 insertions(+), 40 deletions(-)

Thanks.

--
tejun

2013-07-30 12:40:39

by Tejun Heo

[permalink] [raw]

Subject: [PATCH 1/9] workqueue: mark WQ_NON_REENTRANT deprecated

dbf2576e37 ("workqueue: make all workqueues non-reentrant") made
WQ_NON_REENTRANT no-op but the following patches didn't remove the
flag or update the documentation. Let's mark the flag deprecated and
update the documentation accordingly.

Signed-off-by: Tejun Heo <[email protected]>
---
Documentation/workqueue.txt | 18 ++++++------------
include/linux/workqueue.h | 7 ++++++-
2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/Documentation/workqueue.txt b/Documentation/workqueue.txt
index a6ab4b6..67113f6 100644
--- a/Documentation/workqueue.txt
+++ b/Documentation/workqueue.txt
@@ -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
@@ -233,6 +223,10 @@ resources, scheduled and executed.

This flag is meaningless for unbound wq.

+Note that the flag WQ_NON_REENTRANT no longer exists as all workqueues
+are now non-reentrant - any work item is guaranteed to be executed by
+at most one worker system-wide at any given time.
+
@max_active:

@max_active determines the maximum number of execution contexts per
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index a0ed78a..594521b 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -295,7 +295,12 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
* Documentation/workqueue.txt.
*/
enum {
- WQ_NON_REENTRANT = 1 << 0, /* guarantee non-reentrance */
+ /*
+ * All wqs are now non-reentrant making the following flag
+ * meaningless. Will be removed.
+ */
+ WQ_NON_REENTRANT = 1 << 0, /* DEPRECATED */
+
WQ_UNBOUND = 1 << 1, /* not bound to any cpu */
WQ_FREEZABLE = 1 << 2, /* freeze during suspend */
WQ_MEM_RECLAIM = 1 << 3, /* may be used for memory reclaim */
--
1.8.3.1

2013-07-30 12:40:49

by Tejun Heo

[permalink] [raw]

Subject: [PATCH 4/9] firewire: WQ_NON_REENTRANT is meaningless and going away

2013-07-30 12:40:50

by Tejun Heo

[permalink] [raw]

Subject: [PATCH 3/9] mmc: WQ_NON_REENTRANT is meaningless and going away

2013-07-30 12:40:47

by Tejun Heo

[permalink] [raw]

Subject: [PATCH 5/9] dlm: WQ_NON_REENTRANT is meaningless and going away

2013-07-30 12:41:33

by Tejun Heo

[permalink] [raw]

Subject: [PATCH 9/9] l2tp: WQ_NON_REENTRANT is meaningless and going away

2013-07-30 12:42:00

by Tejun Heo

[permalink] [raw]

Subject: [PATCH 8/9] ceph: WQ_NON_REENTRANT is meaningless and going away

2013-07-30 12:40:46

by Tejun Heo

[permalink] [raw]

Subject: [PATCH 6/9] gfs2: WQ_NON_REENTRANT is meaningless and going away

2013-07-30 12:43:07

by Tejun Heo

[permalink] [raw]

Subject: [PATCH 7/9] xfs: WQ_NON_REENTRANT is meaningless and going away

2013-07-30 12:43:30

by Tejun Heo

[permalink] [raw]

Subject: [PATCH 2/9] dm: WQ_NON_REENTRANT is meaningless and going away

Hello,

Please route this through the subsystem tree. As written in the
description, this shouldn't make any functional difference and just
prepares for the removal of WQ_NON_REENTRANT which is already noop.

Thanks.

------ 8< -------
dbf2576e37 ("workqueue: make all workqueues non-reentrant") made
WQ_NON_REENTRANT no-op and the flag is going away. Remove its usages.

This patch doesn't introduce any behavior changes.

Signed-off-by: Tejun Heo <[email protected]>
Cc: Alasdair Kergon <[email protected]>
Cc: Mike Snitzer <[email protected]>
Cc: [email protected]
---
drivers/md/dm-crypt.c | 10 ++--------
drivers/md/dm-kcopyd.c | 3 +--
drivers/md/dm-raid1.c | 3 +--
drivers/md/dm.c | 3 +--
4 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 6d2d41a..0fce0bc 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1645,20 +1645,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;
diff --git a/drivers/md/dm-kcopyd.c b/drivers/md/dm-kcopyd.c
index d581fe5..3a7cade 100644
--- a/drivers/md/dm-kcopyd.c
+++ b/drivers/md/dm-kcopyd.c
@@ -833,8 +833,7 @@ struct dm_kcopyd_client *dm_kcopyd_client_create(struct dm_kcopyd_throttle *thro
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;

diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c
index 699b5be..9584443 100644
--- a/drivers/md/dm-raid1.c
+++ b/drivers/md/dm-raid1.c
@@ -1080,8 +1080,7 @@ static int mirror_ctr(struct dm_target *ti, unsigned int argc, char **argv)
ti->per_bio_data_size = sizeof(struct dm_raid1_bio_record);
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;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 9e39d2b..ef095a9 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1946,8 +1946,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;

--
1.8.3.1

2013-07-30 13:52:46

by Stefan Richter

[permalink] [raw]

Subject: Re: [PATCH 4/9] firewire: WQ_NON_REENTRANT is meaningless and going away

On Jul 30 Tejun Heo wrote:
> Please route this through the subsystem tree. As written in the
> description, this shouldn't make any functional difference and just
> prepares for the removal of WQ_NON_REENTRANT which is already noop.

Committed to linux1394.git and pushed out to the for-next branch.

> ------ 8< -------
> dbf2576e37 ("workqueue: make all workqueues non-reentrant") made
> WQ_NON_REENTRANT no-op and the flag is going away. Remove its usages.
>
> This patch doesn't introduce any behavior changes.
>
> Signed-off-by: Tejun Heo <[email protected]>
> Cc: Stefan Richter <[email protected]>
> Cc: [email protected]
> ---
> drivers/firewire/core-transaction.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
> index 28a94c7..e5af0e3 100644
> --- a/drivers/firewire/core-transaction.c
> +++ b/drivers/firewire/core-transaction.c
> @@ -1262,8 +1262,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;
>

--
Stefan Richter
-=====-===-= -=== ====-
http://arcgraph.de/sr/

2013-07-30 14:10:49

by Steven Whitehouse

[permalink] [raw]

Subject: Re: [PATCH 6/9] gfs2: WQ_NON_REENTRANT is meaningless and going away

Hi,

On Tue, 2013-07-30 at 08:40 -0400, Tejun Heo wrote:
> Hello,
>
> Please route this through the subsystem tree. As written in the
> description, this shouldn't make any functional difference and just
> prepares for the removal of WQ_NON_REENTRANT which is already noop.
>
> Thanks.
>
Now in the GFS2 -nmw tree. Thanks,

Steve.

> ------ 8< -------
> dbf2576e37 ("workqueue: make all workqueues non-reentrant") made
> WQ_NON_REENTRANT no-op and the flag is going away. Remove its usages.
>
> This patch doesn't introduce any behavior changes.
>
> Signed-off-by: Tejun Heo <[email protected]>
> Cc: Steven Whitehouse <[email protected]>
> Cc: [email protected]
> ---
> fs/gfs2/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/main.c b/fs/gfs2/main.c
> index e04d0e0..7b0f504 100644
> --- a/fs/gfs2/main.c
> +++ b/fs/gfs2/main.c
> @@ -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;
>

2013-07-30 18:13:50

by Ben Myers

[permalink] [raw]

Subject: Re: [PATCH 7/9] xfs: WQ_NON_REENTRANT is meaningless and going away

On Tue, Jul 30, 2013 at 08:40:26AM -0400, Tejun Heo wrote:
> Hello,
>
> Please route this through the subsystem tree. As written in the
> description, this shouldn't make any functional difference and just
> prepares for the removal of WQ_NON_REENTRANT which is already noop.
>
> Thanks.
>
> ------ 8< -------
> dbf2576e37 ("workqueue: make all workqueues non-reentrant") made
> WQ_NON_REENTRANT no-op and the flag is going away. Remove its usages.
>
> This patch doesn't introduce any behavior changes.
>
> Signed-off-by: Tejun Heo <[email protected]>
> Cc: Ben Myers <[email protected]>
> Cc: Alex Elder <[email protected]>
> Cc: [email protected]

Looks good.
Reviewed-by: Ben Myers <[email protected]>

Applied.

2013-07-31 06:08:36

by Lai Jiangshan

[permalink] [raw]

Subject: Re: [PATCH 1/9] workqueue: mark WQ_NON_REENTRANT deprecated

On 07/30/2013 08:40 PM, Tejun Heo wrote:
> dbf2576e37 ("workqueue: make all workqueues non-reentrant") made
> WQ_NON_REENTRANT no-op but the following patches didn't remove the
> flag or update the documentation. Let's mark the flag deprecated and
> update the documentation accordingly.
>
> Signed-off-by: Tejun Heo <[email protected]>

Acked-by: Lai Jiangshan <[email protected]>

> ---
> Documentation/workqueue.txt | 18 ++++++------------
> include/linux/workqueue.h | 7 ++++++-
> 2 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/workqueue.txt b/Documentation/workqueue.txt
> index a6ab4b6..67113f6 100644
> --- a/Documentation/workqueue.txt
> +++ b/Documentation/workqueue.txt
> @@ -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
> @@ -233,6 +223,10 @@ resources, scheduled and executed.
>
> This flag is meaningless for unbound wq.
>
> +Note that the flag WQ_NON_REENTRANT no longer exists as all workqueues
> +are now non-reentrant - any work item is guaranteed to be executed by
> +at most one worker system-wide at any given time.
> +
> @max_active:
>
> @max_active determines the maximum number of execution contexts per
> diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
> index a0ed78a..594521b 100644
> --- a/include/linux/workqueue.h
> +++ b/include/linux/workqueue.h
> @@ -295,7 +295,12 @@ static inline unsigned int work_static(struct work_struct *work) { return 0; }
> * Documentation/workqueue.txt.
> */
> enum {
> - WQ_NON_REENTRANT = 1 << 0, /* guarantee non-reentrance */
> + /*
> + * All wqs are now non-reentrant making the following flag
> + * meaningless. Will be removed.
> + */
> + WQ_NON_REENTRANT = 1 << 0, /* DEPRECATED */
> +
> WQ_UNBOUND = 1 << 1, /* not bound to any cpu */
> WQ_FREEZABLE = 1 << 2, /* freeze during suspend */
> WQ_MEM_RECLAIM = 1 << 3, /* may be used for memory reclaim */