Received-SPF: pass (google.com: domain of linux-kernel+bounces-59106-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <e4945d3d-f5a4-46c6-ab1e-937e19721e1f@samsung.com>
Date: Fri, 9 Feb 2024 09:15:01 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/2 wq/for-6.9] async: Use a dedicated unbound workqueue
 with raised min_active
Content-Language: en-US
To: Tejun Heo <tj@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>, linux-kernel@vger.kernel.org,
	Lai Jiangshan <jiangshanlai@gmail.com>
From: Marek Szyprowski <m.szyprowski@samsung.com>
In-Reply-To: <ZcVuWLKN8OHRQuRx@slm.duckdns.org>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <ZcVtzJvJCRV5OLM-@slm.duckdns.org>
	<CGME20240209001422eucas1p252a0c403258d107c653c7f2bfbe49f4c@eucas1p2.samsung.com>
	<ZcVuWLKN8OHRQuRx@slm.duckdns.org>

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