mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] wifi: add WQ_UNBOUND to alloc_workqueue users and rename system_unbound_wq
@ 2025-09-26  8:38 Marco Crivellari
  2025-09-26  8:38 ` [PATCH 1/2] wifi: replace use of system_unbound_wq with system_dfl_wq Marco Crivellari
  2025-09-26  8:38 ` [PATCH 2/2] wifi: WQ_PERCPU added to alloc_workqueue users Marco Crivellari
  0 siblings, 2 replies; 5+ messages in thread
From: Marco Crivellari @ 2025-09-26  8:38 UTC (permalink / raw)
  To: linux-kernel, linux-wireless
  Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
	Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
	Johannes Berg


Hi!

Below is a summary of a discussion about the Workqueue API and cpu isolation
considerations. Details and more information are available here:

        "workqueue: Always use wq_select_unbound_cpu() for WORK_CPU_UNBOUND."
        https://lore.kernel.org/all/20250221112003.1dSuoGyc@linutronix.de/

=== Current situation: problems ===

Let's consider a nohz_full system with isolated CPUs: wq_unbound_cpumask is
set to the housekeeping CPUs, for !WQ_UNBOUND the local CPU is selected.

This leads to different scenarios if a work item is scheduled on an isolated
CPU where "delay" value is 0 or greater then 0:
        schedule_delayed_work(, 0);

This will be handled by __queue_work() that will queue the work item on the
current local (isolated) CPU, while:

        schedule_delayed_work(, 1);

Will move the timer on an housekeeping CPU, and schedule the work there.

Currently if a user enqueue a work item using schedule_delayed_work() the
used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
schedule_work() that is using system_wq and queue_work(), that makes use
again of WORK_CPU_UNBOUND.

This lack of consistentcy cannot be addressed without refactoring the API.

=== Plan and future plans ===

This patchset is the first stone on a refactoring needed in order to
address the points aforementioned; it will have a positive impact also
on the cpu isolation, in the long term, moving away percpu workqueue in
favor to an unbound model.

These are the main steps:
1)  API refactoring (that this patch is introducing)
    -   Make more clear and uniform the system wq names, both per-cpu and
        unbound. This to avoid any possible confusion on what should be
        used.

    -   Introduction of WQ_PERCPU: this flag is the complement of WQ_UNBOUND,
        introduced in this patchset and used on all the callers that are not
        currently using WQ_UNBOUND.

        WQ_UNBOUND will be removed in a future release cycle.

        Most users don't need to be per-cpu, because they don't have
        locality requirements, because of that, a next future step will be
        make "unbound" the default behavior.

2)  Check who really needs to be per-cpu
    -   Remove the WQ_PERCPU flag when is not strictly required.

3)  Add a new API (prefer local cpu)
    -   There are users that don't require a local execution, like mentioned
        above; despite that, local execution yeld to performance gain.

        This new API will prefer the local execution, without requiring it.

=== Introduced Changes by this series ===

1)	[P 1] replace system_unbound_wq with system_dfl_workqueue
		system_unbound_wq is to be used when locality is not required.
		This should be the default choice. Make this clear renaming the
		workqueue in system_dfl_wq.

2)	[P 2] WQ_PERCPU added to alloc_workqueue() users
		Every alloc_workqueue() users should specify one among WQ_PERCPU
		or WQ_UNBOUND.
		
		Make this change ensuring that every alloc_workqueue() without
		WQ_UNBOUND now make use of WQ_PERCPU.


Thanks! 


Marco Crivellari (2):
  wifi: replace use of system_unbound_wq with system_dfl_wq
  wifi: WQ_PERCPU added to alloc_workqueue users

 drivers/net/wireless/ath/ath6kl/usb.c         | 2 +-
 drivers/net/wireless/quantenna/qtnfmac/core.c | 3 ++-
 drivers/net/wireless/realtek/rtlwifi/base.c   | 2 +-
 drivers/net/wireless/realtek/rtw88/usb.c      | 3 ++-
 drivers/net/wireless/silabs/wfx/main.c        | 2 +-
 drivers/net/wireless/st/cw1200/bh.c           | 4 ++--
 net/wireless/core.c                           | 4 ++--
 net/wireless/sysfs.c                          | 2 +-
 8 files changed, 12 insertions(+), 10 deletions(-)

