All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup
@ 2023-04-21  2:50 Tejun Heo
  2023-04-21  2:50   ` Tejun Heo
                   ` (22 more replies)
  0 siblings, 23 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai; +Cc: linux-kernel, kernel-team

Hello,

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary
and contains the following 22 patches.

 0001-powerpc-workqueue-Use-alloc_ordered_workqueue-to-cre.patch
 0002-greybus-Use-alloc_ordered_workqueue-to-create-ordere.patch
 0003-IB-hfi1-Use-alloc_ordered_workqueue-to-create-ordere.patch
 0004-dm-integrity-Use-alloc_ordered_workqueue-to-create-o.patch
 0005-media-amphion-Use-alloc_ordered_workqueue-to-create-.patch
 0006-net-thunderx-Use-alloc_ordered_workqueue-to-create-o.patch
 0007-net-octeontx2-Use-alloc_ordered_workqueue-to-create-.patch
 0008-wifi-ath10-11-12k-Use-alloc_ordered_workqueue-to-cre.patch
 0009-wifi-iwlwifi-Use-alloc_ordered_workqueue-to-create-o.patch
 0010-wifi-mwifiex-Use-alloc_ordered_workqueue-to-create-o.patch
 0011-net-wwan-t7xx-Use-alloc_ordered_workqueue-to-create-.patch
 0012-scsi-Use-alloc_ordered_workqueue-to-create-ordered-w.patch
 0013-virt-acrn-Use-alloc_ordered_workqueue-to-create-orde.patch
 0014-soc-qcom-qmi-Use-alloc_ordered_workqueue-to-create-o.patch
 0015-xen-pvcalls-Use-alloc_ordered_workqueue-to-create-or.patch
 0016-btrfs-Use-alloc_ordered_workqueue-to-create-ordered-.patch
 0017-cifs-Use-alloc_ordered_workqueue-to-create-ordered-w.patch
 0018-net-qrtr-Use-alloc_ordered_workqueue-to-create-order.patch
 0019-rxrpc-Use-alloc_ordered_workqueue-to-create-ordered-.patch
 0020-crypto-octeontx2-Use-alloc_ordered_workqueue-to-crea.patch
 0021-media-coda-Use-alloc_ordered_workqueue-to-create-ord.patch
 0022-workqueue-Don-t-implicitly-make-UNBOUND-workqueues-w.patch

0001-0021 convert the existing users and 0022 drops the implicit ordered
promotion logic from alloc_workqueue(). I'll keep an eye out for a while
after merging 0022. Thankfully, these are pretty easy to grep for. The
patches can also be found in the following git branch.

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git ordered-cleanup

diffstat follows. Thanks.

 arch/powerpc/kernel/tau_6xx.c                        |    2 -
 arch/powerpc/platforms/pseries/dlpar.c               |    3 --
 drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c   |   12 ++++-----
 drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c   |    6 ++--
 drivers/greybus/connection.c                         |    4 +--
 drivers/greybus/svc.c                                |    2 -
 drivers/infiniband/hw/hfi1/init.c                    |    7 ++---
 drivers/md/dm-integrity.c                            |    4 +--
 drivers/md/dm.c                                      |    2 -
 drivers/media/platform/amphion/vpu_core.c            |    2 -
 drivers/media/platform/amphion/vpu_v4l2.c            |    2 -
 drivers/media/platform/chips-media/coda-common.c     |    2 -
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c    |    3 --
 drivers/net/ethernet/marvell/octeontx2/af/rvu.c      |    5 +---
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c |   13 ++++------
 drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c |    5 +---
 drivers/net/wireless/ath/ath10k/qmi.c                |    3 --
 drivers/net/wireless/ath/ath11k/qmi.c                |    3 --
 drivers/net/wireless/ath/ath12k/qmi.c                |    3 --
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c      |    4 +--
 drivers/net/wireless/marvell/mwifiex/cfg80211.c      |   13 ++++------
 drivers/net/wireless/marvell/mwifiex/main.c          |   22 ++++++++----------
 drivers/net/wwan/t7xx/t7xx_hif_cldma.c               |   13 +++++-----
 drivers/net/wwan/t7xx/t7xx_hif_dpmaif_tx.c           |    5 ++--
 drivers/scsi/NCR5380.c                               |    5 +---
 drivers/scsi/hosts.c                                 |   12 ++++-----
 drivers/scsi/libiscsi.c                              |    5 +---
 drivers/soc/qcom/qmi_interface.c                     |    2 -
 drivers/virt/acrn/ioreq.c                            |    4 +--
 drivers/xen/pvcalls-back.c                           |    4 +--
 fs/btrfs/disk-io.c                                   |    2 -
 fs/btrfs/scrub.c                                     |    6 +++-
 fs/cifs/dfs_cache.c                                  |    2 -
 include/linux/workqueue.h                            |    4 ---
 kernel/workqueue.c                                   |   23 +++----------------
 net/qrtr/ns.c                                        |    2 -
 net/rxrpc/af_rxrpc.c                                 |    2 -
 37 files changed, 92 insertions(+), 121 deletions(-)


^ permalink raw reply	[flat|nested] 70+ messages in thread

* [PATCH 01/22] powerpc, workqueue: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
@ 2023-04-21  2:50   ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 02/22] greybus: " Tejun Heo
                     ` (21 subsequent siblings)
  22 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Michael Ellerman,
	Nicholas Piggin, Christophe Leroy, Nathan Lynch, linuxppc-dev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/kernel/tau_6xx.c          | 2 +-
 arch/powerpc/platforms/pseries/dlpar.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
index 828d0f4106d2..cba6dd15de3b 100644
--- a/arch/powerpc/kernel/tau_6xx.c
+++ b/arch/powerpc/kernel/tau_6xx.c
@@ -200,7 +200,7 @@ static int __init TAU_init(void)
 	tau_int_enable = IS_ENABLED(CONFIG_TAU_INT) &&
 			 !strcmp(cur_cpu_spec->platform, "ppc750");
 
-	tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1);
+	tau_workq = alloc_ordered_workqueue("tau", 0);
 	if (!tau_workq)
 		return -ENOMEM;
 
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 75ffdbcd2865..e9117b03807e 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -564,8 +564,7 @@ int __init dlpar_workqueue_init(void)
 	if (pseries_hp_wq)
 		return 0;
 
-	pseries_hp_wq = alloc_workqueue("pseries hotplug workqueue",
-			WQ_UNBOUND, 1);
+	pseries_hp_wq = alloc_ordered_workqueue("pseries hotplug workqueue", 0);
 
 	return pseries_hp_wq ? 0 : -ENOMEM;
 }
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 01/22] powerpc, workqueue: Use alloc_ordered_workqueue() to create ordered workqueues
@ 2023-04-21  2:50   ` Tejun Heo
  0 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: Nathan Lynch, kernel-team, linux-kernel, Nicholas Piggin,
	Tejun Heo, linuxppc-dev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/kernel/tau_6xx.c          | 2 +-
 arch/powerpc/platforms/pseries/dlpar.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
index 828d0f4106d2..cba6dd15de3b 100644
--- a/arch/powerpc/kernel/tau_6xx.c
+++ b/arch/powerpc/kernel/tau_6xx.c
@@ -200,7 +200,7 @@ static int __init TAU_init(void)
 	tau_int_enable = IS_ENABLED(CONFIG_TAU_INT) &&
 			 !strcmp(cur_cpu_spec->platform, "ppc750");
 
-	tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1);
+	tau_workq = alloc_ordered_workqueue("tau", 0);
 	if (!tau_workq)
 		return -ENOMEM;
 
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 75ffdbcd2865..e9117b03807e 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -564,8 +564,7 @@ int __init dlpar_workqueue_init(void)
 	if (pseries_hp_wq)
 		return 0;
 
-	pseries_hp_wq = alloc_workqueue("pseries hotplug workqueue",
-			WQ_UNBOUND, 1);
+	pseries_hp_wq = alloc_ordered_workqueue("pseries hotplug workqueue", 0);
 
 	return pseries_hp_wq ? 0 : -ENOMEM;
 }
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 02/22] greybus: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
  2023-04-21  2:50   ` Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21  5:03   ` Greg Kroah-Hartman
                     ` (3 more replies)
  2023-04-21  2:50 ` [PATCH 03/22] IB/hfi1: " Tejun Heo
                   ` (20 subsequent siblings)
  22 siblings, 4 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, greybus-dev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Alex Elder <elder@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: greybus-dev@lists.linaro.org
---
 drivers/greybus/connection.c | 4 ++--
 drivers/greybus/svc.c        | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/greybus/connection.c b/drivers/greybus/connection.c
index e3799a53a193..9c88861986c8 100644
--- a/drivers/greybus/connection.c
+++ b/drivers/greybus/connection.c
@@ -187,8 +187,8 @@ _gb_connection_create(struct gb_host_device *hd, int hd_cport_id,
 	spin_lock_init(&connection->lock);
 	INIT_LIST_HEAD(&connection->operations);
 
-	connection->wq = alloc_workqueue("%s:%d", WQ_UNBOUND, 1,
-					 dev_name(&hd->dev), hd_cport_id);
+	connection->wq = alloc_ordered_workqueue("%s:%d", 0, dev_name(&hd->dev),
+						 hd_cport_id);
 	if (!connection->wq) {
 		ret = -ENOMEM;
 		goto err_free_connection;
diff --git a/drivers/greybus/svc.c b/drivers/greybus/svc.c
index 16cced80867a..0d7e749174a4 100644
--- a/drivers/greybus/svc.c
+++ b/drivers/greybus/svc.c
@@ -1318,7 +1318,7 @@ struct gb_svc *gb_svc_create(struct gb_host_device *hd)
 	if (!svc)
 		return NULL;
 
-	svc->wq = alloc_workqueue("%s:svc", WQ_UNBOUND, 1, dev_name(&hd->dev));
+	svc->wq = alloc_ordered_workqueue("%s:svc", 0, dev_name(&hd->dev));
 	if (!svc->wq) {
 		kfree(svc);
 		return NULL;
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 03/22] IB/hfi1: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
  2023-04-21  2:50   ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 02/22] greybus: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21 14:02   ` Dennis Dalessandro
  2023-04-21  2:50   ` [dm-devel] " Tejun Heo
                   ` (19 subsequent siblings)
  22 siblings, 1 reply; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Dennis Dalessandro,
	Jason Gunthorpe, Leon Romanovsky, linux-rdma

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: linux-rdma@vger.kernel.org
---
 drivers/infiniband/hw/hfi1/init.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c
index 62b6c5020039..e03d867cda13 100644
--- a/drivers/infiniband/hw/hfi1/init.c
+++ b/drivers/infiniband/hw/hfi1/init.c
@@ -755,14 +755,13 @@ static int create_workqueues(struct hfi1_devdata *dd)
 		}
 		if (!ppd->link_wq) {
 			/*
-			 * Make the link workqueue single-threaded to enforce
+			 * Make the link workqueue ordered to enforce
 			 * serialization.
 			 */
 			ppd->link_wq =
-				alloc_workqueue(
+				alloc_ordered_workqueue(
 				    "hfi_link_%d_%d",
-				    WQ_SYSFS | WQ_MEM_RECLAIM | WQ_UNBOUND,
-				    1, /* max_active */
+				    WQ_SYSFS | WQ_MEM_RECLAIM,
 				    dd->unit, pidx);
 			if (!ppd->link_wq)
 				goto wq_error;
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 04/22] dm integrity: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
@ 2023-04-21  2:50   ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 02/22] greybus: " Tejun Heo
                     ` (21 subsequent siblings)
  22 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Alasdair Kergon,
	Mike Snitzer, dm-devel

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Alasdair Kergon <agk@redhat.com>
Cc: Mike Snitzer <snitzer@kernel.org>
Cc: dm-devel@redhat.com
Cc: linux-kernel@vger.kernel.org
---
 drivers/md/dm-integrity.c | 4 ++--
 drivers/md/dm.c           | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index b0d5057fbdd9..6b36554f9103 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -4268,10 +4268,10 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
 	}
 
 	/*
-	 * If this workqueue were percpu, it would cause bio reordering
+	 * If this workqueue weren't ordered, it would cause bio reordering
 	 * and reduced performance.
 	 */
-	ic->wait_wq = alloc_workqueue("dm-integrity-wait", WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
+	ic->wait_wq = alloc_ordered_workqueue("dm-integrity-wait", WQ_MEM_RECLAIM);
 	if (!ic->wait_wq) {
 		ti->error = "Cannot allocate workqueue";
 		r = -ENOMEM;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index dfde0088147a..bd3342613e4c 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -207,7 +207,7 @@ static int __init local_init(void)
 	if (r)
 		return r;
 
-	deferred_remove_workqueue = alloc_workqueue("kdmremove", WQ_UNBOUND, 1);
+	deferred_remove_workqueue = alloc_ordered_workqueue("kdmremove", 0);
 	if (!deferred_remove_workqueue) {
 		r = -ENOMEM;
 		goto out_uevent_exit;
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [dm-devel] [PATCH 04/22] dm integrity: Use alloc_ordered_workqueue() to create ordered workqueues
@ 2023-04-21  2:50   ` Tejun Heo
  0 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: kernel-team, Mike Snitzer, linux-kernel, dm-devel, Tejun Heo,
	Alasdair Kergon

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Alasdair Kergon <agk@redhat.com>
Cc: Mike Snitzer <snitzer@kernel.org>
Cc: dm-devel@redhat.com
Cc: linux-kernel@vger.kernel.org
---
 drivers/md/dm-integrity.c | 4 ++--
 drivers/md/dm.c           | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index b0d5057fbdd9..6b36554f9103 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -4268,10 +4268,10 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
 	}
 
 	/*
-	 * If this workqueue were percpu, it would cause bio reordering
+	 * If this workqueue weren't ordered, it would cause bio reordering
 	 * and reduced performance.
 	 */
