[PATCH 1/2 wq/for-6.9] workqueue: Implement workqueue_set_min_active()

[PATCH 1/2 wq/for-6.9] workqueue: Implement workqueue_set_min_active()

[Tejun Heo]

Since 5797b1c18919 ("workqueue: Implement system-wide nr_active enforcement
for unbound workqueues"), unbound workqueues have separate min_active which
sets the number of interdependent work items that can be handled. This value
is currently initialized to WQ_DFL_MIN_ACTIVE which is 8. This isn't high
enough for some users, let's add an interface to adjust the setting.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 include/linux/workqueue.h |    2 ++
 kernel/workqueue.c        |   27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -553,6 +553,8 @@ extern bool flush_rcu_work(struct rcu_wo
 
 extern void workqueue_set_max_active(struct workqueue_struct *wq,
 				     int max_active);
+extern void workqueue_set_min_active(struct workqueue_struct *wq,
+				     int min_active);
 extern struct work_struct *current_work(void);
 extern bool current_is_workqueue_rescuer(void);
 extern bool workqueue_congested(int cpu, struct workqueue_struct *wq);
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -5627,6 +5627,33 @@ void workqueue_set_max_active(struct wor
 EXPORT_SYMBOL_GPL(workqueue_set_max_active);
 
 /**
+ * workqueue_set_min_active - adjust min_active of an unbound workqueue
+ * @wq: target unbound workqueue
+ * @min_active: new min_active value
+ *
+ * Set min_active of an unbound workqueue. Unlike other types of workqueues, an
+ * unbound workqueue is not guaranteed to be able to process max_active
+ * interdependent work items. Instead, an unbound workqueue is guaranteed to be
+ * able to process min_active number of interdependent work items which is
+ * %WQ_DFL_MIN_ACTIVE by default.
+ *
+ * Use this function to adjust the min_active value between 0 and the current
+ * max_active.
+ */
+void workqueue_set_min_active(struct workqueue_struct *wq, int min_active)
+{
+	/* min_active is only meaningful for non-ordered unbound workqueues */
+	if (WARN_ON((wq->flags & (WQ_BH | WQ_UNBOUND | __WQ_ORDERED)) !=
+		    WQ_UNBOUND))
+		return;
+
+	mutex_lock(&wq->mutex);
+	wq->saved_min_active = clamp(min_active, 0, wq->saved_max_active);
+	wq_adjust_max_active(wq);
+	mutex_unlock(&wq->mutex);
+}
+
+/**
  * current_work - retrieve %current task's work struct
  *
  * Determine if %current task is a workqueue worker and what it's working on.

[PATCH 2/2 wq/for-6.9] async: Use a dedicated unbound workqueue with raised min_active

[Tejun Heo]

Async can schedule a number of interdependent work items. However, since
5797b1c18919 ("workqueue: Implement system-wide nr_active enforcement for
unbound workqueues"), unbound workqueues have separate min_active which sets
the number of interdependent work items that can be handled. This default
value is 8 which isn't sufficient for async and can lead to stalls during
resume from suspend in some cases.

Let's use a dedicated unbound workqueue with raised min_active.

Signed-off-by: Tejun Heo <tj@kernel.org>
Link: http://lkml.kernel.org/r/708a65cc-79ec-44a6-8454-a93d0f3114c3@samsung.com
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
---
Hello,

Marek, can you please see whether this solves the hang while resuming? If it
does and Rafael is okay with it, I'll route this patch through wq/for-6.9.

Thanks.

 include/linux/async.h |    1 +
 init/main.c           |    1 +
 kernel/async.c        |   17 ++++++++++++++++-
 3 files changed, 18 insertions(+), 1 deletion(-)

--- a/include/linux/async.h
+++ b/include/linux/async.h
@@ -120,4 +120,5 @@ extern void async_synchronize_cookie(asy
 extern void async_synchronize_cookie_domain(async_cookie_t cookie,
 					    struct async_domain *domain);
 extern bool current_is_async(void);
+extern void async_init(void);
 #endif
--- a/init/main.c
+++ b/init/main.c
@@ -1545,6 +1545,7 @@ static noinline void __init kernel_init_
 	sched_init_smp();
 
 	workqueue_init_topology();
+	async_init();
 	padata_init();
 	page_alloc_init_late();
 
--- a/kernel/async.c
+++ b/kernel/async.c
@@ -64,6 +64,7 @@ static async_cookie_t next_cookie = 1;
 static LIST_HEAD(async_global_pending);	/* pending from all registered doms */
 static ASYNC_DOMAIN(async_dfl_domain);
 static DEFINE_SPINLOCK(async_lock);
+static struct workqueue_struct *async_wq;
 
 struct async_entry {
 	struct list_head	domain_list;
@@ -174,7 +175,7 @@ static async_cookie_t __async_schedule_n
 	spin_unlock_irqrestore(&async_lock, flags);
 
 	/* schedule for execution */
-	queue_work_node(node, system_unbound_wq, &entry->work);
+	queue_work_node(node, async_wq, &entry->work);
 
 	return newcookie;
 }
@@ -345,3 +346,17 @@ bool current_is_async(void)
 	return worker && worker->current_func == async_run_entry_fn;
 }
 EXPORT_SYMBOL_GPL(current_is_async);
+
+void __init async_init(void)
+{
+	/*
+	 * Async can schedule a number of interdependent work items. However,
+	 * unbound workqueues can handle only upto min_active interdependent
+	 * work items. The default min_active of 8 isn't sufficient for async
+	 * and can lead to stalls. Let's use a dedicated workqueue with raised
+	 * min_active.
+	 */
+	async_wq = alloc_workqueue("async", WQ_UNBOUND, 0);
+	BUG_ON(!async_wq);
+	workqueue_set_min_active(async_wq, WQ_DFL_ACTIVE);
+}

Re: [PATCH 2/2 wq/for-6.9] async: Use a dedicated unbound workqueue with raised min_active

[Marek Szyprowski]

On 09.02.2024 01:14, Tejun Heo wrote:
> Async can schedule a number of interdependent work items. However, since
> 5797b1c18919 ("workqueue: Implement system-wide nr_active enforcement for
> unbound workqueues"), unbound workqueues have separate min_active which sets
> the number of interdependent work items that can be handled. This default
> value is 8 which isn't sufficient for async and can lead to stalls during
> resume from suspend in some cases.
>
> Let's use a dedicated unbound workqueue with raised min_active.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Link: http://lkml.kernel.org/r/708a65cc-79ec-44a6-8454-a93d0f3114c3@samsung.com
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> ---
> Hello,
>
> Marek, can you please see whether this solves the hang while resuming? If it
> does and Rafael is okay with it, I'll route this patch through wq/for-6.9.

Works fine here and fixes the suspend/resume issue.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>


> Thanks.
>
>   include/linux/async.h |    1 +
>   init/main.c           |    1 +
>   kernel/async.c        |   17 ++++++++++++++++-
>   3 files changed, 18 insertions(+), 1 deletion(-)
>
> --- a/include/linux/async.h
> +++ b/include/linux/async.h
> @@ -120,4 +120,5 @@ extern void async_synchronize_cookie(asy
>   extern void async_synchronize_cookie_domain(async_cookie_t cookie,
>   					    struct async_domain *domain);
>   extern bool current_is_async(void);
> +extern void async_init(void);
>   #endif
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1545,6 +1545,7 @@ static noinline void __init kernel_init_
>   	sched_init_smp();
>   
>   	workqueue_init_topology();
> +	async_init();
>   	padata_init();
>   	page_alloc_init_late();
>   
> --- a/kernel/async.c
> +++ b/kernel/async.c
> @@ -64,6 +64,7 @@ static async_cookie_t next_cookie = 1;
>   static LIST_HEAD(async_global_pending);	/* pending from all registered doms */
>   static ASYNC_DOMAIN(async_dfl_domain);
>   static DEFINE_SPINLOCK(async_lock);
> +static struct workqueue_struct *async_wq;
>   
>   struct async_entry {
>   	struct list_head	domain_list;
> @@ -174,7 +175,7 @@ static async_cookie_t __async_schedule_n
>   	spin_unlock_irqrestore(&async_lock, flags);
>   
>   	/* schedule for execution */
> -	queue_work_node(node, system_unbound_wq, &entry->work);
> +	queue_work_node(node, async_wq, &entry->work);
>   
>   	return newcookie;
>   }
> @@ -345,3 +346,17 @@ bool current_is_async(void)
>   	return worker && worker->current_func == async_run_entry_fn;
>   }
>   EXPORT_SYMBOL_GPL(current_is_async);
> +
> +void __init async_init(void)
> +{
> +	/*
> +	 * Async can schedule a number of interdependent work items. However,
> +	 * unbound workqueues can handle only upto min_active interdependent
> +	 * work items. The default min_active of 8 isn't sufficient for async
> +	 * and can lead to stalls. Let's use a dedicated workqueue with raised
> +	 * min_active.
> +	 */
> +	async_wq = alloc_workqueue("async", WQ_UNBOUND, 0);
> +	BUG_ON(!async_wq);
> +	workqueue_set_min_active(async_wq, WQ_DFL_ACTIVE);
> +}
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 2/2 wq/for-6.9] async: Use a dedicated unbound workqueue with raised min_active

[Tejun Heo]

On Thu, Feb 08, 2024 at 02:14:16PM -1000, Tejun Heo wrote:
> Async can schedule a number of interdependent work items. However, since
> 5797b1c18919 ("workqueue: Implement system-wide nr_active enforcement for
> unbound workqueues"), unbound workqueues have separate min_active which sets
> the number of interdependent work items that can be handled. This default
> value is 8 which isn't sufficient for async and can lead to stalls during
> resume from suspend in some cases.
> 
> Let's use a dedicated unbound workqueue with raised min_active.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Link: http://lkml.kernel.org/r/708a65cc-79ec-44a6-8454-a93d0f3114c3@samsung.com
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>

Applying to wq/for-6.9. Rafael, if you have any objections, please holler.

Thanks.

-- 
tejun