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@xxxxxxxxxx>
> Link: http://lkml.kernel.org/r/708a65cc-79ec-44a6-8454-a93d0f3114c3@xxxxxxxxxxx
> Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> Cc: Rafael J. Wysocki <rjw@xxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxx>


> 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