-	ic->wait_wq = alloc_workqueue("dm-integrity-wait", WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
+	ic->wait_wq = alloc_ordered_workqueue("dm-integrity-wait", WQ_MEM_RECLAIM);
 	if (!ic->wait_wq) {
 		ti->error = "Cannot allocate workqueue";
 		r = -ENOMEM;
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index dfde0088147a..bd3342613e4c 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -207,7 +207,7 @@ static int __init local_init(void)
 	if (r)
 		return r;
 
-	deferred_remove_workqueue = alloc_workqueue("kdmremove", WQ_UNBOUND, 1);
+	deferred_remove_workqueue = alloc_ordered_workqueue("kdmremove", 0);
 	if (!deferred_remove_workqueue) {
 		r = -ENOMEM;
 		goto out_uevent_exit;
-- 
2.40.0

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 05/22] media: amphion: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (3 preceding siblings ...)
  2023-04-21  2:50   ` [dm-devel] " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21  2:50   ` Tejun Heo
                   ` (17 subsequent siblings)
  22 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Ming Qian, Shijie Qin,
	Zhou Peng, Mauro Carvalho Chehab, linux-media

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Ming Qian <ming.qian@nxp.com>
Cc: Shijie Qin <shijie.qin@nxp.com>
Cc: Zhou Peng <eagle.zhou@nxp.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org
---
 drivers/media/platform/amphion/vpu_core.c | 2 +-
 drivers/media/platform/amphion/vpu_v4l2.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/amphion/vpu_core.c b/drivers/media/platform/amphion/vpu_core.c
index f9ec1753f7c8..57d56c680c07 100644
--- a/drivers/media/platform/amphion/vpu_core.c
+++ b/drivers/media/platform/amphion/vpu_core.c
@@ -254,7 +254,7 @@ static int vpu_core_register(struct device *dev, struct vpu_core *core)
 	if (vpu_core_is_exist(vpu, core))
 		return 0;
 
-	core->workqueue = alloc_workqueue("vpu", WQ_UNBOUND | WQ_MEM_RECLAIM, 1);
+	core->workqueue = alloc_ordered_workqueue("vpu", WQ_MEM_RECLAIM);
 	if (!core->workqueue) {
 		dev_err(core->dev, "fail to alloc workqueue\n");
 		return -ENOMEM;
diff --git a/drivers/media/platform/amphion/vpu_v4l2.c b/drivers/media/platform/amphion/vpu_v4l2.c
index 6773b885597c..a48edb445eea 100644
--- a/drivers/media/platform/amphion/vpu_v4l2.c
+++ b/drivers/media/platform/amphion/vpu_v4l2.c
@@ -740,7 +740,7 @@ int vpu_v4l2_open(struct file *file, struct vpu_inst *inst)
 	inst->fh.ctrl_handler = &inst->ctrl_handler;
 	file->private_data = &inst->fh;
 	inst->state = VPU_CODEC_STATE_DEINIT;
-	inst->workqueue = alloc_workqueue("vpu_inst", WQ_UNBOUND | WQ_MEM_RECLAIM, 1);
+	inst->workqueue = alloc_ordered_workqueue("vpu_inst", WQ_MEM_RECLAIM);
 	if (inst->workqueue) {
 		INIT_WORK(&inst->msg_work, vpu_inst_run_work);
 		ret = kfifo_init(&inst->msg_fifo,
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
@ 2023-04-21  2:50   ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 02/22] greybus: " Tejun Heo
                     ` (21 subsequent siblings)
  22 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Sunil Goutham,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-arm-kernel, netdev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Sunil Goutham <sgoutham@marvell.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index 7eb2ddbe9bad..a317feb8decb 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -1126,8 +1126,7 @@ static int bgx_lmac_enable(struct bgx *bgx, u8 lmacid)
 	}
 
 poll:
-	lmac->check_link = alloc_workqueue("check_link", WQ_UNBOUND |
-					   WQ_MEM_RECLAIM, 1);
+	lmac->check_link = alloc_ordered_workqueue("check_link", WQ_MEM_RECLAIM);
 	if (!lmac->check_link)
 		return -ENOMEM;
 	INIT_DELAYED_WORK(&lmac->dwork, bgx_poll_for_link);
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to create ordered workqueues
@ 2023-04-21  2:50   ` Tejun Heo
  0 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Sunil Goutham,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-arm-kernel, netdev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Sunil Goutham <sgoutham@marvell.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index 7eb2ddbe9bad..a317feb8decb 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -1126,8 +1126,7 @@ static int bgx_lmac_enable(struct bgx *bgx, u8 lmacid)
 	}
 
 poll:
-	lmac->check_link = alloc_workqueue("check_link", WQ_UNBOUND |
-					   WQ_MEM_RECLAIM, 1);
+	lmac->check_link = alloc_ordered_workqueue("check_link", WQ_MEM_RECLAIM);
 	if (!lmac->check_link)
 		return -ENOMEM;
 	INIT_DELAYED_WORK(&lmac->dwork, bgx_poll_for_link);
-- 
2.40.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 07/22] net: octeontx2: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (5 preceding siblings ...)
  2023-04-21  2:50   ` Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21  6:16   ` [EXT] " Sunil Kovvuri Goutham
  2023-05-08 23:56   ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 08/22] wifi: ath10/11/12k: " Tejun Heo
                   ` (15 subsequent siblings)
  22 siblings, 2 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Sunil Goutham,
	Ratheesh Kannoth, Srujana Challa, Geetha sowjanya, netdev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Sunil Goutham <sgoutham@marvell.com>
Cc: Ratheesh Kannoth <rkannoth@marvell.com>
Cc: Srujana Challa <schalla@marvell.com>
Cc: Geetha sowjanya <gakula@marvell.com>
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu.c     |  5 ++---
 .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c    | 13 +++++--------
 .../net/ethernet/marvell/octeontx2/nic/otx2_vf.c    |  5 ++---
 3 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index 8683ce57ed3f..207041c81184 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -3013,9 +3013,8 @@ static int rvu_flr_init(struct rvu *rvu)
 			    cfg | BIT_ULL(22));
 	}
 
-	rvu->flr_wq = alloc_workqueue("rvu_afpf_flr",
-				      WQ_UNBOUND | WQ_HIGHPRI | WQ_MEM_RECLAIM,
-				       1);
+	rvu->flr_wq = alloc_ordered_workqueue("rvu_afpf_flr",
+					      WQ_HIGHPRI | WQ_MEM_RECLAIM);
 	if (!rvu->flr_wq)
 		return -ENOMEM;
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index 179433d0a54a..7b3114105757 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -271,8 +271,7 @@ static int otx2_pf_flr_init(struct otx2_nic *pf, int num_vfs)
 {
 	int vf;
 
-	pf->flr_wq = alloc_workqueue("otx2_pf_flr_wq",
-				     WQ_UNBOUND | WQ_HIGHPRI, 1);
+	pf->flr_wq = alloc_ordered_workqueue("otx2_pf_flr_wq", WQ_HIGHPRI);
 	if (!pf->flr_wq)
 		return -ENOMEM;
 
@@ -593,9 +592,8 @@ static int otx2_pfvf_mbox_init(struct otx2_nic *pf, int numvfs)
 	if (!pf->mbox_pfvf)
 		return -ENOMEM;
 
-	pf->mbox_pfvf_wq = alloc_workqueue("otx2_pfvf_mailbox",
-					   WQ_UNBOUND | WQ_HIGHPRI |
-					   WQ_MEM_RECLAIM, 1);
+	pf->mbox_pfvf_wq = alloc_ordered_workqueue("otx2_pfvf_mailbox",
+						   WQ_HIGHPRI | WQ_MEM_RECLAIM);
 	if (!pf->mbox_pfvf_wq)
 		return -ENOMEM;
 
@@ -1063,9 +1061,8 @@ static int otx2_pfaf_mbox_init(struct otx2_nic *pf)
 	int err;
 
 	mbox->pfvf = pf;
-	pf->mbox_wq = alloc_workqueue("otx2_pfaf_mailbox",
-				      WQ_UNBOUND | WQ_HIGHPRI |
-				      WQ_MEM_RECLAIM, 1);
+	pf->mbox_wq = alloc_ordered_workqueue("otx2_pfaf_mailbox",
+					      WQ_HIGHPRI | WQ_MEM_RECLAIM);
 	if (!pf->mbox_wq)
 		return -ENOMEM;
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
index ab126f8706c7..1f16e0dcbb3e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
@@ -297,9 +297,8 @@ static int otx2vf_vfaf_mbox_init(struct otx2_nic *vf)
 	int err;
 
 	mbox->pfvf = vf;
-	vf->mbox_wq = alloc_workqueue("otx2_vfaf_mailbox",
-				      WQ_UNBOUND | WQ_HIGHPRI |
-				      WQ_MEM_RECLAIM, 1);
+	vf->mbox_wq = alloc_ordered_workqueue("otx2_vfaf_mailbox",
+					      WQ_HIGHPRI | WQ_MEM_RECLAIM);
 	if (!vf->mbox_wq)
 		return -ENOMEM;
 
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 08/22] wifi: ath10/11/12k: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (6 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 07/22] net: octeontx2: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 09/22] wifi: iwlwifi: " Tejun Heo
                   ` (14 subsequent siblings)
  22 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Kalle Valo,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-wireless, netdev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 drivers/net/wireless/ath/ath10k/qmi.c | 3 +--
 drivers/net/wireless/ath/ath11k/qmi.c | 3 +--
 drivers/net/wireless/ath/ath12k/qmi.c | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c
index 90f457b8e1fe..ebedef8767cd 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -1082,8 +1082,7 @@ int ath10k_qmi_init(struct ath10k *ar, u32 msa_size)
 	if (ret)
 		goto err;
 
-	qmi->event_wq = alloc_workqueue("ath10k_qmi_driver_event",
-					WQ_UNBOUND, 1);
+	qmi->event_wq = alloc_ordered_workqueue("ath10k_qmi_driver_event", 0);
 	if (!qmi->event_wq) {
 		ath10k_err(ar, "failed to allocate workqueue\n");
 		ret = -EFAULT;
diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index ab923e24b0a9..26b252e62909 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -3256,8 +3256,7 @@ int ath11k_qmi_init_service(struct ath11k_base *ab)
 		return ret;
 	}
 
-	ab->qmi.event_wq = alloc_workqueue("ath11k_qmi_driver_event",
-					   WQ_UNBOUND, 1);
+	ab->qmi.event_wq = alloc_ordered_workqueue("ath11k_qmi_driver_event", 0);
 	if (!ab->qmi.event_wq) {
 		ath11k_err(ab, "failed to allocate workqueue\n");
 		return -EFAULT;
diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index 979a63f2e2ab..471810877eed 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -3054,8 +3054,7 @@ int ath12k_qmi_init_service(struct ath12k_base *ab)
 		return ret;
 	}
 
-	ab->qmi.event_wq = alloc_workqueue("ath12k_qmi_driver_event",
-					   WQ_UNBOUND, 1);
+	ab->qmi.event_wq = alloc_ordered_workqueue("ath12k_qmi_driver_event", 0);
 	if (!ab->qmi.event_wq) {
 		ath12k_err(ab, "failed to allocate workqueue\n");
 		return -EFAULT;
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 09/22] wifi: iwlwifi: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (7 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 08/22] wifi: ath10/11/12k: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-24 17:31   ` Johannes Berg
  2023-05-08 23:59   ` [PATCH 09/22] wifi: iwlwifi: Use alloc_ordered_workqueue() to create ordered workqueues Tejun Heo
  2023-04-21  2:50 ` [PATCH 10/22] wifi: mwifiex: " Tejun Heo
                   ` (13 subsequent siblings)
  22 siblings, 2 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Kalle Valo,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Gregory Greenman, Johannes Berg, Avraham Stern, Kees Cook,
	Mordechay Goodstein, Haim, Dreyfuss, linux-wireless, netdev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Gregory Greenman <gregory.greenman@intel.com>
Cc: Johannes Berg <johannes.berg@intel.com>
Cc: Avraham Stern <avraham.stern@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mordechay Goodstein <mordechay.goodstein@intel.com>
Cc: "Haim, Dreyfuss" <haim.dreyfuss@intel.com>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index 0a9af1ad1f20..cd17b601b172 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -3576,8 +3576,8 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct pci_dev *pdev,
 	init_waitqueue_head(&trans_pcie->fw_reset_waitq);
 	init_waitqueue_head(&trans_pcie->imr_waitq);
 
-	trans_pcie->rba.alloc_wq = alloc_workqueue("rb_allocator",
-						   WQ_HIGHPRI | WQ_UNBOUND, 1);
+	trans_pcie->rba.alloc_wq = alloc_ordered_workqueue("rb_allocator",
+							   WQ_HIGHPRI);
 	if (!trans_pcie->rba.alloc_wq) {
 		ret = -ENOMEM;
 		goto out_free_trans;
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 10/22] wifi: mwifiex: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (8 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 09/22] wifi: iwlwifi: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-25 18:14   ` Brian Norris
  2023-04-21  2:50 ` [PATCH 11/22] net: wwan: t7xx: Use alloc_ordered_workqueue() to create ordered workqueues Tejun Heo
                   ` (12 subsequent siblings)
  22 siblings, 1 reply; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Amitkumar Karwar,
	Ganapathi Bhat, Sharvari Harisangam, Xinming Hu, Kalle Valo,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-wireless, netdev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Amitkumar Karwar <amitkarwar@gmail.com>
Cc: Ganapathi Bhat <ganapathi017@gmail.com>
Cc: Sharvari Harisangam <sharvari.harisangam@nxp.com>
Cc: Xinming Hu <huxinming820@gmail.com>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 .../net/wireless/marvell/mwifiex/cfg80211.c   | 13 +++++------
 drivers/net/wireless/marvell/mwifiex/main.c   | 22 +++++++++----------
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index bcd564dc3554..5a7be57ed78a 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -3124,10 +3124,9 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
 
 	SET_NETDEV_DEV(dev, adapter->dev);
 