-- 
2.51.0


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

* [PATCH 1/2] wifi: replace use of system_unbound_wq with system_dfl_wq
  2025-09-26  8:38 [PATCH 0/2] wifi: add WQ_UNBOUND to alloc_workqueue users and rename system_unbound_wq Marco Crivellari
@ 2025-09-26  8:38 ` Marco Crivellari
  2025-09-26  8:38 ` [PATCH 2/2] wifi: WQ_PERCPU added to alloc_workqueue users Marco Crivellari
  1 sibling, 0 replies; 5+ messages in thread
From: Marco Crivellari @ 2025-09-26  8:38 UTC (permalink / raw)
  To: linux-kernel, linux-wireless
  Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
	Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
	Johannes Berg

Currently if a user enqueue a work item using schedule_delayed_work() the
used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
schedule_work() that is using system_wq and queue_work(), that makes use
again of WORK_CPU_UNBOUND.

This lack of consistentcy cannot be addressed without refactoring the API.

system_unbound_wq should be the default workqueue so as not to enforce
locality constraints for random work whenever it's not required.

Adding system_dfl_wq to encourage its use when unbound work should be used.

The old system_unbound_wq will be kept for a few release cycles.

Suggested-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
---
 net/wireless/core.c  | 4 ++--
 net/wireless/sysfs.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 797f9f2004a6..93b8e18ca630 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -428,7 +428,7 @@ static void cfg80211_wiphy_work(struct work_struct *work)
 	if (wk) {
 		list_del_init(&wk->entry);
 		if (!list_empty(&rdev->wiphy_work_list))
-			queue_work(system_unbound_wq, work);
+			queue_work(system_dfl_wq, work);
 		spin_unlock_irq(&rdev->wiphy_work_lock);
 
 		trace_wiphy_work_run(&rdev->wiphy, wk);
@@ -1698,7 +1698,7 @@ void wiphy_work_queue(struct wiphy *wiphy, struct wiphy_work *work)
 		list_add_tail(&work->entry, &rdev->wiphy_work_list);
 	spin_unlock_irqrestore(&rdev->wiphy_work_lock, flags);
 
-	queue_work(system_unbound_wq, &rdev->wiphy_work);
+	queue_work(system_dfl_wq, &rdev->wiphy_work);
 }
 EXPORT_SYMBOL_GPL(wiphy_work_queue);
 
diff --git a/net/wireless/sysfs.c b/net/wireless/sysfs.c
index 62f26618f674..8d142856e385 100644
--- a/net/wireless/sysfs.c
+++ b/net/wireless/sysfs.c
@@ -137,7 +137,7 @@ static int wiphy_resume(struct device *dev)
 	if (rdev->wiphy.registered && rdev->ops->resume)
 		ret = rdev_resume(rdev);
 	rdev->suspended = false;
-	queue_work(system_unbound_wq, &rdev->wiphy_work);
+	queue_work(system_dfl_wq, &rdev->wiphy_work);
 	wiphy_unlock(&rdev->wiphy);
 
 	if (ret)
-- 
2.51.0


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

