Re: [PATCH] bcache: remove unnecessary flush_workqueue

[Coly Li <colyli@suse.de>]

On 3/27/22 3:20 PM, Li Lei wrote:
> All pending works will be drained by destroy_workqueue(), no need to call
> flush_workqueue() explicitly.
>
> Signed-off-by: Li Lei <lilei@szsandstone.com>
> ---
>   drivers/md/bcache/writeback.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c
> index 9ee0005874cd..2a6d9f39a9b1 100644
> --- a/drivers/md/bcache/writeback.c
> +++ b/drivers/md/bcache/writeback.c
> @@ -793,10 +793,9 @@ static int bch_writeback_thread(void *arg)
>   		}
>   	}
>   
> -	if (dc->writeback_write_wq) {
> -		flush_workqueue(dc->writeback_write_wq);
> +	if (dc->writeback_write_wq)
>   		destroy_workqueue(dc->writeback_write_wq);
> -	}
> +
>   	cached_dev_put(dc);
>   	wait_for_kthread_stop();
>   

The above code is from commit 7e865eba00a3 ("bcache: fix potential 
deadlock in cached_def_free()"). I explicitly added extra 
flush_workqueue() was because of workqueue_sysfs_unregister(wq) in 
destory_workqueue().

workqueue_sysfs_unregister() is not simple because it calls 
device_unregister(), and the code path is long. During reboot I am not 
sure whether extra deadlocking issue might be introduced. So the safe 
way is to explicitly call flush_workqueue() firstly to wait for all 
kwork finished, then destroy it.

It has been ~3 years passed, now I am totally OK with your above change. 
But could you please test your patch with lockdep enabled, and see 
whether there is no lock warning observed? Then I'd like to add it into 
my test directory.


Thanks.

Coly Li