-	priv->dfs_cac_workqueue = alloc_workqueue("MWIFIEX_DFS_CAC%s",
-						  WQ_HIGHPRI |
-						  WQ_MEM_RECLAIM |
-						  WQ_UNBOUND, 1, name);
+	priv->dfs_cac_workqueue =
+		alloc_ordered_workqueue("MWIFIEX_DFS_CAC%s",
+					WQ_HIGHPRI | WQ_MEM_RECLAIM, name);
 	if (!priv->dfs_cac_workqueue) {
 		mwifiex_dbg(adapter, ERROR, "cannot alloc DFS CAC queue\n");
 		ret = -ENOMEM;
@@ -3136,9 +3135,9 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct wiphy *wiphy,
 
 	INIT_DELAYED_WORK(&priv->dfs_cac_work, mwifiex_dfs_cac_work_queue);
 
-	priv->dfs_chan_sw_workqueue = alloc_workqueue("MWIFIEX_DFS_CHSW%s",
-						      WQ_HIGHPRI | WQ_UNBOUND |
-						      WQ_MEM_RECLAIM, 1, name);
+	priv->dfs_chan_sw_workqueue =
+		alloc_ordered_workqueue("MWIFIEX_DFS_CHSW%s",
+					WQ_HIGHPRI | WQ_MEM_RECLAIM, name);
 	if (!priv->dfs_chan_sw_workqueue) {
 		mwifiex_dbg(adapter, ERROR, "cannot alloc DFS channel sw queue\n");
 		ret = -ENOMEM;
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index ea22a08e6c08..19a6107d115c 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1546,18 +1546,17 @@ mwifiex_reinit_sw(struct mwifiex_adapter *adapter)
 		adapter->rx_work_enabled = true;
 
 	adapter->workqueue =
-		alloc_workqueue("MWIFIEX_WORK_QUEUE",
-				WQ_HIGHPRI | WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
+		alloc_ordered_workqueue("MWIFIEX_WORK_QUEUE",
+					WQ_HIGHPRI | WQ_MEM_RECLAIM);
 	if (!adapter->workqueue)
 		goto err_kmalloc;
 
 	INIT_WORK(&adapter->main_work, mwifiex_main_work_queue);
 
 	if (adapter->rx_work_enabled) {
-		adapter->rx_workqueue = alloc_workqueue("MWIFIEX_RX_WORK_QUEUE",
-							WQ_HIGHPRI |
-							WQ_MEM_RECLAIM |
-							WQ_UNBOUND, 1);
+		adapter->rx_workqueue =
+			alloc_ordered_workqueue("MWIFIEX_RX_WORK_QUEUE",
+						WQ_HIGHPRI | WQ_MEM_RECLAIM);
 		if (!adapter->rx_workqueue)
 			goto err_kmalloc;
 		INIT_WORK(&adapter->rx_work, mwifiex_rx_work_queue);
@@ -1701,18 +1700,17 @@ mwifiex_add_card(void *card, struct completion *fw_done,
 		adapter->rx_work_enabled = true;
 
 	adapter->workqueue =
-		alloc_workqueue("MWIFIEX_WORK_QUEUE",
-				WQ_HIGHPRI | WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
+		alloc_ordered_workqueue("MWIFIEX_WORK_QUEUE",
+					WQ_HIGHPRI | WQ_MEM_RECLAIM);
 	if (!adapter->workqueue)
 		goto err_kmalloc;
 
 	INIT_WORK(&adapter->main_work, mwifiex_main_work_queue);
 
 	if (adapter->rx_work_enabled) {
-		adapter->rx_workqueue = alloc_workqueue("MWIFIEX_RX_WORK_QUEUE",
-							WQ_HIGHPRI |
-							WQ_MEM_RECLAIM |
-							WQ_UNBOUND, 1);
+		adapter->rx_workqueue =
+			alloc_ordered_workqueue("MWIFIEX_RX_WORK_QUEUE",
+						WQ_HIGHPRI | WQ_MEM_RECLAIM);
 		if (!adapter->rx_workqueue)
 			goto err_kmalloc;
 
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 11/22] net: wwan: t7xx: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (9 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 10/22] wifi: mwifiex: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 12/22] scsi: " Tejun Heo
                   ` (11 subsequent siblings)
  22 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Chandrashekar Devegowda,
	Intel Corporation, Chiranjeevi Rapolu, Liu Haijun,
	M Chetan Kumar, Ricardo Martinez, Loic Poulain, Sergey Ryazanov,
	Johannes Berg, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
Cc: Intel Corporation <linuxwwan@intel.com>
Cc: Chiranjeevi Rapolu <chiranjeevi.rapolu@linux.intel.com>
Cc: Liu Haijun <haijun.liu@mediatek.com>
Cc: M Chetan Kumar <m.chetan.kumar@linux.intel.com>
Cc: Ricardo Martinez <ricardo.martinez@linux.intel.com>
Cc: Loic Poulain <loic.poulain@linaro.org>
Cc: Sergey Ryazanov <ryazanov.s.a@gmail.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org
---
 drivers/net/wwan/t7xx/t7xx_hif_cldma.c     | 13 +++++++------
 drivers/net/wwan/t7xx/t7xx_hif_dpmaif_tx.c |  5 +++--
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wwan/t7xx/t7xx_hif_cldma.c b/drivers/net/wwan/t7xx/t7xx_hif_cldma.c
index aec3a18d44bd..7162bf38a8c9 100644
--- a/drivers/net/wwan/t7xx/t7xx_hif_cldma.c
+++ b/drivers/net/wwan/t7xx/t7xx_hif_cldma.c
@@ -1293,9 +1293,9 @@ int t7xx_cldma_init(struct cldma_ctrl *md_ctrl)
 	for (i = 0; i < CLDMA_TXQ_NUM; i++) {
 		md_cd_queue_struct_init(&md_ctrl->txq[i], md_ctrl, MTK_TX, i);
 		md_ctrl->txq[i].worker =
-			alloc_workqueue("md_hif%d_tx%d_worker",
-					WQ_UNBOUND | WQ_MEM_RECLAIM | (i ? 0 : WQ_HIGHPRI),
-					1, md_ctrl->hif_id, i);
+			alloc_ordered_workqueue("md_hif%d_tx%d_worker",
+					WQ_MEM_RECLAIM | (i ? 0 : WQ_HIGHPRI),
+					md_ctrl->hif_id, i);
 		if (!md_ctrl->txq[i].worker)
 			goto err_workqueue;
 
@@ -1306,9 +1306,10 @@ int t7xx_cldma_init(struct cldma_ctrl *md_ctrl)
 		md_cd_queue_struct_init(&md_ctrl->rxq[i], md_ctrl, MTK_RX, i);
 		INIT_WORK(&md_ctrl->rxq[i].cldma_work, t7xx_cldma_rx_done);
 
-		md_ctrl->rxq[i].worker = alloc_workqueue("md_hif%d_rx%d_worker",
-							 WQ_UNBOUND | WQ_MEM_RECLAIM,
-							 1, md_ctrl->hif_id, i);
+		md_ctrl->rxq[i].worker =
+			alloc_ordered_workqueue("md_hif%d_rx%d_worker",
+						WQ_MEM_RECLAIM,
+						md_ctrl->hif_id, i);
 		if (!md_ctrl->rxq[i].worker)
 			goto err_workqueue;
 	}
diff --git a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_tx.c b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_tx.c
index 46514208d4f9..8dab025a088a 100644
--- a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_tx.c
+++ b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif_tx.c
@@ -618,8 +618,9 @@ int t7xx_dpmaif_txq_init(struct dpmaif_tx_queue *txq)
 		return ret;
 	}
 
-	txq->worker = alloc_workqueue("md_dpmaif_tx%d_worker", WQ_UNBOUND | WQ_MEM_RECLAIM |
-				      (txq->index ? 0 : WQ_HIGHPRI), 1, txq->index);
+	txq->worker = alloc_ordered_workqueue("md_dpmaif_tx%d_worker",
+				WQ_MEM_RECLAIM | (txq->index ? 0 : WQ_HIGHPRI),
+				txq->index);
 	if (!txq->worker)
 		return -ENOMEM;
 
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 12/22] scsi: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (10 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 11/22] net: wwan: t7xx: Use alloc_ordered_workqueue() to create ordered workqueues Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21 14:23   ` Benjamin Block
  2023-05-08 23:57   ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 13/22] virt: acrn: " Tejun Heo
                   ` (10 subsequent siblings)
  22 siblings, 2 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, James E.J. Bottomley,
	Martin K. Petersen, linux-scsi

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/NCR5380.c  |  5 ++---
 drivers/scsi/hosts.c    | 12 ++++++------
 drivers/scsi/libiscsi.c |  5 ++---
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index ca85bddb582b..b18dd4591492 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -415,9 +415,8 @@ static int NCR5380_init(struct Scsi_Host *instance, int flags)
 	hostdata->flags = flags;
 
 	INIT_WORK(&hostdata->main_task, NCR5380_main);
-	hostdata->work_q = alloc_workqueue("ncr5380_%d",
-	                        WQ_UNBOUND | WQ_MEM_RECLAIM,
-	                        1, instance->host_no);
+	hostdata->work_q = alloc_ordered_workqueue("ncr5380_%d",
+	                        WQ_MEM_RECLAIM, instance->host_no);
 	if (!hostdata->work_q)
 		return -ENOMEM;
 
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 9b6fbbe15d92..30bf9f49ca6c 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -294,9 +294,9 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
 	if (shost->transportt->create_work_queue) {
 		snprintf(shost->work_q_name, sizeof(shost->work_q_name),
 			 "scsi_wq_%d", shost->host_no);
-		shost->work_q = alloc_workqueue("%s",
-			WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND,
-			1, shost->work_q_name);
+		shost->work_q = alloc_ordered_workqueue("%s",
+			WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM,
+			shost->work_q_name);
 
 		if (!shost->work_q) {
 			error = -EINVAL;
@@ -510,9 +510,9 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 		goto fail;
 	}
 
-	shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d",
-					WQ_UNBOUND | WQ_MEM_RECLAIM | WQ_SYSFS,
-					   1, shost->host_no);
+	shost->tmf_work_q = alloc_ordered_workqueue("scsi_tmf_%d",
+						    WQ_MEM_RECLAIM | WQ_SYSFS,
+						    shost->host_no);
 	if (!shost->tmf_work_q) {
 		shost_printk(KERN_WARNING, shost,
 			     "failed to create tmf workq\n");
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 127f3d7f19dc..d0eba590dc69 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2907,9 +2907,8 @@ struct Scsi_Host *iscsi_host_alloc(struct scsi_host_template *sht,
 	ihost = shost_priv(shost);
 
 	if (xmit_can_sleep) {
-		ihost->workq = alloc_workqueue("iscsi_q_%d",
-			WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND,
-			1, shost->host_no);
+		ihost->workq = alloc_ordered_workqueue("iscsi_q_%d",
+			WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM, shost->host_no);
 		if (!ihost->workq)
 			goto free_host;
 	}
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 13/22] virt: acrn: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (11 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 12/22] scsi: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21  5:03   ` Greg Kroah-Hartman
  2023-05-08 23:58   ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 14/22] soc: qcom: qmi: " Tejun Heo
                   ` (9 subsequent siblings)
  22 siblings, 2 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Fei Li, Greg Kroah-Hartman

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Fei Li <fei1.li@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/virt/acrn/ioreq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/virt/acrn/ioreq.c b/drivers/virt/acrn/ioreq.c
index d75ab3f66da4..cecdc1c13af7 100644
--- a/drivers/virt/acrn/ioreq.c
+++ b/drivers/virt/acrn/ioreq.c
@@ -576,8 +576,8 @@ static void ioreq_resume(void)
 int acrn_ioreq_intr_setup(void)
 {
 	acrn_setup_intr_handler(ioreq_intr_handler);
-	ioreq_wq = alloc_workqueue("ioreq_wq",
-				   WQ_HIGHPRI | WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
+	ioreq_wq = alloc_ordered_workqueue("ioreq_wq",
+					   WQ_HIGHPRI | WQ_MEM_RECLAIM);
 	if (!ioreq_wq) {
 		dev_err(acrn_dev.this_device, "Failed to alloc workqueue!\n");
 		acrn_remove_intr_handler();
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 14/22] soc: qcom: qmi: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (12 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 13/22] virt: acrn: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 15/22] xen/pvcalls: " Tejun Heo
                   ` (8 subsequent siblings)
  22 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, linux-arm-msm

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: linux-arm-msm@vger.kernel.org
---
 drivers/soc/qcom/qmi_interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
index 820bdd9f8e46..78d7361fdcf2 100644
--- a/drivers/soc/qcom/qmi_interface.c
+++ b/drivers/soc/qcom/qmi_interface.c
@@ -650,7 +650,7 @@ int qmi_handle_init(struct qmi_handle *qmi, size_t recv_buf_size,
 	if (!qmi->recv_buf)
 		return -ENOMEM;
 
-	qmi->wq = alloc_workqueue("qmi_msg_handler", WQ_UNBOUND, 1);
+	qmi->wq = alloc_ordered_workqueue("qmi_msg_handler", 0);
 	if (!qmi->wq) {
 		ret = -ENOMEM;
 		goto err_free_recv_buf;
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 15/22] xen/pvcalls: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (13 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 14/22] soc: qcom: qmi: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-05-08 11:58   ` Juergen Gross
  2023-05-08 23:59   ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 16/22] btrfs: " Tejun Heo
                   ` (7 subsequent siblings)
  22 siblings, 2 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Juergen Gross,
	Stefano Stabellini, Oleksandr Tyshchenko, xen-devel

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Cc: xen-devel@lists.xenproject.org
---
 drivers/xen/pvcalls-back.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
index 1f5219e12cc3..b41516f3f84a 100644
--- a/drivers/xen/pvcalls-back.c
+++ b/drivers/xen/pvcalls-back.c
@@ -361,7 +361,7 @@ static struct sock_mapping *pvcalls_new_active_socket(
 	map->data.in = map->bytes;
 	map->data.out = map->bytes + XEN_FLEX_RING_SIZE(map->ring_order);
 
-	map->ioworker.wq = alloc_workqueue("pvcalls_io", WQ_UNBOUND, 1);
+	map->ioworker.wq = alloc_ordered_workqueue("pvcalls_io", 0);
 	if (!map->ioworker.wq)
 		goto out;
 	atomic_set(&map->io, 1);
@@ -637,7 +637,7 @@ static int pvcalls_back_bind(struct xenbus_device *dev,
 
 	INIT_WORK(&map->register_work, __pvcalls_back_accept);
 	spin_lock_init(&map->copy_lock);
-	map->wq = alloc_workqueue("pvcalls_wq", WQ_UNBOUND, 1);
+	map->wq = alloc_ordered_workqueue("pvcalls_wq", 0);
 	if (!map->wq) {
 		ret = -ENOMEM;
 		goto out;
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 16/22] btrfs: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (14 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 15/22] xen/pvcalls: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-30  4:40   ` Wang Yugui
  2023-04-21  2:50 ` [PATCH 17/22] cifs: " Tejun Heo
                   ` (6 subsequent siblings)
  22 siblings, 1 reply; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Chris Mason, Josef Bacik,
	David Sterba, linux-btrfs

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Chris Mason <clm@fb.com>
Cc: Josef Bacik <josef@toxicpanda.com>
Cc: David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org
---
 fs/btrfs/disk-io.c | 2 +-
 fs/btrfs/scrub.c   | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 9e1596bb208d..b1f6ff69dbe1 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2218,7 +2218,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
 	fs_info->qgroup_rescan_workers =
 		btrfs_alloc_workqueue(fs_info, "qgroup-rescan", flags, 1, 0);
 	fs_info->discard_ctl.discard_workers =
-		alloc_workqueue("btrfs_discard", WQ_UNBOUND | WQ_FREEZABLE, 1);
+		alloc_ordered_workqueue("btrfs_discard", WQ_FREEZABLE);
 
 	if (!(fs_info->workers && fs_info->hipri_workers &&
 	      fs_info->delalloc_workers && fs_info->flush_workers &&
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 69c93ae333f6..70882358bdb0 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -4245,8 +4245,10 @@ static noinline_for_stack int scrub_workers_get(struct btrfs_fs_info *fs_info,
 	if (refcount_inc_not_zero(&fs_info->scrub_workers_refcnt))
 		return 0;
 
-	scrub_workers = alloc_workqueue("btrfs-scrub", flags,
-					is_dev_replace ? 1 : max_active);
+	if (is_dev_replace)
+		scrub_workers = alloc_ordered_workqueue("btrfs-scrub", flags);
+	else
+		scrub_workers = alloc_workqueue("btrfs-scrub", flags, max_active);
 	if (!scrub_workers)
 		goto fail_scrub_workers;
 
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 17/22] cifs: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (15 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 16/22] btrfs: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21 18:38   ` Paulo Alcantara
  2023-04-21  2:50 ` [PATCH 18/22] net: qrtr: " Tejun Heo
                   ` (5 subsequent siblings)
  22 siblings, 1 reply; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Steve French,
	Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	linux-cifs, samba-technical

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Steve French <sfrench@samba.org>
Cc: Paulo Alcantara <pc@cjr.nz>
Cc: Ronnie Sahlberg <lsahlber@redhat.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
---
 fs/cifs/dfs_cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c
index 30cbdf8514a5..aece6d51bce7 100644
--- a/fs/cifs/dfs_cache.c
+++ b/fs/cifs/dfs_cache.c
@@ -290,7 +290,7 @@ int dfs_cache_init(void)
 	int rc;
 	int i;
 
-	dfscache_wq = alloc_workqueue("cifs-dfscache", WQ_FREEZABLE | WQ_UNBOUND, 1);
+	dfscache_wq = alloc_ordered_workqueue("cifs-dfscache", WQ_FREEZABLE);
 	if (!dfscache_wq)
 		return -ENOMEM;
 
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 18/22] net: qrtr: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (16 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 17/22] cifs: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 19/22] rxrpc: " Tejun Heo
                   ` (4 subsequent siblings)
  22 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Manivannan Sadhasivam,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-arm-msm, netdev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 net/qrtr/ns.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
index 0f25a386138c..0f7a729f1a1f 100644
--- a/net/qrtr/ns.c
+++ b/net/qrtr/ns.c
@@ -783,7 +783,7 @@ int qrtr_ns_init(void)
 		goto err_sock;
 	}
 
-	qrtr_ns.workqueue = alloc_workqueue("qrtr_ns_handler", WQ_UNBOUND, 1);
+	qrtr_ns.workqueue = alloc_ordered_workqueue("qrtr_ns_handler", 0);
 	if (!qrtr_ns.workqueue) {
 		ret = -ENOMEM;
 		goto err_sock;
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 19/22] rxrpc: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (17 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 18/22] net: qrtr: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 20/22] crypto: octeontx2: " Tejun Heo
                   ` (3 subsequent siblings)
  22 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, David Howells, Marc Dionne,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-afs, netdev

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Marc Dionne <marc.dionne@auristor.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-afs@lists.infradead.org
Cc: netdev@vger.kernel.org
---
 net/rxrpc/af_rxrpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 102f5cbff91a..e1822c12990d 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -972,7 +972,7 @@ static int __init af_rxrpc_init(void)
 		goto error_call_jar;
 	}
 
-	rxrpc_workqueue = alloc_workqueue("krxrpcd", WQ_HIGHPRI | WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
+	rxrpc_workqueue = alloc_ordered_workqueue("krxrpcd", WQ_HIGHPRI | WQ_MEM_RECLAIM);
 	if (!rxrpc_workqueue) {
 		pr_notice("Failed to allocate work queue\n");
 		goto error_work_queue;
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 20/22] crypto: octeontx2: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (18 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 19/22] rxrpc: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 21/22] media: coda: " Tejun Heo
                   ` (2 subsequent siblings)
  22 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Boris Brezillon,
	Arnaud Ebalard, Srujana Challa, Herbert Xu, David S. Miller,
	Shijith Thotton, Vladis Dronov, Vincent Mailhol, Wolfram Sang,
	Alexander Lobakin, Minghao Chi, ye xingchen, linux-crypto

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Boris Brezillon <bbrezillon@kernel.org>
Cc: Arnaud Ebalard <arno@natisbad.org>
Cc: Srujana Challa <schalla@marvell.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Shijith Thotton <sthotton@marvell.com>
Cc: Vladis Dronov <vdronov@redhat.com>
Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Alexander Lobakin <alobakin@pm.me>
Cc: Minghao Chi <chi.minghao@zte.com.cn>
Cc: ye xingchen <ye.xingchen@zte.com.cn>
Cc: linux-crypto@vger.kernel.org
---
 drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c | 12 ++++++------
 drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c |  6 +++---
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c b/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
index ddf6e913c1c4..30e6acfc93d9 100644
--- a/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
+++ b/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
@@ -357,9 +357,9 @@ static int cptpf_vfpf_mbox_init(struct otx2_cptpf_dev *cptpf, int num_vfs)
 	u64 vfpf_mbox_base;
 	int err, i;
 
-	cptpf->vfpf_mbox_wq = alloc_workqueue("cpt_vfpf_mailbox",
-					      WQ_UNBOUND | WQ_HIGHPRI |
-					      WQ_MEM_RECLAIM, 1);
+	cptpf->vfpf_mbox_wq =
+		alloc_ordered_workqueue("cpt_vfpf_mailbox",
+					WQ_HIGHPRI | WQ_MEM_RECLAIM);
 	if (!cptpf->vfpf_mbox_wq)
 		return -ENOMEM;
 
@@ -453,9 +453,9 @@ static int cptpf_afpf_mbox_init(struct otx2_cptpf_dev *cptpf)
 	resource_size_t offset;
 	int err;
 
-	cptpf->afpf_mbox_wq = alloc_workqueue("cpt_afpf_mailbox",
-					      WQ_UNBOUND | WQ_HIGHPRI |
-					      WQ_MEM_RECLAIM, 1);
+	cptpf->afpf_mbox_wq =
+		alloc_ordered_workqueue("cpt_afpf_mailbox",
+					WQ_HIGHPRI | WQ_MEM_RECLAIM);
 	if (!cptpf->afpf_mbox_wq)
 		return -ENOMEM;
 
diff --git a/drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c b/drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c
index 392e9fee05e8..6023a7adb70c 100644
--- a/drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c
+++ b/drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c
@@ -75,9 +75,9 @@ static int cptvf_pfvf_mbox_init(struct otx2_cptvf_dev *cptvf)
 	resource_size_t offset, size;
 	int ret;
 
-	cptvf->pfvf_mbox_wq = alloc_workqueue("cpt_pfvf_mailbox",
-					      WQ_UNBOUND | WQ_HIGHPRI |
-					      WQ_MEM_RECLAIM, 1);
+	cptvf->pfvf_mbox_wq =
+		alloc_ordered_workqueue("cpt_pfvf_mailbox",
+					WQ_HIGHPRI | WQ_MEM_RECLAIM);
 	if (!cptvf->pfvf_mbox_wq)
 		return -ENOMEM;
 
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 21/22] media: coda: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (19 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 20/22] crypto: octeontx2: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-21  2:50 ` [PATCH 22/22] workqueue: Don't implicitly make UNBOUND workqueues w/ @max_active==1 ordered Tejun Heo
  2023-05-25  4:54 ` (subset) [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Bjorn Andersson
  22 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Philipp Zabel,
	Mauro Carvalho Chehab, linux-media

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org
---
 drivers/media/platform/chips-media/coda-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/chips-media/coda-common.c b/drivers/media/platform/chips-media/coda-common.c
index af71eea04dbd..c8ecfe760028 100644
--- a/drivers/media/platform/chips-media/coda-common.c
+++ b/drivers/media/platform/chips-media/coda-common.c
@@ -3268,7 +3268,7 @@ static int coda_probe(struct platform_device *pdev)
 						       &dev->iram.blob);
 	}
 
-	dev->workqueue = alloc_workqueue("coda", WQ_UNBOUND | WQ_MEM_RECLAIM, 1);
+	dev->workqueue = alloc_ordered_workqueue("coda", WQ_MEM_RECLAIM);
 	if (!dev->workqueue) {
 		dev_err(&pdev->dev, "unable to alloc workqueue\n");
 		ret = -ENOMEM;
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* [PATCH 22/22] workqueue: Don't implicitly make UNBOUND workqueues w/ @max_active==1 ordered
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (20 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 21/22] media: coda: " Tejun Heo
@ 2023-04-21  2:50 ` Tejun Heo
  2023-04-24  5:03   ` Christoph Hellwig
  2023-05-25  4:54 ` (subset) [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Bjorn Andersson
  22 siblings, 1 reply; 70+ messages in thread
From: Tejun Heo @ 2023-04-21  2:50 UTC (permalink / raw)
  To: jiangshanlai; +Cc: linux-kernel, kernel-team, Tejun Heo, Christoph Hellwig

5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
automoatically promoted UNBOUND workqueues w/ @max_active==1 to ordered
workqueues because UNBOUND workqueues w/ @max_active==1 used to be the way
to create ordered workqueues and the new NUMA support broke it. These
problems can be subtle and the fact that they can only trigger on NUMA
machines made them even more difficult to debug.

However, overloading the UNBOUND allocation interface this way creates other
issues. It's difficult to tell whether a given workqueue actually needs to
be ordered and users that legitimately want a min concurrency level wq
unexpectedly gets an ordered one instead. With planned UNBOUND workqueue
udpates to improve execution locality and more prevalence of chiplet designs
which can benefit from such improvements, this isn't a state we wanna be in
forever.

There aren't that many UNBOUND w/ @max_active==1 users in the tree and the
preceding patches audited all and converted them to
alloc_ordered_workqueue() as appropriate. This patch removes the implicit
promotion of UNBOUND w/ @max_active==1 workqueues to ordered ones.

Workqueue will also add a debug option to make all unordered UNBOUND
workqueues to use per-cpu pool_workqueues so that these problems can be
surfaced easier on most machines.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
---
 include/linux/workqueue.h |  4 +---
 kernel/workqueue.c        | 23 ++++-------------------
 2 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index ac551b8ee7d9..e547a90f0328 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -339,7 +339,6 @@ enum {
 	__WQ_DRAINING		= 1 << 16, /* internal: workqueue is draining */
 	__WQ_ORDERED		= 1 << 17, /* internal: workqueue is ordered */
 	__WQ_LEGACY		= 1 << 18, /* internal: create*_workqueue() */
-	__WQ_ORDERED_EXPLICIT	= 1 << 19, /* internal: alloc_ordered_workqueue() */
 
 	WQ_MAX_ACTIVE		= 512,	  /* I like 512, better ideas? */
 	WQ_MAX_UNBOUND_PER_CPU	= 4,	  /* 4 * #cpus for unbound wq */
@@ -417,8 +416,7 @@ alloc_workqueue(const char *fmt, unsigned int flags, int max_active, ...);
  * Pointer to the allocated workqueue on success, %NULL on failure.
  */
 #define alloc_ordered_workqueue(fmt, flags, args...)			\
-	alloc_workqueue(fmt, WQ_UNBOUND | __WQ_ORDERED |		\
-			__WQ_ORDERED_EXPLICIT | (flags), 1, ##args)
+	alloc_workqueue(fmt, WQ_UNBOUND | __WQ_ORDERED | (flags), 1, ##args)
 
 #define create_workqueue(name)						\
 	alloc_workqueue("%s", __WQ_LEGACY | WQ_MEM_RECLAIM, 1, (name))
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index b8b541caed48..00bdcc3c5b36 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -4180,12 +4180,8 @@ static int apply_workqueue_attrs_locked(struct workqueue_struct *wq,
 		return -EINVAL;
 
 	/* creating multiple pwqs breaks ordering guarantee */
-	if (!list_empty(&wq->pwqs)) {
-		if (WARN_ON(wq->flags & __WQ_ORDERED_EXPLICIT))
-			return -EINVAL;
-
-		wq->flags &= ~__WQ_ORDERED;
-	}
+	if (WARN_ON(wq->flags & __WQ_ORDERED))
+		return -EINVAL;
 
 	ctx = apply_wqattrs_prepare(wq, attrs, wq_unbound_cpumask);
 	if (!ctx)
@@ -4408,16 +4404,6 @@ struct workqueue_struct *alloc_workqueue(const char *fmt,
 	struct workqueue_struct *wq;
 	struct pool_workqueue *pwq;
 
-	/*
-	 * Unbound && max_active == 1 used to imply ordered, which is no
-	 * longer the case on NUMA machines due to per-node pools.  While
-	 * alloc_ordered_workqueue() is the right way to create an ordered
-	 * workqueue, keep the previous behavior to avoid subtle breakages
-	 * on NUMA.
-	 */
-	if ((flags & WQ_UNBOUND) && max_active == 1)
-		flags |= __WQ_ORDERED;
-
 	/* see the comment above the definition of WQ_POWER_EFFICIENT */
 	if ((flags & WQ_POWER_EFFICIENT) && wq_power_efficient)
 		flags |= WQ_UNBOUND;
@@ -4625,14 +4611,13 @@ void workqueue_set_max_active(struct workqueue_struct *wq, int max_active)
 	struct pool_workqueue *pwq;
 
 	/* disallow meddling with max_active for ordered workqueues */
-	if (WARN_ON(wq->flags & __WQ_ORDERED_EXPLICIT))
+	if (WARN_ON(wq->flags & __WQ_ORDERED))
 		return;
 
 	max_active = wq_clamp_max_active(max_active, wq->flags, wq->name);
 
 	mutex_lock(&wq->mutex);
 
-	wq->flags &= ~__WQ_ORDERED;
 	wq->saved_max_active = max_active;
 
 	for_each_pwq(pwq, wq)
@@ -5868,7 +5853,7 @@ int workqueue_sysfs_register(struct workqueue_struct *wq)
 	 * attributes breaks ordering guarantee.  Disallow exposing ordered
 	 * workqueues.
 	 */
-	if (WARN_ON(wq->flags & __WQ_ORDERED_EXPLICIT))
+	if (WARN_ON(wq->flags & __WQ_ORDERED))
 		return -EINVAL;
 
 	wq->wq_dev = wq_dev = kzalloc(sizeof(*wq_dev), GFP_KERNEL);
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 70+ messages in thread

* Re: [PATCH 02/22] greybus: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 02/22] greybus: " Tejun Heo
@ 2023-04-21  5:03   ` Greg Kroah-Hartman
  2023-04-21  6:29   ` Johan Hovold
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 70+ messages in thread
From: Greg Kroah-Hartman @ 2023-04-21  5:03 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jiangshanlai, linux-kernel, kernel-team, Johan Hovold,
	Alex Elder, greybus-dev

On Thu, Apr 20, 2023 at 04:50:26PM -1000, Tejun Heo wrote:
> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
> 
> While this has worked okay, overloading the UNBOUND allocation interface
> this way creates other issues. It's difficult to tell whether a given
> workqueue actually needs to be ordered and users that legitimately want a
> min concurrency level wq unexpectedly gets an ordered one instead. With
> planned UNBOUND workqueue updates to improve execution locality and more
> prevalence of chiplet designs which can benefit from such improvements, this
> isn't a state we wanna be in forever.
> 
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Alex Elder <elder@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: greybus-dev@lists.linaro.org

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 13/22] virt: acrn: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 13/22] virt: acrn: " Tejun Heo
@ 2023-04-21  5:03   ` Greg Kroah-Hartman
  2023-05-08 23:58   ` Tejun Heo
  1 sibling, 0 replies; 70+ messages in thread
From: Greg Kroah-Hartman @ 2023-04-21  5:03 UTC (permalink / raw)
  To: Tejun Heo; +Cc: jiangshanlai, linux-kernel, kernel-team, Fei Li

On Thu, Apr 20, 2023 at 04:50:37PM -1000, Tejun Heo wrote:
> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
> 
> While this has worked okay, overloading the UNBOUND allocation interface
> this way creates other issues. It's difficult to tell whether a given
> workqueue actually needs to be ordered and users that legitimately want a
> min concurrency level wq unexpectedly gets an ordered one instead. With
> planned UNBOUND workqueue updates to improve execution locality and more
> prevalence of chiplet designs which can benefit from such improvements, this
> isn't a state we wanna be in forever.
> 
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Fei Li <fei1.li@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/virt/acrn/ioreq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 01/22] powerpc, workqueue: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50   ` Tejun Heo
@ 2023-04-21  5:20     ` Michael Ellerman
  -1 siblings, 0 replies; 70+ messages in thread
From: Michael Ellerman @ 2023-04-21  5:20 UTC (permalink / raw)
  To: Tejun Heo, jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Nicholas Piggin,
	Christophe Leroy, Nathan Lynch, linuxppc-dev

Tejun Heo <tj@kernel.org> writes:
> BACKGROUND
> ==========
>
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
>
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
>
> While this has worked okay, overloading the UNBOUND allocation interface
> this way creates other issues. It's difficult to tell whether a given
> workqueue actually needs to be ordered and users that legitimately want a
> min concurrency level wq unexpectedly gets an ordered one instead. With
> planned UNBOUND workqueue updates to improve execution locality and more
> prevalence of chiplet designs which can benefit from such improvements, this
> isn't a state we wanna be in forever.
>
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
>
> WHAT TO LOOK FOR
> ================
>
> The conversions are from
>
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
>
> to
>
>   alloc_ordered_workqueue(flags, args...)
>
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
>
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
>
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
>  arch/powerpc/kernel/tau_6xx.c          | 2 +-
>  arch/powerpc/platforms/pseries/dlpar.c | 3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)


> diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
> index 828d0f4106d2..cba6dd15de3b 100644
> --- a/arch/powerpc/kernel/tau_6xx.c
> +++ b/arch/powerpc/kernel/tau_6xx.c
> @@ -200,7 +200,7 @@ static int __init TAU_init(void)
>  	tau_int_enable = IS_ENABLED(CONFIG_TAU_INT) &&
>  			 !strcmp(cur_cpu_spec->platform, "ppc750");
>  
> -	tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1);
> +	tau_workq = alloc_ordered_workqueue("tau", 0);
>  	if (!tau_workq)
>  		return -ENOMEM;
>  
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index 75ffdbcd2865..e9117b03807e 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -564,8 +564,7 @@ int __init dlpar_workqueue_init(void)
>  	if (pseries_hp_wq)
>  		return 0;
>  
> -	pseries_hp_wq = alloc_workqueue("pseries hotplug workqueue",
> -			WQ_UNBOUND, 1);
> +	pseries_hp_wq = alloc_ordered_workqueue("pseries hotplug workqueue", 0);
>  
>  	return pseries_hp_wq ? 0 : -ENOMEM;
>  }

The change log of commit 9054619ef54a ("powerpc/pseries: Add pseries
hotplug workqueue") makes it fairly clear that this code does explicitly
want an ordered queue.

cheers

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 01/22] powerpc, workqueue: Use alloc_ordered_workqueue() to create ordered workqueues
@ 2023-04-21  5:20     ` Michael Ellerman
  0 siblings, 0 replies; 70+ messages in thread
From: Michael Ellerman @ 2023-04-21  5:20 UTC (permalink / raw)
  To: Tejun Heo, jiangshanlai
  Cc: Nathan Lynch, kernel-team, linux-kernel, Nicholas Piggin,
	Tejun Heo, linuxppc-dev

Tejun Heo <tj@kernel.org> writes:
> BACKGROUND
> ==========
>
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
>
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
>
> While this has worked okay, overloading the UNBOUND allocation interface
> this way creates other issues. It's difficult to tell whether a given
> workqueue actually needs to be ordered and users that legitimately want a
> min concurrency level wq unexpectedly gets an ordered one instead. With
> planned UNBOUND workqueue updates to improve execution locality and more
> prevalence of chiplet designs which can benefit from such improvements, this
> isn't a state we wanna be in forever.
>
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
>
> WHAT TO LOOK FOR
> ================
>
> The conversions are from
>
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
>
> to
>
>   alloc_ordered_workqueue(flags, args...)
>
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
>
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
>
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
>  arch/powerpc/kernel/tau_6xx.c          | 2 +-
>  arch/powerpc/platforms/pseries/dlpar.c | 3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)


> diff --git a/arch/powerpc/kernel/tau_6xx.c b/arch/powerpc/kernel/tau_6xx.c
> index 828d0f4106d2..cba6dd15de3b 100644
> --- a/arch/powerpc/kernel/tau_6xx.c
> +++ b/arch/powerpc/kernel/tau_6xx.c
> @@ -200,7 +200,7 @@ static int __init TAU_init(void)
>  	tau_int_enable = IS_ENABLED(CONFIG_TAU_INT) &&
>  			 !strcmp(cur_cpu_spec->platform, "ppc750");
>  
> -	tau_workq = alloc_workqueue("tau", WQ_UNBOUND, 1);
> +	tau_workq = alloc_ordered_workqueue("tau", 0);
>  	if (!tau_workq)
>  		return -ENOMEM;
>  
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index 75ffdbcd2865..e9117b03807e 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -564,8 +564,7 @@ int __init dlpar_workqueue_init(void)
>  	if (pseries_hp_wq)
>  		return 0;
>  
> -	pseries_hp_wq = alloc_workqueue("pseries hotplug workqueue",
> -			WQ_UNBOUND, 1);
> +	pseries_hp_wq = alloc_ordered_workqueue("pseries hotplug workqueue", 0);
>  
>  	return pseries_hp_wq ? 0 : -ENOMEM;
>  }

The change log of commit 9054619ef54a ("powerpc/pseries: Add pseries
hotplug workqueue") makes it fairly clear that this code does explicitly
want an ordered queue.

cheers

^ permalink raw reply	[flat|nested] 70+ messages in thread

* RE: [EXT] [PATCH 07/22] net: octeontx2: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 07/22] net: octeontx2: " Tejun Heo
@ 2023-04-21  6:16   ` Sunil Kovvuri Goutham
  2023-05-08 23:56   ` Tejun Heo
  1 sibling, 0 replies; 70+ messages in thread
From: Sunil Kovvuri Goutham @ 2023-04-21  6:16 UTC (permalink / raw)
  To: Tejun Heo, jiangshanlai
  Cc: linux-kernel, kernel-team, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Ratheesh Kannoth, Srujana Challa,
	Geethasowjanya Akula, netdev



> -----Original Message-----
> From: Tejun Heo <htejun@gmail.com> On Behalf Of Tejun Heo
> Sent: Friday, April 21, 2023 8:21 AM
> To: jiangshanlai@gmail.com
> Cc: linux-kernel@vger.kernel.org; kernel-team@meta.com; Tejun Heo
> <tj@kernel.org>; David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Sunil Kovvuri Goutham <sgoutham@marvell.com>;
> Ratheesh Kannoth <rkannoth@marvell.com>; Srujana Challa
> <schalla@marvell.com>; Geethasowjanya Akula <gakula@marvell.com>;
> netdev@vger.kernel.org
> Subject: [EXT] [PATCH 07/22] net: octeontx2: Use alloc_ordered_workqueue()
> to create ordered workqueues
> 
> External Email
> 
> ----------------------------------------------------------------------
> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an ordered
> workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1
> to be ordered"). Because there were users that depended on the ordered
> execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered") made workqueue allocation path to implicitly promote UNBOUND
> workqueues w/
> @max_active==1 to ordered workqueues.
> 
> While this has worked okay, overloading the UNBOUND allocation interface this
> way creates other issues. It's difficult to tell whether a given workqueue actually
> needs to be ordered and users that legitimately want a min concurrency level wq
> unexpectedly gets an ordered one instead. With planned UNBOUND workqueue
> updates to improve execution locality and more prevalence of chiplet designs
> which can benefit from such improvements, this isn't a state we wanna be in
> forever.
> 
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as
> necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion is
> in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion through. The
> behavior will stay exactly the same and we can always reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Sunil Goutham <sgoutham@marvell.com>
> Cc: Ratheesh Kannoth <rkannoth@marvell.com>
> Cc: Srujana Challa <schalla@marvell.com>
> Cc: Geetha sowjanya <gakula@marvell.com>
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/marvell/octeontx2/af/rvu.c     |  5 ++---
>  .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c    | 13 +++++--------
>  .../net/ethernet/marvell/octeontx2/nic/otx2_vf.c    |  5 ++---
>  3 files changed, 9 insertions(+), 14 deletions(-)
> 
Reviewed-by: Sunil Goutham <sgoutham@marvell.com> 

^ permalink raw reply	[flat|nested] 70+ messages in thread

* RE: [EXT] [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50   ` Tejun Heo
@ 2023-04-21  6:19     ` Sunil Kovvuri Goutham
  -1 siblings, 0 replies; 70+ messages in thread
From: Sunil Kovvuri Goutham @ 2023-04-21  6:19 UTC (permalink / raw)
  To: Tejun Heo, jiangshanlai
  Cc: linux-kernel, kernel-team, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-arm-kernel, netdev



> -----Original Message-----
> From: Tejun Heo <htejun@gmail.com> On Behalf Of Tejun Heo
> Sent: Friday, April 21, 2023 8:21 AM
> To: jiangshanlai@gmail.com
> Cc: linux-kernel@vger.kernel.org; kernel-team@meta.com; Tejun Heo
> <tj@kernel.org>; Sunil Kovvuri Goutham <sgoutham@marvell.com>; David S.
> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; linux-arm-
> kernel@lists.infradead.org; netdev@vger.kernel.org
> Subject: [EXT] [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to
> create ordered workqueues
> 
> External Email
> 
> ----------------------------------------------------------------------
> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an ordered
> workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1
> to be ordered"). Because there were users that depended on the ordered
> execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered") made workqueue allocation path to implicitly promote UNBOUND
> workqueues w/
> @max_active==1 to ordered workqueues.
> 
> While this has worked okay, overloading the UNBOUND allocation interface this
> way creates other issues. It's difficult to tell whether a given workqueue actually
> needs to be ordered and users that legitimately want a min concurrency level wq
> unexpectedly gets an ordered one instead. With planned UNBOUND workqueue
> updates to improve execution locality and more prevalence of chiplet designs
> which can benefit from such improvements, this isn't a state we wanna be in
> forever.
> 
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as
> necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion is
> in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion through. The
> behavior will stay exactly the same and we can always reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Sunil Goutham <sgoutham@marvell.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> index 7eb2ddbe9bad..a317feb8decb 100644
> --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> @@ -1126,8 +1126,7 @@ static int bgx_lmac_enable(struct bgx *bgx, u8
> lmacid)
>  	}
> 
>  poll:
> -	lmac->check_link = alloc_workqueue("check_link", WQ_UNBOUND |
> -					   WQ_MEM_RECLAIM, 1);
> +	lmac->check_link = alloc_ordered_workqueue("check_link",
> +WQ_MEM_RECLAIM);
>  	if (!lmac->check_link)
>  		return -ENOMEM;
>  	INIT_DELAYED_WORK(&lmac->dwork, bgx_poll_for_link);
> --
> 2.40.0

Reviewed-by: Sunil Goutham <sgoutham@marvell.com>


^ permalink raw reply	[flat|nested] 70+ messages in thread

* RE: [EXT] [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to create ordered workqueues
@ 2023-04-21  6:19     ` Sunil Kovvuri Goutham
  0 siblings, 0 replies; 70+ messages in thread
From: Sunil Kovvuri Goutham @ 2023-04-21  6:19 UTC (permalink / raw)
  To: Tejun Heo, jiangshanlai
  Cc: linux-kernel, kernel-team, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-arm-kernel, netdev



> -----Original Message-----
> From: Tejun Heo <htejun@gmail.com> On Behalf Of Tejun Heo
> Sent: Friday, April 21, 2023 8:21 AM
> To: jiangshanlai@gmail.com
> Cc: linux-kernel@vger.kernel.org; kernel-team@meta.com; Tejun Heo
> <tj@kernel.org>; Sunil Kovvuri Goutham <sgoutham@marvell.com>; David S.
> Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; linux-arm-
> kernel@lists.infradead.org; netdev@vger.kernel.org
> Subject: [EXT] [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to
> create ordered workqueues
> 
> External Email
> 
> ----------------------------------------------------------------------
> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an ordered
> workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1
> to be ordered"). Because there were users that depended on the ordered
> execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered") made workqueue allocation path to implicitly promote UNBOUND
> workqueues w/
> @max_active==1 to ordered workqueues.
> 
> While this has worked okay, overloading the UNBOUND allocation interface this
> way creates other issues. It's difficult to tell whether a given workqueue actually
> needs to be ordered and users that legitimately want a min concurrency level wq
> unexpectedly gets an ordered one instead. With planned UNBOUND workqueue
> updates to improve execution locality and more prevalence of chiplet designs
> which can benefit from such improvements, this isn't a state we wanna be in
> forever.
> 
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as
> necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion is
> in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion through. The
> behavior will stay exactly the same and we can always reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Sunil Goutham <sgoutham@marvell.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> index 7eb2ddbe9bad..a317feb8decb 100644
> --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> @@ -1126,8 +1126,7 @@ static int bgx_lmac_enable(struct bgx *bgx, u8
> lmacid)
>  	}
> 
>  poll:
> -	lmac->check_link = alloc_workqueue("check_link", WQ_UNBOUND |
> -					   WQ_MEM_RECLAIM, 1);
> +	lmac->check_link = alloc_ordered_workqueue("check_link",
> +WQ_MEM_RECLAIM);
>  	if (!lmac->check_link)
>  		return -ENOMEM;
>  	INIT_DELAYED_WORK(&lmac->dwork, bgx_poll_for_link);
> --
> 2.40.0

Reviewed-by: Sunil Goutham <sgoutham@marvell.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 02/22] greybus: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 02/22] greybus: " Tejun Heo
  2023-04-21  5:03   ` Greg Kroah-Hartman
@ 2023-04-21  6:29   ` Johan Hovold
  2023-04-21  8:15   ` Alex Elder
  2023-05-08 23:55   ` Tejun Heo
  3 siblings, 0 replies; 70+ messages in thread