* [PATCH 2/2] wifi: WQ_PERCPU added to alloc_workqueue users
  2025-09-26  8:38 [PATCH 0/2] wifi: add WQ_UNBOUND to alloc_workqueue users and rename system_unbound_wq Marco Crivellari
  2025-09-26  8:38 ` [PATCH 1/2] wifi: replace use of system_unbound_wq with system_dfl_wq Marco Crivellari
@ 2025-09-26  8:38 ` Marco Crivellari
  2025-10-20 14:21   ` Johannes Berg
  1 sibling, 1 reply; 5+ messages in thread
From: Marco Crivellari @ 2025-09-26  8:38 UTC (permalink / raw)
  To: linux-kernel, linux-wireless
  Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
	Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
	Johannes Berg

Currently if a user enqueue a work item using schedule_delayed_work() the
used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
schedule_work() that is using system_wq and queue_work(), that makes use
again of WORK_CPU_UNBOUND.
This lack of consistentcy cannot be addressed without refactoring the API.

alloc_workqueue() treats all queues as per-CPU by default, while unbound
workqueues must opt-in via WQ_UNBOUND.

This default is suboptimal: most workloads benefit from unbound queues,
allowing the scheduler to place worker threads where they’re needed and
reducing noise when CPUs are isolated.

This patch adds a new WQ_PERCPU flag, to explicitly request the use of
the per-CPU behavior. Both flags coexist for one release cycle to allow
callers to transition their calls.

Once migration is complete, WQ_UNBOUND can be removed and unbound will
become the implicit default.

With the introduction of the WQ_PERCPU flag (equivalent to !WQ_UNBOUND),
any alloc_workqueue() caller that doesn’t explicitly specify WQ_UNBOUND
must now use WQ_PERCPU.

All existing users have been updated accordingly.

Suggested-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
---
 drivers/net/wireless/ath/ath6kl/usb.c         | 2 +-
 drivers/net/wireless/quantenna/qtnfmac/core.c | 3 ++-
 drivers/net/wireless/realtek/rtlwifi/base.c   | 2 +-
 drivers/net/wireless/realtek/rtw88/usb.c      | 3 ++-
 drivers/net/wireless/silabs/wfx/main.c        | 2 +-
 drivers/net/wireless/st/cw1200/bh.c           | 4 ++--
 6 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/usb.c b/drivers/net/wireless/ath/ath6kl/usb.c
index 38bb501fc553..bfb21725d779 100644
--- a/drivers/net/wireless/ath/ath6kl/usb.c
+++ b/drivers/net/wireless/ath/ath6kl/usb.c
@@ -637,7 +637,7 @@ static struct ath6kl_usb *ath6kl_usb_create(struct usb_interface *interface)
 	ar_usb = kzalloc(sizeof(struct ath6kl_usb), GFP_KERNEL);
 	if (ar_usb == NULL)
 		return NULL;
-	ar_usb->wq = alloc_workqueue("ath6kl_wq", 0, 0);
+	ar_usb->wq = alloc_workqueue("ath6kl_wq", WQ_PERCPU, 0);
 	if (!ar_usb->wq) {
 		kfree(ar_usb);
 		return NULL;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c b/drivers/net/wireless/quantenna/qtnfmac/core.c
index 825b05dd3271..38af6cdc2843 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.c
@@ -714,7 +714,8 @@ int qtnf_core_attach(struct qtnf_bus *bus)
 		goto error;
 	}
 
-	bus->hprio_workqueue = alloc_workqueue("QTNF_HPRI", WQ_HIGHPRI, 0);
+	bus->hprio_workqueue = alloc_workqueue("QTNF_HPRI",
+					       WQ_HIGHPRI | WQ_PERCPU, 0);
 	if (!bus->hprio_workqueue) {
 		pr_err("failed to alloc high prio workqueue\n");
 		ret = -ENOMEM;
diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c b/drivers/net/wireless/realtek/rtlwifi/base.c
index e26feb8de658..2786e4ee67eb 100644
--- a/drivers/net/wireless/realtek/rtlwifi/base.c
+++ b/drivers/net/wireless/realtek/rtlwifi/base.c
@@ -445,7 +445,7 @@ static int _rtl_init_deferred_work(struct ieee80211_hw *hw)
 	struct rtl_priv *rtlpriv = rtl_priv(hw);
 	struct workqueue_struct *wq;
 
-	wq = alloc_workqueue("%s", 0, 0, rtlpriv->cfg->name);
+	wq = alloc_workqueue("%s", WQ_PERCPU, 0, rtlpriv->cfg->name);
 	if (!wq)
 		return -ENOMEM;
 
diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 3b5126ffc81a..db60e142268d 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -965,7 +965,8 @@ static int rtw_usb_init_rx(struct rtw_dev *rtwdev)
 	struct sk_buff *rx_skb;
 	int i;
 
-	rtwusb->rxwq = alloc_workqueue("rtw88_usb: rx wq", WQ_BH, 0);
+	rtwusb->rxwq = alloc_workqueue("rtw88_usb: rx wq", WQ_BH | WQ_PERCPU,
+				       0);
 	if (!rtwusb->rxwq) {
 		rtw_err(rtwdev, "failed to create RX work queue\n");
 		return -ENOMEM;
diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
index a61128debbad..dda36e41eed1 100644
--- a/drivers/net/wireless/silabs/wfx/main.c
+++ b/drivers/net/wireless/silabs/wfx/main.c
@@ -364,7 +364,7 @@ int wfx_probe(struct wfx_dev *wdev)
 	wdev->pdata.gpio_wakeup = NULL;
 	wdev->poll_irq = true;
 
-	wdev->bh_wq = alloc_workqueue("wfx_bh_wq", WQ_HIGHPRI, 0);
+	wdev->bh_wq = alloc_workqueue("wfx_bh_wq", WQ_HIGHPRI | WQ_PERCPU, 0);
 	if (!wdev->bh_wq)
 		return -ENOMEM;
 
diff --git a/drivers/net/wireless/st/cw1200/bh.c b/drivers/net/wireless/st/cw1200/bh.c
index 3b4ded2ac801..3f07f4e1deee 100644
--- a/drivers/net/wireless/st/cw1200/bh.c
+++ b/drivers/net/wireless/st/cw1200/bh.c
@@ -54,8 +54,8 @@ int cw1200_register_bh(struct cw1200_common *priv)
 	int err = 0;
 	/* Realtime workqueue */
 	priv->bh_workqueue = alloc_workqueue("cw1200_bh",
-				WQ_MEM_RECLAIM | WQ_HIGHPRI
-				| WQ_CPU_INTENSIVE, 1);
+				WQ_MEM_RECLAIM | WQ_HIGHPRI | WQ_CPU_INTENSIVE | WQ_PERCPU,
+				1);
 
 	if (!priv->bh_workqueue)
 		return -ENOMEM;
-- 
2.51.0


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

* Re: [PATCH 2/2] wifi: WQ_PERCPU added to alloc_workqueue users
  2025-09-26  8:38 ` [PATCH 2/2] wifi: WQ_PERCPU added to alloc_workqueue users Marco Crivellari
