Re: [PATCH v5 2/2] remoteproc: core: change to ordered workqueue for crash handler

[Bjorn Andersson <andersson@kernel.org>]

On Fri, Dec 02, 2022 at 05:45:32PM +0800, Maria Yu wrote:
> Only the first detected crash needed to be handled, so change
> to ordered workqueue to avoid unnecessary multi active work at
> the same time.

In cab8300b5621 ("remoteproc: Use unbounded workqueue for recovery
work") Mukesh specifically said that it was required that multiple
remoteproc instances should be allowed to recover concurrently.

Is this no longer the case? Or am I perhaps misunderstanding the
nuances of the different work queue models?

> This will reduce the pm_relax unnecessary concurrency.

I'm not sure I understand this sentence, unless I remove the word
"pm_relax", was it added by mistake?


If so, is the support for concurrent recovery really unnecessary?

I know we have cases where we spend time in the recovery process just
waiting for things to happen, so allowing recovery to run concurrent
between instances sounds like a good idea.

Regards,
Bjorn

> 
> Signed-off-by: Maria Yu <quic_aiquny@quicinc.com>
> ---
>  drivers/remoteproc/remoteproc_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index c2d0af048c69..4b973eea10bb 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -2728,8 +2728,8 @@ static void __exit rproc_exit_panic(void)
>  
>  static int __init remoteproc_init(void)
>  {
> -	rproc_recovery_wq = alloc_workqueue("rproc_recovery_wq",
> -						WQ_UNBOUND | WQ_FREEZABLE, 0);
> +	rproc_recovery_wq = alloc_ordered_workqueue("rproc_recovery_wq",
> +						WQ_FREEZABLE, 0);
>  	if (!rproc_recovery_wq) {
>  		pr_err("remoteproc: creation of rproc_recovery_wq failed\n");
>  		return -ENOMEM;
> -- 
> 2.17.1
>