From: Johan Hovold @ 2023-04-21  6:29 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jiangshanlai, linux-kernel, kernel-team, Alex Elder,
	Greg Kroah-Hartman, greybus-dev

On Thu, Apr 20, 2023 at 04:50:26PM -1000, Tejun Heo wrote:
> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().

> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Alex Elder <elder@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: greybus-dev@lists.linaro.org

Acked-by: Johan Hovold <johan@kernel.org>

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 02/22] greybus: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 02/22] greybus: " Tejun Heo
  2023-04-21  5:03   ` Greg Kroah-Hartman
  2023-04-21  6:29   ` Johan Hovold
@ 2023-04-21  8:15   ` Alex Elder
  2023-05-08 23:55   ` Tejun Heo
  3 siblings, 0 replies; 70+ messages in thread
From: Alex Elder @ 2023-04-21  8:15 UTC (permalink / raw)
  To: Tejun Heo, jiangshanlai
  Cc: linux-kernel, kernel-team, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, greybus-dev

On 4/20/23 9:50 PM, Tejun Heo wrote:
> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
> 
> While this has worked okay, overloading the UNBOUND allocation interface
> this way creates other issues. It's difficult to tell whether a given
> workqueue actually needs to be ordered and users that legitimately want a
> min concurrency level wq unexpectedly gets an ordered one instead. With
> planned UNBOUND workqueue updates to improve execution locality and more
> prevalence of chiplet designs which can benefit from such improvements, this
> isn't a state we wanna be in forever.
> 
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>    alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>    alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.