@ 2025-10-20 14:21   ` Johannes Berg
  2025-10-21  8:25     ` Marco Crivellari
  0 siblings, 1 reply; 5+ messages in thread
From: Johannes Berg @ 2025-10-20 14:21 UTC (permalink / raw)
  To: Marco Crivellari, linux-kernel, linux-wireless
  Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
	Sebastian Andrzej Siewior, Michal Hocko

Hi,

So I don't know if I really need to tell you this, but generally the
subject should be _imperative_, not describing what you did after the
fact (i.e. "add WQ_PERCPU to ..." rather than "added ...")

> All existing users have been updated accordingly.

Surely this is not _all_ existing users? :)

> Suggested-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
> ---
>  drivers/net/wireless/ath/ath6kl/usb.c         | 2 +-
>  drivers/net/wireless/quantenna/qtnfmac/core.c | 3 ++-
>  drivers/net/wireless/realtek/rtlwifi/base.c   | 2 +-
>  drivers/net/wireless/realtek/rtw88/usb.c      | 3 ++-
>  drivers/net/wireless/silabs/wfx/main.c        | 2 +-
>  drivers/net/wireless/st/cw1200/bh.c           | 4 ++--

These have different maintainers, please split up accordingly.

>  6 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath6kl/usb.c b/drivers/net/wireless/ath/ath6kl/usb.c
> index 38bb501fc553..bfb21725d779 100644
> --- a/drivers/net/wireless/ath/ath6kl/usb.c
> +++ b/drivers/net/wireless/ath/ath6kl/usb.c
> @@ -637,7 +637,7 @@ static struct ath6kl_usb *ath6kl_usb_create(struct usb_interface *interface)
>  	ar_usb = kzalloc(sizeof(struct ath6kl_usb), GFP_KERNEL);
>  	if (ar_usb == NULL)
>  		return NULL;
> -	ar_usb->wq = alloc_workqueue("ath6kl_wq", 0, 0);
> +	ar_usb->wq = alloc_workqueue("ath6kl_wq", WQ_PERCPU, 0);
>  	if (!ar_usb->wq) {
>  		kfree(ar_usb);

I'd also think that WQ_PERCPU doesn't actually make sense for any of
these instances, but for those that still have an active maintainer I'll
defer to them, of course.


(and patch 1 should be prefixed with "wifi: cfg80211:" I'd think)

johannes

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

* Re: [PATCH 2/2] wifi: WQ_PERCPU added to alloc_workqueue users
  2025-10-20 14:21   ` Johannes Berg
@ 2025-10-21  8:25     ` Marco Crivellari
  0 siblings, 0 replies; 5+ messages in thread
From: Marco Crivellari @ 2025-10-21  8:25 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-kernel, linux-wireless, Tejun Heo, Lai Jiangshan,
	Frederic Weisbecker, Sebastian Andrzej Siewior, Michal Hocko

Hi Johannes,

On Mon, Oct 20, 2025 at 4:21 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> Hi,
>
> So I don't know if I really need to tell you this, but generally the
> subject should be _imperative_, not describing what you did after the
> fact (i.e. "add WQ_PERCPU to ..." rather than "added ...")

Aha, thanks for the hint.

> > All existing users have been updated accordingly.
>
> Surely this is not _all_ existing users? :)

Yeah :-)

