diff mbox series

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

Message ID ZcVuWLKN8OHRQuRx@slm.duckdns.org
State New
Headers
Series [1/2,wq/for-6.9] workqueue: Implement workqueue_set_min_active() |

Commit Message

Tejun Heo Feb. 9, 2024, 12:14 a.m. UTC 
  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(-)

Comments

Marek Szyprowski Feb. 9, 2024, 8:15 a.m. UTC | #1 
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
Tejun Heo Feb. 9, 2024, 9:12 p.m. UTC | #2 
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.
diff mbox series

Patch

--- 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);
+}