Both of the workqueues affected here should be ordered.

Acked-by: Alex Elder <elder@linaro.org>

> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Alex Elder <elder@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: greybus-dev@lists.linaro.org
> ---
>   drivers/greybus/connection.c | 4 ++--
>   drivers/greybus/svc.c        | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/greybus/connection.c b/drivers/greybus/connection.c
> index e3799a53a193..9c88861986c8 100644
> --- a/drivers/greybus/connection.c
> +++ b/drivers/greybus/connection.c
> @@ -187,8 +187,8 @@ _gb_connection_create(struct gb_host_device *hd, int hd_cport_id,
>   	spin_lock_init(&connection->lock);
>   	INIT_LIST_HEAD(&connection->operations);
>   
> -	connection->wq = alloc_workqueue("%s:%d", WQ_UNBOUND, 1,
> -					 dev_name(&hd->dev), hd_cport_id);
> +	connection->wq = alloc_ordered_workqueue("%s:%d", 0, dev_name(&hd->dev),
> +						 hd_cport_id);
>   	if (!connection->wq) {
>   		ret = -ENOMEM;
>   		goto err_free_connection;
> diff --git a/drivers/greybus/svc.c b/drivers/greybus/svc.c
> index 16cced80867a..0d7e749174a4 100644
> --- a/drivers/greybus/svc.c
> +++ b/drivers/greybus/svc.c
> @@ -1318,7 +1318,7 @@ struct gb_svc *gb_svc_create(struct gb_host_device *hd)
>   	if (!svc)
>   		return NULL;
>   
> -	svc->wq = alloc_workqueue("%s:svc", WQ_UNBOUND, 1, dev_name(&hd->dev));
> +	svc->wq = alloc_ordered_workqueue("%s:svc", 0, dev_name(&hd->dev));
>   	if (!svc->wq) {
>   		kfree(svc);
>   		return NULL;


^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50   ` Tejun Heo
@ 2023-04-21 14:01     ` Jakub Kicinski
  -1 siblings, 0 replies; 70+ messages in thread
From: Jakub Kicinski @ 2023-04-21 14:01 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jiangshanlai, linux-kernel, kernel-team, Sunil Goutham,
	David S. Miller, Eric Dumazet, Paolo Abeni, linux-arm-kernel,
	netdev

On Thu, 20 Apr 2023 16:50:30 -1000 Tejun Heo wrote:
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Sunil Goutham <sgoutham@marvell.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: netdev@vger.kernel.org

You take this via your tree directly to Linus T?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to create ordered workqueues
@ 2023-04-21 14:01     ` Jakub Kicinski
  0 siblings, 0 replies; 70+ messages in thread
From: Jakub Kicinski @ 2023-04-21 14:01 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jiangshanlai, linux-kernel, kernel-team, Sunil Goutham,
	David S. Miller, Eric Dumazet, Paolo Abeni, linux-arm-kernel,
	netdev

On Thu, 20 Apr 2023 16:50:30 -1000 Tejun Heo wrote:
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Sunil Goutham <sgoutham@marvell.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: netdev@vger.kernel.org

You take this via your tree directly to Linus T?

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 03/22] IB/hfi1: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 03/22] IB/hfi1: " Tejun Heo
@ 2023-04-21 14:02   ` Dennis Dalessandro
  2023-05-08 23:56     ` Tejun Heo
  0 siblings, 1 reply; 70+ messages in thread
From: Dennis Dalessandro @ 2023-04-21 14:02 UTC (permalink / raw)
  To: Tejun Heo, jiangshanlai
  Cc: linux-kernel, kernel-team, Jason Gunthorpe, Leon Romanovsky, linux-rdma

On 4/20/23 10:50 PM, Tejun Heo wrote:
> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
> 
> While this has worked okay, overloading the UNBOUND allocation interface
> this way creates other issues. It's difficult to tell whether a given
> workqueue actually needs to be ordered and users that legitimately want a
> min concurrency level wq unexpectedly gets an ordered one instead. With
> planned UNBOUND workqueue updates to improve execution locality and more
> prevalence of chiplet designs which can benefit from such improvements, this
> isn't a state we wanna be in forever.
> 
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: linux-rdma@vger.kernel.org
> ---
>  drivers/infiniband/hw/hfi1/init.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c
> index 62b6c5020039..e03d867cda13 100644
> --- a/drivers/infiniband/hw/hfi1/init.c
> +++ b/drivers/infiniband/hw/hfi1/init.c
> @@ -755,14 +755,13 @@ static int create_workqueues(struct hfi1_devdata *dd)
>  		}
>  		if (!ppd->link_wq) {
>  			/*
> -			 * Make the link workqueue single-threaded to enforce
> +			 * Make the link workqueue ordered to enforce
>  			 * serialization.
>  			 */
>  			ppd->link_wq =
> -				alloc_workqueue(
> +				alloc_ordered_workqueue(
>  				    "hfi_link_%d_%d",
> -				    WQ_SYSFS | WQ_MEM_RECLAIM | WQ_UNBOUND,
> -				    1, /* max_active */
> +				    WQ_SYSFS | WQ_MEM_RECLAIM,
>  				    dd->unit, pidx);
>  			if (!ppd->link_wq)
>  				goto wq_error;

Seems OK to me.

Acked-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21 14:01     ` Jakub Kicinski
@ 2023-04-21 14:13       ` Tejun Heo
  -1 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21 14:13 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: jiangshanlai, linux-kernel, kernel-team, Sunil Goutham,
	David S. Miller, Eric Dumazet, Paolo Abeni, linux-arm-kernel,
	netdev

On Fri, Apr 21, 2023 at 07:01:08AM -0700, Jakub Kicinski wrote:
> On Thu, 20 Apr 2023 16:50:30 -1000 Tejun Heo wrote:
> > Signed-off-by: Tejun Heo <tj@kernel.org>
> > Cc: Sunil Goutham <sgoutham@marvell.com>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Paolo Abeni <pabeni@redhat.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: netdev@vger.kernel.org
> 
> You take this via your tree directly to Linus T?

Yeah, that'd be my preference unless someone is really against it.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to create ordered workqueues
@ 2023-04-21 14:13       ` Tejun Heo
  0 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-04-21 14:13 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: jiangshanlai, linux-kernel, kernel-team, Sunil Goutham,
	David S. Miller, Eric Dumazet, Paolo Abeni, linux-arm-kernel,
	netdev

On Fri, Apr 21, 2023 at 07:01:08AM -0700, Jakub Kicinski wrote:
> On Thu, 20 Apr 2023 16:50:30 -1000 Tejun Heo wrote:
> > Signed-off-by: Tejun Heo <tj@kernel.org>
> > Cc: Sunil Goutham <sgoutham@marvell.com>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Paolo Abeni <pabeni@redhat.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: netdev@vger.kernel.org
> 
> You take this via your tree directly to Linus T?

Yeah, that'd be my preference unless someone is really against it.

Thanks.

-- 
tejun

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 12/22] scsi: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 12/22] scsi: " Tejun Heo
@ 2023-04-21 14:23   ` Benjamin Block
  2023-05-08 23:57   ` Tejun Heo
  1 sibling, 0 replies; 70+ messages in thread
From: Benjamin Block @ 2023-04-21 14:23 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jiangshanlai, linux-kernel, kernel-team, James E.J. Bottomley,
	Martin K. Petersen, linux-scsi

On Thu, Apr 20, 2023 at 04:50:36PM -1000, Tejun Heo wrote:
> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
> 
> While this has worked okay, overloading the UNBOUND allocation interface
> this way creates other issues. It's difficult to tell whether a given
> workqueue actually needs to be ordered and users that legitimately want a
> min concurrency level wq unexpectedly gets an ordered one instead. With
> planned UNBOUND workqueue updates to improve execution locality and more
> prevalence of chiplet designs which can benefit from such improvements, this
> isn't a state we wanna be in forever.
> 
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
> Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> Cc: linux-scsi@vger.kernel.org
> ---
>  drivers/scsi/NCR5380.c  |  5 ++---
>  drivers/scsi/hosts.c    | 12 ++++++------
>  drivers/scsi/libiscsi.c |  5 ++---
>  3 files changed, 10 insertions(+), 12 deletions(-)
> 

The conversions look good to me.


Reviewed-by: Benjamin Block <bblock@linux.ibm.com>

-- 
Best Regards, Benjamin Block        /        Linux on IBM Z Kernel Development
IBM Deutschland Research & Development GmbH    /   https://www.ibm.com/privacy
Vors. Aufs.-R.: Gregor Pillen         /         Geschäftsführung: David Faller
Sitz der Ges.: Böblingen     /    Registergericht: AmtsG Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21 14:13       ` Tejun Heo
@ 2023-04-21 14:28         ` Jakub Kicinski
  -1 siblings, 0 replies; 70+ messages in thread
From: Jakub Kicinski @ 2023-04-21 14:28 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jiangshanlai, linux-kernel, kernel-team, Sunil Goutham,
	David S. Miller, Eric Dumazet, Paolo Abeni, linux-arm-kernel,
	netdev

On Fri, 21 Apr 2023 04:13:20 -1000 Tejun Heo wrote:
> On Fri, Apr 21, 2023 at 07:01:08AM -0700, Jakub Kicinski wrote:
> > On Thu, 20 Apr 2023 16:50:30 -1000 Tejun Heo wrote:  
> > > Signed-off-by: Tejun Heo <tj@kernel.org>
> > > Cc: Sunil Goutham <sgoutham@marvell.com>
> > > Cc: "David S. Miller" <davem@davemloft.net>
> > > Cc: Eric Dumazet <edumazet@google.com>
> > > Cc: Jakub Kicinski <kuba@kernel.org>
> > > Cc: Paolo Abeni <pabeni@redhat.com>
> > > Cc: linux-arm-kernel@lists.infradead.org
> > > Cc: netdev@vger.kernel.org  
> > 
> > You take this via your tree directly to Linus T?  
> 
> Yeah, that'd be my preference unless someone is really against it.

Acked-by: Jakub Kicinski <kuba@kernel.org>

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to create ordered workqueues
@ 2023-04-21 14:28         ` Jakub Kicinski
  0 siblings, 0 replies; 70+ messages in thread
From: Jakub Kicinski @ 2023-04-21 14:28 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jiangshanlai, linux-kernel, kernel-team, Sunil Goutham,
	David S. Miller, Eric Dumazet, Paolo Abeni, linux-arm-kernel,
	netdev

On Fri, 21 Apr 2023 04:13:20 -1000 Tejun Heo wrote:
> On Fri, Apr 21, 2023 at 07:01:08AM -0700, Jakub Kicinski wrote:
> > On Thu, 20 Apr 2023 16:50:30 -1000 Tejun Heo wrote:  
> > > Signed-off-by: Tejun Heo <tj@kernel.org>
> > > Cc: Sunil Goutham <sgoutham@marvell.com>
> > > Cc: "David S. Miller" <davem@davemloft.net>
> > > Cc: Eric Dumazet <edumazet@google.com>
> > > Cc: Jakub Kicinski <kuba@kernel.org>
> > > Cc: Paolo Abeni <pabeni@redhat.com>
> > > Cc: linux-arm-kernel@lists.infradead.org
> > > Cc: netdev@vger.kernel.org  
> > 
> > You take this via your tree directly to Linus T?  
> 
> Yeah, that'd be my preference unless someone is really against it.

Acked-by: Jakub Kicinski <kuba@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 17/22] cifs: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 17/22] cifs: " Tejun Heo
@ 2023-04-21 18:38   ` Paulo Alcantara
  2023-05-08 23:58     ` Tejun Heo
  0 siblings, 1 reply; 70+ messages in thread
From: Paulo Alcantara @ 2023-04-21 18:38 UTC (permalink / raw)
  To: Tejun Heo, jiangshanlai
  Cc: linux-kernel, kernel-team, Tejun Heo, Steve French,
	Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	linux-cifs, samba-technical

Tejun Heo <tj@kernel.org> writes:

> BACKGROUND
> ==========
>
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
>
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
>
> While this has worked okay, overloading the UNBOUND allocation interface
> this way creates other issues. It's difficult to tell whether a given
> workqueue actually needs to be ordered and users that legitimately want a
> min concurrency level wq unexpectedly gets an ordered one instead. With
> planned UNBOUND workqueue updates to improve execution locality and more
> prevalence of chiplet designs which can benefit from such improvements, this
> isn't a state we wanna be in forever.
>
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
>
> WHAT TO LOOK FOR
> ================
>
> The conversions are from
>
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
>
> to
>
>   alloc_ordered_workqueue(flags, args...)
>
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
>
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
>
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Steve French <sfrench@samba.org>
> Cc: Paulo Alcantara <pc@cjr.nz>
> Cc: Ronnie Sahlberg <lsahlber@redhat.com>
> Cc: Shyam Prasad N <sprasad@microsoft.com>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: linux-cifs@vger.kernel.org
> Cc: samba-technical@lists.samba.org
> ---
>  fs/cifs/dfs_cache.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Paulo Alcantara (SUSE) <pc@manguebit.com>

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 22/22] workqueue: Don't implicitly make UNBOUND workqueues w/ @max_active==1 ordered
  2023-04-21  2:50 ` [PATCH 22/22] workqueue: Don't implicitly make UNBOUND workqueues w/ @max_active==1 ordered Tejun Heo
@ 2023-04-24  5:03   ` Christoph Hellwig
  0 siblings, 0 replies; 70+ messages in thread
From: Christoph Hellwig @ 2023-04-24  5:03 UTC (permalink / raw)
  To: Tejun Heo; +Cc: jiangshanlai, linux-kernel, kernel-team, Christoph Hellwig

On Thu, Apr 20, 2023 at 04:50:46PM -1000, Tejun Heo wrote:
> There aren't that many UNBOUND w/ @max_active==1 users in the tree and the
> preceding patches audited all and converted them to
> alloc_ordered_workqueue() as appropriate. This patch removes the implicit
> promotion of UNBOUND w/ @max_active==1 workqueues to ordered ones.

And because you only Cc'ed me on this patch but not the rest of the
series there is no way to make me see it.  Either cc me everywhere
because it's actually important, or preferably just drop fringe Ccs.

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 09/22] wifi: iwlwifi: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 09/22] wifi: iwlwifi: " Tejun Heo
@ 2023-04-24 17:31   ` Johannes Berg
  2023-05-05 22:52     ` [PATCH] wifi: iwlwifi: Use default @max_active for trans_pcie->rba.alloc_wq Tejun Heo
  2023-05-08 23:59   ` [PATCH 09/22] wifi: iwlwifi: Use alloc_ordered_workqueue() to create ordered workqueues Tejun Heo
  1 sibling, 1 reply; 70+ messages in thread
From: Johannes Berg @ 2023-04-24 17:31 UTC (permalink / raw)
  To: Tejun Heo, jiangshanlai
  Cc: linux-kernel, kernel-team, Kalle Valo, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Gregory Greenman,
	Avraham Stern, Kees Cook, Mordechay Goodstein, Haim, Dreyfuss,
	linux-wireless, netdev

On Thu, 2023-04-20 at 16:50 -1000, Tejun Heo wrote:
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.

This workqueue only has a single work struct queued on it, I'm not
_entirely_ sure why there's even a separate workqueue (possibly for
priority reasons etc.), but surely with just a single work struct, order
cannot really matter.