> > Suggested-by: Tejun Heo <tj@kernel.org>
> > Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
> > ---
> >  drivers/net/wireless/ath/ath6kl/usb.c         | 2 +-
> >  drivers/net/wireless/quantenna/qtnfmac/core.c | 3 ++-
> >  drivers/net/wireless/realtek/rtlwifi/base.c   | 2 +-
> >  drivers/net/wireless/realtek/rtw88/usb.c      | 3 ++-
> >  drivers/net/wireless/silabs/wfx/main.c        | 2 +-
> >  drivers/net/wireless/st/cw1200/bh.c           | 4 ++--
>
> These have different maintainers, please split up accordingly.

Sure, I will do it, like I did for the others.

> >  6 files changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath6kl/usb.c b/drivers/net/wireless/ath/ath6kl/usb.c
> > index 38bb501fc553..bfb21725d779 100644
> > --- a/drivers/net/wireless/ath/ath6kl/usb.c
> > +++ b/drivers/net/wireless/ath/ath6kl/usb.c
> > @@ -637,7 +637,7 @@ static struct ath6kl_usb *ath6kl_usb_create(struct usb_interface *interface)
> >       ar_usb = kzalloc(sizeof(struct ath6kl_usb), GFP_KERNEL);
> >       if (ar_usb == NULL)
> >               return NULL;
> > -     ar_usb->wq = alloc_workqueue("ath6kl_wq", 0, 0);
> > +     ar_usb->wq = alloc_workqueue("ath6kl_wq", WQ_PERCPU, 0);
> >       if (!ar_usb->wq) {
> >               kfree(ar_usb);
>
> I'd also think that WQ_PERCPU doesn't actually make sense for any of
> these instances, but for those that still have an active maintainer I'll
> defer to them, of course.

Thanks for the pointer!

-- 

Marco Crivellari

L3 Support Engineer, Technology & Product

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

end of thread, other threads:[~2025-10-21  8:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-26  8:38 [PATCH 0/2] wifi: add WQ_UNBOUND to alloc_workqueue users and rename system_unbound_wq Marco Crivellari
2025-09-26  8:38 ` [PATCH 1/2] wifi: replace use of system_unbound_wq with system_dfl_wq Marco Crivellari
2025-09-26  8:38 ` [PATCH 2/2] wifi: WQ_PERCPU added to alloc_workqueue users Marco Crivellari
2025-10-20 14:21   ` Johannes Berg
2025-10-21  8:25     ` Marco Crivellari

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®