Re: [PATCH] bcache: remove unnecessary flush_workqueue

[Coly Li <colyli@suse.de>]

On 4/9/22 9:17 AM, 李磊 wrote:
> Coly Li <colyli@suse.de> 于2022年4月8日周五 00:54写道：
>> 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.
>>
> OK，I will test this scenario.


Any progress?


Coly Li