johannes


^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 10/22] wifi: mwifiex: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 10/22] wifi: mwifiex: " Tejun Heo
@ 2023-04-25 18:14   ` Brian Norris
  2023-05-05 22:53     ` [PATCH] wifi: mwifiex: Use default @max_active for workqueues Tejun Heo
  0 siblings, 1 reply; 70+ messages in thread
From: Brian Norris @ 2023-04-25 18:14 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jiangshanlai, linux-kernel, kernel-team, Amitkumar Karwar,
	Ganapathi Bhat, Sharvari Harisangam, Xinming Hu, Kalle Valo,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-wireless, netdev

On Thu, Apr 20, 2023 at 04:50:34PM -1000, Tejun Heo wrote:
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Amitkumar Karwar <amitkarwar@gmail.com>
> Cc: Ganapathi Bhat <ganapathi017@gmail.com>
> Cc: Sharvari Harisangam <sharvari.harisangam@nxp.com>
> Cc: Xinming Hu <huxinming820@gmail.com>
> Cc: Kalle Valo <kvalo@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  .../net/wireless/marvell/mwifiex/cfg80211.c   | 13 +++++------
>  drivers/net/wireless/marvell/mwifiex/main.c   | 22 +++++++++----------
>  2 files changed, 16 insertions(+), 19 deletions(-)

These work queues only ever get a single work item on them, so
"ordering" can't really matter. This could go either way -- a comment or
the current patch; so:

