[PATCH 2/2] nvme-pci: don't wait freeze during resetting

[Chao Leng]

On 2022/8/25 22:15, Ming Lei wrote:
> On Thu, Aug 25, 2022 at 07:42:44AM -0600, Keith Busch wrote:
>> On Thu, Aug 25, 2022 at 07:34:30PM +0800, Ming Lei wrote:
>>> On Thu, Aug 25, 2022 at 06:05:30PM +0800, Chao Leng wrote:
>>>>
>>>>
>>>> On 2022/8/21 16:47, Ming Lei wrote:
>>>>> First it isn't necessary to call nvme_wait_freeze in nvme_reset_work().
>>>>> If tagset isn't allocated, there can't be any inflight IOs; otherwise
>>>>> blk_mq_update_nr_hw_queues can freeze & wait queues.
>>>>>
>>>>> Second, since commit bdd6316094e0 ("block: Allow unfreezing of a queue
>>>>> while requests are in progress"), it is fine to unfreeze queue without
>>>>> draining inflight IOs.
>>>>>
>>>>> Signed-off-by: Ming Lei <ming.lei at redhat.com>
>>>>> ---
>>>>>    drivers/nvme/host/pci.c | 1 -
>>>>>    1 file changed, 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
>>>>> index 3a1c37f32f30..91b2903fcc24 100644
>>>>> --- a/drivers/nvme/host/pci.c
>>>>> +++ b/drivers/nvme/host/pci.c
>>>>> @@ -2910,7 +2910,6 @@ static void nvme_reset_work(struct work_struct *work)
>>>>>    		nvme_free_tagset(dev);
>>>>>    	} else {
>>>>>    		nvme_start_queues(&dev->ctrl);
>>>>> -		nvme_wait_freeze(&dev->ctrl);
>>>> It is not safe.
>>>> nvme_dev_add may call blk_mq_update_nr_hw_queues, blk_mq_update_nr_hw_queues
>>>> require the state is frozen.
>>>
>>> blk_mq_update_nr_hw_queues() calls blk_freeze_queue() for every queue in
>>> this tagset, so it needn't nvme's freeze wait.
>>
>> Right, the blk-mq realloc should be fine, and it's really a very uncommon event
>> that nvme needs to realloc the tag set.
>>
>> I'm trying to remember why I added the wait freeze here in the first place, and
>> I don't recall a good reason for it. I think I had a controller that always
>> timed out IO, and the resets would just continue indefinitely. Waiting for
>> freeze within a reset causes the next IO timeout to trigger a driver detach.
>> That doesn't sound like a good idea, though, since it tears down the admin
>> queue too.
> 
> The wait becomes not necessary since commit bdd6316094e0 ("block: Allow unfreezing
> of a queue while requests are in progress").
Yes, I didn't notice this before. The wait freeze in nvme is redundant.

Reviewed-by: Chao Leng <lengchao at huawei.com>
> 
> 
> Thanks,
> Ming
> 
> 
> .
>