Re: [PATCH 3/3] ib_srp: Fix a deadlock

[Leon Romanovsky <leonro@mellanox.com>]

On Tue, Feb 15, 2022 at 10:26:50AM -0800, Bart Van Assche wrote:
> Wait on tl_err_work instead of flushing system_long_wq since flushing
> system_long_wq is deadlock-prone.
> 
> Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
> Fixes: ef6c49d87c34 ("IB/srp: Eliminate state SRP_TARGET_DEAD")
> Reported-by: syzbot+831661966588c802aae9@syzkaller.appspotmail.com
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/infiniband/ulp/srp/ib_srp.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> index 2db7429b42e1..8e1561a6d325 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -4044,12 +4044,10 @@ static void srp_remove_one(struct ib_device *device, void *client_data)
>  		mutex_lock(&host->target_mutex);
>  		list_for_each_entry(target, &host->target_list, list)
>  			srp_queue_remove_work(target);
> +		list_for_each_entry(target, &host->target_list, list)
> +			flush_work(&target->tl_err_work);

Sorry for my silly question, but why do you do flush and not cancel
here? You anyway remove SRP device, so the result of flush is not
really important, am I right?

Thanks

>  		mutex_unlock(&host->target_mutex);
>  
> -		/*
> -		 * Wait for tl_err and target port removal tasks.
> -		 */
> -		flush_workqueue(system_long_wq);
>  		flush_workqueue(srp_remove_wq);
>  
>  		kfree(host);