Reviewed-by: Brian Norris <briannorris@chromium.org>

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 16/22] btrfs: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 16/22] btrfs: " Tejun Heo
@ 2023-04-30  4:40   ` Wang Yugui
  2023-05-05 22:58     ` [PATCH v2 " Tejun Heo
  0 siblings, 1 reply; 70+ messages in thread
From: Wang Yugui @ 2023-04-30  4:40 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jiangshanlai, linux-kernel, kernel-team, Chris Mason,
	Josef Bacik, David Sterba, linux-btrfs

Hi,

> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
> 
> While this has worked okay, overloading the UNBOUND allocation interface
> this way creates other issues. It's difficult to tell whether a given
> workqueue actually needs to be ordered and users that legitimately want a
> min concurrency level wq unexpectedly gets an ordered one instead. With
> planned UNBOUND workqueue updates to improve execution locality and more
> prevalence of chiplet designs which can benefit from such improvements, this
> isn't a state we wanna be in forever.
> 
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.

Should we  add alloc_ordered_workqueue to 
btrfs_alloc_workqueue() of fs/btrfs/async-thread.c too?

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2023/04/30

> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Chris Mason <clm@fb.com>
> Cc: Josef Bacik <josef@toxicpanda.com>
> Cc: David Sterba <dsterba@suse.com>
> Cc: linux-btrfs@vger.kernel.org
> ---
>  fs/btrfs/disk-io.c | 2 +-
>  fs/btrfs/scrub.c   | 6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 9e1596bb208d..b1f6ff69dbe1 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2218,7 +2218,7 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info)
>  	fs_info->qgroup_rescan_workers =
>  		btrfs_alloc_workqueue(fs_info, "qgroup-rescan", flags, 1, 0);
>  	fs_info->discard_ctl.discard_workers =
> -		alloc_workqueue("btrfs_discard", WQ_UNBOUND | WQ_FREEZABLE, 1);
> +		alloc_ordered_workqueue("btrfs_discard", WQ_FREEZABLE);
>  
>  	if (!(fs_info->workers && fs_info->hipri_workers &&
>  	      fs_info->delalloc_workers && fs_info->flush_workers &&
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 69c93ae333f6..70882358bdb0 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -4245,8 +4245,10 @@ static noinline_for_stack int scrub_workers_get(struct btrfs_fs_info *fs_info,
>  	if (refcount_inc_not_zero(&fs_info->scrub_workers_refcnt))
>  		return 0;
>  
> -	scrub_workers = alloc_workqueue("btrfs-scrub", flags,
> -					is_dev_replace ? 1 : max_active);
> +	if (is_dev_replace)
> +		scrub_workers = alloc_ordered_workqueue("btrfs-scrub", flags);
> +	else
> +		scrub_workers = alloc_workqueue("btrfs-scrub", flags, max_active);
>  	if (!scrub_workers)
>  		goto fail_scrub_workers;
>  
> -- 
> 2.40.0



^ permalink raw reply	[flat|nested] 70+ messages in thread

* [PATCH] wifi: iwlwifi: Use default @max_active for trans_pcie->rba.alloc_wq
  2023-04-24 17:31   ` Johannes Berg
@ 2023-05-05 22:52     ` Tejun Heo
  2023-05-08 16:05       ` Johannes Berg
  0 siblings, 1 reply; 70+ messages in thread
From: Tejun Heo @ 2023-05-05 22:52 UTC (permalink / raw)
  To: Johannes Berg
  Cc: jiangshanlai, linux-kernel, kernel-team, Kalle Valo,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Gregory Greenman, Avraham Stern, Kees Cook, Mordechay Goodstein,
	Haim, Dreyfuss, linux-wireless, netdev

trans_pcie->rba.alloc_wq only hosts a single work item and thus doesn't need
explicit concurrency limit. Let's use the default @max_active. This doesn't
cost anything and clearly expresses that @max_active doesn't matter.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Gregory Greenman <gregory.greenman@intel.com>
Cc: Johannes Berg <johannes.berg@intel.com>
Cc: Avraham Stern <avraham.stern@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mordechay Goodstein <mordechay.goodstein@intel.com>
Cc: "Haim, Dreyfuss" <haim.dreyfuss@intel.com>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
Hello,

Johannes, do you mind acking this patch instead?

Thanks.

 drivers/net/wireless/intel/iwlwifi/pcie/trans.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -3577,7 +3577,7 @@ struct iwl_trans *iwl_trans_pcie_alloc(s
 	init_waitqueue_head(&trans_pcie->imr_waitq);
 
 	trans_pcie->rba.alloc_wq = alloc_workqueue("rb_allocator",
-						   WQ_HIGHPRI | WQ_UNBOUND, 1);
+						   WQ_HIGHPRI | WQ_UNBOUND, 0);
 	if (!trans_pcie->rba.alloc_wq) {
 		ret = -ENOMEM;
 		goto out_free_trans;

^ permalink raw reply	[flat|nested] 70+ messages in thread

* [PATCH] wifi: mwifiex: Use default @max_active for workqueues
  2023-04-25 18:14   ` Brian Norris
@ 2023-05-05 22:53     ` Tejun Heo
  0 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-05 22:53 UTC (permalink / raw)
  To: Brian Norris
  Cc: jiangshanlai, linux-kernel, kernel-team, Amitkumar Karwar,
	Ganapathi Bhat, Sharvari Harisangam, Xinming Hu, Kalle Valo,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	linux-wireless, netdev

These workqueues only host a single work item and thus doen't need explicit
concurrency limit. Let's use the default @max_active. This doesn't cost
anything and clearly expresses that @max_active doesn't matter.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Amitkumar Karwar <amitkarwar@gmail.com>
Cc: Ganapathi Bhat <ganapathi017@gmail.com>
Cc: Sharvari Harisangam <sharvari.harisangam@nxp.com>
Cc: Xinming Hu <huxinming820@gmail.com>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
Hello, Brian.

Do you mind acking this patch instead?

Thanks.

 drivers/net/wireless/marvell/mwifiex/cfg80211.c |    4 ++--
 drivers/net/wireless/marvell/mwifiex/main.c     |    8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -3127,7 +3127,7 @@ struct wireless_dev *mwifiex_add_virtual
 	priv->dfs_cac_workqueue = alloc_workqueue("MWIFIEX_DFS_CAC%s",
 						  WQ_HIGHPRI |
 						  WQ_MEM_RECLAIM |
-						  WQ_UNBOUND, 1, name);
+						  WQ_UNBOUND, 0, name);
 	if (!priv->dfs_cac_workqueue) {
 		mwifiex_dbg(adapter, ERROR, "cannot alloc DFS CAC queue\n");
 		ret = -ENOMEM;
@@ -3138,7 +3138,7 @@ struct wireless_dev *mwifiex_add_virtual
 
 	priv->dfs_chan_sw_workqueue = alloc_workqueue("MWIFIEX_DFS_CHSW%s",
 						      WQ_HIGHPRI | WQ_UNBOUND |
-						      WQ_MEM_RECLAIM, 1, name);
+						      WQ_MEM_RECLAIM, 0, name);
 	if (!priv->dfs_chan_sw_workqueue) {
 		mwifiex_dbg(adapter, ERROR, "cannot alloc DFS channel sw queue\n");
 		ret = -ENOMEM;
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1547,7 +1547,7 @@ mwifiex_reinit_sw(struct mwifiex_adapter
 
 	adapter->workqueue =
 		alloc_workqueue("MWIFIEX_WORK_QUEUE",
-				WQ_HIGHPRI | WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
+				WQ_HIGHPRI | WQ_MEM_RECLAIM | WQ_UNBOUND, 0);
 	if (!adapter->workqueue)
 		goto err_kmalloc;
 
@@ -1557,7 +1557,7 @@ mwifiex_reinit_sw(struct mwifiex_adapter
 		adapter->rx_workqueue = alloc_workqueue("MWIFIEX_RX_WORK_QUEUE",
 							WQ_HIGHPRI |
 							WQ_MEM_RECLAIM |
-							WQ_UNBOUND, 1);
+							WQ_UNBOUND, 0);
 		if (!adapter->rx_workqueue)
 			goto err_kmalloc;
 		INIT_WORK(&adapter->rx_work, mwifiex_rx_work_queue);
@@ -1702,7 +1702,7 @@ mwifiex_add_card(void *card, struct comp
 
 	adapter->workqueue =
 		alloc_workqueue("MWIFIEX_WORK_QUEUE",
-				WQ_HIGHPRI | WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
+				WQ_HIGHPRI | WQ_MEM_RECLAIM | WQ_UNBOUND, 0);
 	if (!adapter->workqueue)
 		goto err_kmalloc;
 
@@ -1712,7 +1712,7 @@ mwifiex_add_card(void *card, struct comp
 		adapter->rx_workqueue = alloc_workqueue("MWIFIEX_RX_WORK_QUEUE",
 							WQ_HIGHPRI |
 							WQ_MEM_RECLAIM |
-							WQ_UNBOUND, 1);
+							WQ_UNBOUND, 0);
 		if (!adapter->rx_workqueue)
 			goto err_kmalloc;
 

^ permalink raw reply	[flat|nested] 70+ messages in thread

* [PATCH v2 16/22] btrfs: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-30  4:40   ` Wang Yugui
@ 2023-05-05 22:58     ` Tejun Heo
  2023-05-06  1:40       ` Wang Yugui
  0 siblings, 1 reply; 70+ messages in thread
From: Tejun Heo @ 2023-05-05 22:58 UTC (permalink / raw)
  To: Wang Yugui
  Cc: jiangshanlai, linux-kernel, kernel-team, Chris Mason,
	Josef Bacik, David Sterba, linux-btrfs

BACKGROUND
==========

When multiple work items are queued to a workqueue, their execution order
doesn't match the queueing order. They may get executed in any order and
simultaneously. When fully serialized execution - one by one in the queueing
order - is needed, an ordered workqueue should be used which can be created
with alloc_ordered_workqueue().

However, alloc_ordered_workqueue() was a later addition. Before it, an
ordered workqueue could be obtained by creating an UNBOUND workqueue with
@max_active==1. This originally was an implementation side-effect which was
broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
ordered"). Because there were users that depended on the ordered execution,
5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
made workqueue allocation path to implicitly promote UNBOUND workqueues w/
@max_active==1 to ordered workqueues.

While this has worked okay, overloading the UNBOUND allocation interface
this way creates other issues. It's difficult to tell whether a given
workqueue actually needs to be ordered and users that legitimately want a
min concurrency level wq unexpectedly gets an ordered one instead. With
planned UNBOUND workqueue updates to improve execution locality and more
prevalence of chiplet designs which can benefit from such improvements, this
isn't a state we wanna be in forever.

This patch series audits all callsites that create an UNBOUND workqueue w/
@max_active==1 and converts them to alloc_ordered_workqueue() as necessary.

WHAT TO LOOK FOR
================

The conversions are from

  alloc_workqueue(WQ_UNBOUND | flags, 1, args..)

to 

  alloc_ordered_workqueue(flags, args...)

which don't cause any functional changes. If you know that fully ordered
execution is not ncessary, please let me know. I'll drop the conversion and
instead add a comment noting the fact to reduce confusion while conversion
is in progress.

If you aren't fully sure, it's completely fine to let the conversion
through. The behavior will stay exactly the same and we can always
reconsider later.

As there are follow-up workqueue core changes, I'd really appreciate if the
patch can be routed through the workqueue tree w/ your acks. Thanks.

v2: btrfs_alloc_workqueue() updated too as suggested by Wang.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Wang Yugui <wangyugui@e16-tech.com>
Cc: Chris Mason <clm@fb.com>
Cc: Josef Bacik <josef@toxicpanda.com>
Cc: David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org
---
Hello,

Wang, yeah, that's a helper that can't tell whether the caller wants an
ordered wq or not, so it needs to be updated too. How does this look?

Thanks.

 fs/btrfs/async-thread.c |    7 +++++--
 fs/btrfs/disk-io.c      |    2 +-
 fs/btrfs/scrub.c        |    6 ++++--
 3 files changed, 10 insertions(+), 5 deletions(-)

--- a/fs/btrfs/async-thread.c
+++ b/fs/btrfs/async-thread.c
@@ -99,8 +99,11 @@ struct btrfs_workqueue *btrfs_alloc_work
 		ret->thresh = thresh;
 	}
 
-	ret->normal_wq = alloc_workqueue("btrfs-%s", flags, ret->current_active,
-					 name);
+	if (ret->current_active == 1)
+		ret->normal_wq = alloc_ordered_workqueue("btrfs-%s", flags, name);
+	else
+		ret->normal_wq = alloc_workqueue("btrfs-%s", flags,
+						 ret->current_active, name);
 	if (!ret->normal_wq) {
 		kfree(ret);
 		return NULL;
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2218,7 +2218,7 @@ static int btrfs_init_workqueues(struct
 	fs_info->qgroup_rescan_workers =
 		btrfs_alloc_workqueue(fs_info, "qgroup-rescan", flags, 1, 0);
 	fs_info->discard_ctl.discard_workers =
-		alloc_workqueue("btrfs_discard", WQ_UNBOUND | WQ_FREEZABLE, 1);
+		alloc_ordered_workqueue("btrfs_discard", WQ_FREEZABLE);
 
 	if (!(fs_info->workers && fs_info->hipri_workers &&
 	      fs_info->delalloc_workers && fs_info->flush_workers &&
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -4245,8 +4245,10 @@ static noinline_for_stack int scrub_work
 	if (refcount_inc_not_zero(&fs_info->scrub_workers_refcnt))
 		return 0;
 
-	scrub_workers = alloc_workqueue("btrfs-scrub", flags,
-					is_dev_replace ? 1 : max_active);
+	if (is_dev_replace)
+		scrub_workers = alloc_ordered_workqueue("btrfs-scrub", flags);
+	else
+		scrub_workers = alloc_workqueue("btrfs-scrub", flags, max_active);
 	if (!scrub_workers)
 		goto fail_scrub_workers;
 

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH v2 16/22] btrfs: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-05-05 22:58     ` [PATCH v2 " Tejun Heo
@ 2023-05-06  1:40       ` Wang Yugui
  2023-05-08 23:50         ` Tejun Heo
  0 siblings, 1 reply; 70+ messages in thread
From: Wang Yugui @ 2023-05-06  1:40 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jiangshanlai, linux-kernel, kernel-team, Chris Mason,
	Josef Bacik, David Sterba, linux-btrfs

Hi,

> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
> 
> While this has worked okay, overloading the UNBOUND allocation interface
> this way creates other issues. It's difficult to tell whether a given
> workqueue actually needs to be ordered and users that legitimately want a
> min concurrency level wq unexpectedly gets an ordered one instead. With
> planned UNBOUND workqueue updates to improve execution locality and more
> prevalence of chiplet designs which can benefit from such improvements, this
> isn't a state we wanna be in forever.
> 
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>   alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to 
> 
>   alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
> 
> v2: btrfs_alloc_workqueue() updated too as suggested by Wang.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Wang Yugui <wangyugui@e16-tech.com>
> Cc: Chris Mason <clm@fb.com>
> Cc: Josef Bacik <josef@toxicpanda.com>
> Cc: David Sterba <dsterba@suse.com>
> Cc: linux-btrfs@vger.kernel.org
> ---
> Hello,
> 
> Wang, yeah, that's a helper that can't tell whether the caller wants an
> ordered wq or not, so it needs to be updated too. How does this look?
> 
> Thanks.
> 
>  fs/btrfs/async-thread.c |    7 +++++--
>  fs/btrfs/disk-io.c      |    2 +-
>  fs/btrfs/scrub.c        |    6 ++++--
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> --- a/fs/btrfs/async-thread.c
> +++ b/fs/btrfs/async-thread.c
> @@ -99,8 +99,11 @@ struct btrfs_workqueue *btrfs_alloc_work
>  		ret->thresh = thresh;
>  	}
>  
> -	ret->normal_wq = alloc_workqueue("btrfs-%s", flags, ret->current_active,
> -					 name);
> +	if (ret->current_active == 1)
> +		ret->normal_wq = alloc_ordered_workqueue("btrfs-%s", flags, name);
> +	else
> +		ret->normal_wq = alloc_workqueue("btrfs-%s", flags,
> +						 ret->current_active, name);
>  	if (!ret->normal_wq) {
>  		kfree(ret);
>  		return NULL;

by test, I noticed some warning caused by
void workqueue_set_max_active(struct workqueue_struct *wq, int max_active)
	if (WARN_ON(wq->flags & __WQ_ORDERED_EXPLICIT))
		return;

so I tested again  with the flowing fix

diff --git a/fs/btrfs/async-thread.c b/fs/btrfs/async-thread.c
index 43c8995..e4b68e9 100644
--- a/fs/btrfs/async-thread.c
+++ b/fs/btrfs/async-thread.c
@@ -99,8 +99,11 @@ struct btrfs_workqueue *btrfs_alloc_workqueue(struct btrfs_fs_info *fs_info,
 		ret->thresh = thresh;
 	}
 
-	ret->normal_wq = alloc_workqueue("btrfs-%s", flags, ret->current_active,
-					 name);
+	if(limit_active == 1)
+		ret->normal_wq = alloc_ordered_workqueue("btrfs-%s", flags, name);
+	else
+		ret->normal_wq = alloc_workqueue("btrfs-%s", flags,
+					 ret->current_active, name);
 	if (!ret->normal_wq) {
 		kfree(ret);
 		return NULL;
@@ -139,7 +139,7 @@ static inline void thresh_exec_hook(struct btrfs_workqueue *wq)
 	long pending;
 	int need_change = 0;
 
-	if (wq->thresh == NO_THRESHOLD)
+	if (wq->thresh == NO_THRESHOLD || wq->limit_active == 1)
 		return;
 
 	atomic_dec(&wq->pending);

we need 'limit_active' at 2nd postition, so I used 'limit_active' and 1st
postition too.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2023/05/06



> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2218,7 +2218,7 @@ static int btrfs_init_workqueues(struct
>  	fs_info->qgroup_rescan_workers =
>  		btrfs_alloc_workqueue(fs_info, "qgroup-rescan", flags, 1, 0);
>  	fs_info->discard_ctl.discard_workers =
> -		alloc_workqueue("btrfs_discard", WQ_UNBOUND | WQ_FREEZABLE, 1);
> +		alloc_ordered_workqueue("btrfs_discard", WQ_FREEZABLE);
>  
>  	if (!(fs_info->workers && fs_info->hipri_workers &&
>  	      fs_info->delalloc_workers && fs_info->flush_workers &&
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -4245,8 +4245,10 @@ static noinline_for_stack int scrub_work
>  	if (refcount_inc_not_zero(&fs_info->scrub_workers_refcnt))
>  		return 0;
>  
> -	scrub_workers = alloc_workqueue("btrfs-scrub", flags,
> -					is_dev_replace ? 1 : max_active);
> +	if (is_dev_replace)
> +		scrub_workers = alloc_ordered_workqueue("btrfs-scrub", flags);
> +	else
> +		scrub_workers = alloc_workqueue("btrfs-scrub", flags, max_active);
>  	if (!scrub_workers)
>  		goto fail_scrub_workers;
>  



^ permalink raw reply related	[flat|nested] 70+ messages in thread

* Re: [PATCH 15/22] xen/pvcalls: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 15/22] xen/pvcalls: " Tejun Heo
@ 2023-05-08 11:58   ` Juergen Gross
  2023-05-08 23:59   ` Tejun Heo
  1 sibling, 0 replies; 70+ messages in thread
From: Juergen Gross @ 2023-05-08 11:58 UTC (permalink / raw)
  To: Tejun Heo, jiangshanlai
  Cc: linux-kernel, kernel-team, Stefano Stabellini,
	Oleksandr Tyshchenko, xen-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 2734 bytes --]

On 21.04.23 04:50, Tejun Heo wrote:
> BACKGROUND
> ==========
> 
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
> 
> While this has worked okay, overloading the UNBOUND allocation interface
> this way creates other issues. It's difficult to tell whether a given
> workqueue actually needs to be ordered and users that legitimately want a
> min concurrency level wq unexpectedly gets an ordered one instead. With
> planned UNBOUND workqueue updates to improve execution locality and more
> prevalence of chiplet designs which can benefit from such improvements, this
> isn't a state we wanna be in forever.
> 
> This patch series audits all callsites that create an UNBOUND workqueue w/
> @max_active==1 and converts them to alloc_ordered_workqueue() as necessary.
> 
> WHAT TO LOOK FOR
> ================
> 
> The conversions are from
> 
>    alloc_workqueue(WQ_UNBOUND | flags, 1, args..)
> 
> to
> 
>    alloc_ordered_workqueue(flags, args...)
> 
> which don't cause any functional changes. If you know that fully ordered
> execution is not ncessary, please let me know. I'll drop the conversion and
> instead add a comment noting the fact to reduce confusion while conversion
> is in progress.
> 
> If you aren't fully sure, it's completely fine to let the conversion
> through. The behavior will stay exactly the same and we can always
> reconsider later.
> 
> As there are follow-up workqueue core changes, I'd really appreciate if the
> patch can be routed through the workqueue tree w/ your acks. Thanks.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Cc: xen-devel@lists.xenproject.org

Acked-by: Juergen Gross <jgross@suse.com>


Juergen


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH] wifi: iwlwifi: Use default @max_active for trans_pcie->rba.alloc_wq
  2023-05-05 22:52     ` [PATCH] wifi: iwlwifi: Use default @max_active for trans_pcie->rba.alloc_wq Tejun Heo
@ 2023-05-08 16:05       ` Johannes Berg
  0 siblings, 0 replies; 70+ messages in thread
From: Johannes Berg @ 2023-05-08 16:05 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jiangshanlai, linux-kernel, kernel-team, Kalle Valo,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Gregory Greenman, Avraham Stern, Kees Cook, Mordechay Goodstein,
	Haim, Dreyfuss, linux-wireless, netdev

On Fri, 2023-05-05 at 12:52 -1000, Tejun Heo wrote:
> trans_pcie->rba.alloc_wq only hosts a single work item and thus doesn't need
> explicit concurrency limit. Let's use the default @max_active. This doesn't
> cost anything and clearly expresses that @max_active doesn't matter.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Kalle Valo <kvalo@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Gregory Greenman <gregory.greenman@intel.com>
> Cc: Johannes Berg <johannes.berg@intel.com>
> Cc: Avraham Stern <avraham.stern@intel.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mordechay Goodstein <mordechay.goodstein@intel.com>
> Cc: "Haim, Dreyfuss" <haim.dreyfuss@intel.com>
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> 

Sure, that seems fine too.

Acked-by: Johannes Berg <johannes.berg@intel.com>

For whatever that's worth, might better to get Gregory ;-)

johannes

> --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> @@ -3577,7 +3577,7 @@ struct iwl_trans *iwl_trans_pcie_alloc(s
>  	init_waitqueue_head(&trans_pcie->imr_waitq);
>  
>  	trans_pcie->rba.alloc_wq = alloc_workqueue("rb_allocator",
> -						   WQ_HIGHPRI | WQ_UNBOUND, 1);
> +						   WQ_HIGHPRI | WQ_UNBOUND, 0);
>  	if (!trans_pcie->rba.alloc_wq) {
>  		ret = -ENOMEM;
>  		goto out_free_trans;
> 


^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH v2 16/22] btrfs: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-05-06  1:40       ` Wang Yugui
@ 2023-05-08 23:50         ` Tejun Heo
  0 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:50 UTC (permalink / raw)
  To: Wang Yugui
  Cc: jiangshanlai, linux-kernel, kernel-team, Chris Mason,
	Josef Bacik, David Sterba, linux-btrfs

On Sat, May 06, 2023 at 09:40:14AM +0800, Wang Yugui wrote:
> by test, I noticed some warning caused by
> void workqueue_set_max_active(struct workqueue_struct *wq, int max_active)
> 	if (WARN_ON(wq->flags & __WQ_ORDERED_EXPLICIT))
> 		return;
> 
> so I tested again  with the flowing fix
> 
> diff --git a/fs/btrfs/async-thread.c b/fs/btrfs/async-thread.c
> index 43c8995..e4b68e9 100644
> --- a/fs/btrfs/async-thread.c
> +++ b/fs/btrfs/async-thread.c
> @@ -99,8 +99,11 @@ struct btrfs_workqueue *btrfs_alloc_workqueue(struct btrfs_fs_info *fs_info,
>  		ret->thresh = thresh;
>  	}
>  
> -	ret->normal_wq = alloc_workqueue("btrfs-%s", flags, ret->current_active,
> -					 name);
> +	if(limit_active == 1)
> +		ret->normal_wq = alloc_ordered_workqueue("btrfs-%s", flags, name);
> +	else
> +		ret->normal_wq = alloc_workqueue("btrfs-%s", flags,
> +					 ret->current_active, name);
>  	if (!ret->normal_wq) {
>  		kfree(ret);
>  		return NULL;
> @@ -139,7 +139,7 @@ static inline void thresh_exec_hook(struct btrfs_workqueue *wq)
>  	long pending;
>  	int need_change = 0;
>  
> -	if (wq->thresh == NO_THRESHOLD)
> +	if (wq->thresh == NO_THRESHOLD || wq->limit_active == 1)
>  		return;
>  
>  	atomic_dec(&wq->pending);
> 
> we need 'limit_active' at 2nd postition, so I used 'limit_active' and 1st
> postition too.

Oh, that most likely means that these workqueues don't need to and shouldn't
be ordered. Will update the patch.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 01/22] powerpc, workqueue: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50   ` Tejun Heo
@ 2023-05-08 23:55     ` Tejun Heo
  -1 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:55 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Michael Ellerman, Nicholas Piggin,
	Christophe Leroy, Nathan Lynch, linuxppc-dev

Applied to wq/for-6.5-cleanup-ordered.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 01/22] powerpc, workqueue: Use alloc_ordered_workqueue() to create ordered workqueues
@ 2023-05-08 23:55     ` Tejun Heo
  0 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:55 UTC (permalink / raw)
  To: jiangshanlai
  Cc: Nathan Lynch, kernel-team, linux-kernel, Nicholas Piggin, linuxppc-dev

Applied to wq/for-6.5-cleanup-ordered.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 02/22] greybus: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 02/22] greybus: " Tejun Heo
                     ` (2 preceding siblings ...)
  2023-04-21  8:15   ` Alex Elder
@ 2023-05-08 23:55   ` Tejun Heo
  3 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:55 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, greybus-dev

Applied to wq/for-6.5-cleanup-ordered.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 03/22] IB/hfi1: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21 14:02   ` Dennis Dalessandro
@ 2023-05-08 23:56     ` Tejun Heo
  2023-05-09  1:35       ` Tejun Heo
  0 siblings, 1 reply; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:56 UTC (permalink / raw)
  To: Dennis Dalessandro
  Cc: jiangshanlai, linux-kernel, kernel-team, Jason Gunthorpe,
	Leon Romanovsky, linux-rdma

Applied to wq/for-6.5-cleanup-ordered.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 07/22] net: octeontx2: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 07/22] net: octeontx2: " Tejun Heo
  2023-04-21  6:16   ` [EXT] " Sunil Kovvuri Goutham
@ 2023-05-08 23:56   ` Tejun Heo
  1 sibling, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:56 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Sunil Goutham, Ratheesh Kannoth,
	Srujana Challa, Geetha sowjanya, netdev

Applied to wq/for-6.5-cleanup-ordered.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 06/22] net: thunderx: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50   ` Tejun Heo
                     ` (2 preceding siblings ...)
  (?)
@ 2023-05-08 23:57   ` Tejun Heo
  -1 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:57 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Sunil Goutham, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-arm-kernel,
	netdev

Applied to wq/for-6.5-cleanup-ordered.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 12/22] scsi: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 12/22] scsi: " Tejun Heo
  2023-04-21 14:23   ` Benjamin Block
@ 2023-05-08 23:57   ` Tejun Heo
  2023-05-09  1:22     ` Tejun Heo
  1 sibling, 1 reply; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:57 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, James E.J. Bottomley,
	Martin K. Petersen, linux-scsi

Applied to wq/for-6.5-cleanup-ordered.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 17/22] cifs: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21 18:38   ` Paulo Alcantara
@ 2023-05-08 23:58     ` Tejun Heo
  0 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:58 UTC (permalink / raw)
  To: Paulo Alcantara
  Cc: jiangshanlai, linux-kernel, kernel-team, Steve French,
	Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Tom Talpey,
	linux-cifs, samba-technical

On Fri, Apr 21, 2023 at 03:38:57PM -0300, Paulo Alcantara wrote:
...
> >  fs/cifs/dfs_cache.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Acked-by: Paulo Alcantara (SUSE) <pc@manguebit.com>

6be2ea33a409 ("cifs: avoid potential races when handling multiple dfs
tcons") made this patch unnecessary. Dropped.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 13/22] virt: acrn: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 13/22] virt: acrn: " Tejun Heo
  2023-04-21  5:03   ` Greg Kroah-Hartman
@ 2023-05-08 23:58   ` Tejun Heo
  1 sibling, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:58 UTC (permalink / raw)
  To: jiangshanlai; +Cc: linux-kernel, kernel-team, Fei Li, Greg Kroah-Hartman

Applied to wq/for-6.5-cleanup-ordered.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 15/22] xen/pvcalls: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 15/22] xen/pvcalls: " Tejun Heo
  2023-05-08 11:58   ` Juergen Gross
@ 2023-05-08 23:59   ` Tejun Heo
  1 sibling, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:59 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Juergen Gross, Stefano Stabellini,
	Oleksandr Tyshchenko, xen-devel

Applied to wq/for-6.5-cleanup-ordered.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 09/22] wifi: iwlwifi: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-04-21  2:50 ` [PATCH 09/22] wifi: iwlwifi: " Tejun Heo
  2023-04-24 17:31   ` Johannes Berg
@ 2023-05-08 23:59   ` Tejun Heo
  2023-05-09 15:25     ` Tejun Heo
  1 sibling, 1 reply; 70+ messages in thread
From: Tejun Heo @ 2023-05-08 23:59 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Kalle Valo, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Gregory Greenman,
	Johannes Berg, Avraham Stern, Kees Cook, Mordechay Goodstein,
	Haim, Dreyfuss, linux-wireless, netdev

Applied to wq/for-6.5-cleanup-ordered.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 12/22] scsi: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-05-08 23:57   ` Tejun Heo
@ 2023-05-09  1:22     ` Tejun Heo
  0 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-09  1:22 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, James E.J. Bottomley,
	Martin K. Petersen, linux-scsi

On Mon, May 08, 2023 at 01:57:30PM -1000, Tejun Heo wrote:
> Applied to wq/for-6.5-cleanup-ordered.

Oops, strike that. All scsi core workqueues have WQ_SYSFS set which means
that their max_active could be adjusted upwards through sysfs. The shouldn't
be ordered workqueues. This only leaves NCR5380 the only remaining
conversion candidate; however, that one only uses a single work item, so the
better thing to do there is using the default @max_active instead.

I'm dropping this patch and will add a patch for NCR5380 in the next round.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 03/22] IB/hfi1: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-05-08 23:56     ` Tejun Heo
@ 2023-05-09  1:35       ` Tejun Heo
  0 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-09  1:35 UTC (permalink / raw)
  To: Dennis Dalessandro
  Cc: jiangshanlai, linux-kernel, kernel-team, Jason Gunthorpe,
	Leon Romanovsky, linux-rdma

On Mon, May 08, 2023 at 01:56:20PM -1000, Tejun Heo wrote:
> Applied to wq/for-6.5-cleanup-ordered.

This has WQ_SYSFS so can't be an ordered workqueue. Sorry, this is a pattern
I missed. Dropped the patch.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: [PATCH 09/22] wifi: iwlwifi: Use alloc_ordered_workqueue() to create ordered workqueues
  2023-05-08 23:59   ` [PATCH 09/22] wifi: iwlwifi: Use alloc_ordered_workqueue() to create ordered workqueues Tejun Heo
@ 2023-05-09 15:25     ` Tejun Heo
  0 siblings, 0 replies; 70+ messages in thread
From: Tejun Heo @ 2023-05-09 15:25 UTC (permalink / raw)
  To: jiangshanlai
  Cc: linux-kernel, kernel-team, Kalle Valo, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Gregory Greenman,
	Johannes Berg, Avraham Stern, Kees Cook, Mordechay Goodstein,
	Haim, Dreyfuss, linux-wireless, netdev

On Mon, May 08, 2023 at 01:59:26PM -1000, Tejun Heo wrote:
> Applied to wq/for-6.5-cleanup-ordered.

This notification is on the wrong patch. The updated one w/ 0 @max_active
was applied.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 70+ messages in thread

* Re: (subset) [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup
  2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
                   ` (21 preceding siblings ...)
  2023-04-21  2:50 ` [PATCH 22/22] workqueue: Don't implicitly make UNBOUND workqueues w/ @max_active==1 ordered Tejun Heo
@ 2023-05-25  4:54 ` Bjorn Andersson
  22 siblings, 0 replies; 70+ messages in thread
From: Bjorn Andersson @ 2023-05-25  4:54 UTC (permalink / raw)
  To: jiangshanlai, Tejun Heo; +Cc: kernel-team, linux-kernel

On Thu, 20 Apr 2023 16:50:24 -1000, Tejun Heo wrote:
> When multiple work items are queued to a workqueue, their execution order
> doesn't match the queueing order. They may get executed in any order and
> simultaneously. When fully serialized execution - one by one in the queueing
> order - is needed, an ordered workqueue should be used which can be created
> with alloc_ordered_workqueue().
> 
> However, alloc_ordered_workqueue() was a later addition. Before it, an
> ordered workqueue could be obtained by creating an UNBOUND workqueue with
> @max_active==1. This originally was an implementation side-effect which was
> broken by 4c16bd327c74 ("workqueue: restore WQ_UNBOUND/max_active==1 to be
> ordered"). Because there were users that depended on the ordered execution,
> 5c0338c68706 ("workqueue: restore WQ_UNBOUND/max_active==1 to be ordered")
> made workqueue allocation path to implicitly promote UNBOUND workqueues w/
> @max_active==1 to ordered workqueues.
> 
> [...]

Applied, thanks!

[14/22] soc: qcom: qmi: Use alloc_ordered_workqueue() to create ordered workqueues
        commit: 56310520308ab863030e9baa9a8f63bb31c94e27

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

^ permalink raw reply	[flat|nested] 70+ messages in thread

end of thread, other threads:[~2023-05-25  4:57 UTC | newest]

Thread overview: 70+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-21  2:50 [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Tejun Heo
2023-04-21  2:50 ` [PATCH 01/22] powerpc, workqueue: Use alloc_ordered_workqueue() to create ordered workqueues Tejun Heo
2023-04-21  2:50   ` Tejun Heo
2023-04-21  5:20   ` Michael Ellerman
2023-04-21  5:20     ` Michael Ellerman
2023-05-08 23:55   ` Tejun Heo
2023-05-08 23:55     ` Tejun Heo
2023-04-21  2:50 ` [PATCH 02/22] greybus: " Tejun Heo
2023-04-21  5:03   ` Greg Kroah-Hartman
2023-04-21  6:29   ` Johan Hovold
2023-04-21  8:15   ` Alex Elder
2023-05-08 23:55   ` Tejun Heo
2023-04-21  2:50 ` [PATCH 03/22] IB/hfi1: " Tejun Heo
2023-04-21 14:02   ` Dennis Dalessandro
2023-05-08 23:56     ` Tejun Heo
2023-05-09  1:35       ` Tejun Heo
2023-04-21  2:50 ` [PATCH 04/22] dm integrity: " Tejun Heo
2023-04-21  2:50   ` [dm-devel] " Tejun Heo
2023-04-21  2:50 ` [PATCH 05/22] media: amphion: " Tejun Heo
2023-04-21  2:50 ` [PATCH 06/22] net: thunderx: " Tejun Heo
2023-04-21  2:50   ` Tejun Heo
2023-04-21  6:19   ` [EXT] " Sunil Kovvuri Goutham
2023-04-21  6:19     ` Sunil Kovvuri Goutham
2023-04-21 14:01   ` Jakub Kicinski
2023-04-21 14:01     ` Jakub Kicinski
2023-04-21 14:13     ` Tejun Heo
2023-04-21 14:13       ` Tejun Heo
2023-04-21 14:28       ` Jakub Kicinski
2023-04-21 14:28         ` Jakub Kicinski
2023-05-08 23:57   ` Tejun Heo
2023-04-21  2:50 ` [PATCH 07/22] net: octeontx2: " Tejun Heo
2023-04-21  6:16   ` [EXT] " Sunil Kovvuri Goutham
2023-05-08 23:56   ` Tejun Heo
2023-04-21  2:50 ` [PATCH 08/22] wifi: ath10/11/12k: " Tejun Heo
2023-04-21  2:50 ` [PATCH 09/22] wifi: iwlwifi: " Tejun Heo
2023-04-24 17:31   ` Johannes Berg
2023-05-05 22:52     ` [PATCH] wifi: iwlwifi: Use default @max_active for trans_pcie->rba.alloc_wq Tejun Heo
2023-05-08 16:05       ` Johannes Berg
2023-05-08 23:59   ` [PATCH 09/22] wifi: iwlwifi: Use alloc_ordered_workqueue() to create ordered workqueues Tejun Heo
2023-05-09 15:25     ` Tejun Heo
2023-04-21  2:50 ` [PATCH 10/22] wifi: mwifiex: " Tejun Heo
2023-04-25 18:14   ` Brian Norris
2023-05-05 22:53     ` [PATCH] wifi: mwifiex: Use default @max_active for workqueues Tejun Heo
2023-04-21  2:50 ` [PATCH 11/22] net: wwan: t7xx: Use alloc_ordered_workqueue() to create ordered workqueues Tejun Heo
2023-04-21  2:50 ` [PATCH 12/22] scsi: " Tejun Heo
2023-04-21 14:23   ` Benjamin Block
2023-05-08 23:57   ` Tejun Heo
2023-05-09  1:22     ` Tejun Heo
2023-04-21  2:50 ` [PATCH 13/22] virt: acrn: " Tejun Heo
2023-04-21  5:03   ` Greg Kroah-Hartman
2023-05-08 23:58   ` Tejun Heo
2023-04-21  2:50 ` [PATCH 14/22] soc: qcom: qmi: " Tejun Heo
2023-04-21  2:50 ` [PATCH 15/22] xen/pvcalls: " Tejun Heo
2023-05-08 11:58   ` Juergen Gross
2023-05-08 23:59   ` Tejun Heo
2023-04-21  2:50 ` [PATCH 16/22] btrfs: " Tejun Heo
2023-04-30  4:40   ` Wang Yugui
2023-05-05 22:58     ` [PATCH v2 " Tejun Heo
2023-05-06  1:40       ` Wang Yugui
2023-05-08 23:50         ` Tejun Heo
2023-04-21  2:50 ` [PATCH 17/22] cifs: " Tejun Heo
2023-04-21 18:38   ` Paulo Alcantara
2023-05-08 23:58     ` Tejun Heo
2023-04-21  2:50 ` [PATCH 18/22] net: qrtr: " Tejun Heo
2023-04-21  2:50 ` [PATCH 19/22] rxrpc: " Tejun Heo
2023-04-21  2:50 ` [PATCH 20/22] crypto: octeontx2: " Tejun Heo
2023-04-21  2:50 ` [PATCH 21/22] media: coda: " Tejun Heo
2023-04-21  2:50 ` [PATCH 22/22] workqueue: Don't implicitly make UNBOUND workqueues w/ @max_active==1 ordered Tejun Heo
2023-04-24  5:03   ` Christoph Hellwig
2023-05-25  4:54 ` (subset) [PATCHSET wq/for-6.5] workqueue: Ordered workqueue creation cleanup Bjorn Andersson

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.