Re: [PATCH 3/4] nvme-apple: move blk_mq_update_nr_hw_queues after nvme_unfreeze

[Nilay Shroff <nilay@linux.ibm.com>]

On 2/9/26 9:05 PM, Keith Busch wrote:
> On Mon, Feb 09, 2026 at 03:58:32PM +0100, Christoph Hellwig wrote:
>> On Mon, Feb 09, 2026 at 04:29:52PM +0800, Yu Kuai wrote:
>>> blk_mq_update_nr_hw_queues() freezes and unfreezes queues internally.
>>> When the queue is already frozen before this call (from nvme_start_freeze
>>> in apple_nvme_disable), the freeze depth becomes 2. The internal unfreeze
>>> only decrements it to 1, leaving the queue still frozen when
>>> debugfs_create_files() is called.
>>>
>>> This triggers WARN_ON_ONCE(q->mq_freeze_depth != 0) in
>>> debugfs_create_files() and risks deadlock.
>>>
>>> Fix this by moving nvme_unfreeze() before blk_mq_update_nr_hw_queues()
>>> so the queue is unfrozen before the call, allowing the internal
>>> freeze/unfreeze to work correctly.
>>>
>>> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
>>> ---
>>>  drivers/nvme/host/apple.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
>>> index 15b3d07f8ccd..1835753ad91a 100644
>>> --- a/drivers/nvme/host/apple.c
>>> +++ b/drivers/nvme/host/apple.c
>>> @@ -1202,8 +1202,8 @@ static void apple_nvme_reset_work(struct work_struct *work)
>>>  
>>>  	nvme_unquiesce_io_queues(&anv->ctrl);
>>>  	nvme_wait_freeze(&anv->ctrl);
>>> -	blk_mq_update_nr_hw_queues(&anv->tagset, 1);
>>>  	nvme_unfreeze(&anv->ctrl);
>>> +	blk_mq_update_nr_hw_queues(&anv->tagset, 1);
>>
>> Looks good on it's own, but it would also good to align the
>> apple driver with the PCI one here more.
> 
> I'm pretty sure this series would deadlock nvme-pci, as that driver
> still leaves the queue frozen when calling blk_mq_update_nr_hw_queues.
> 
> We've left it frozen on purpose, though. The idea was to prevent new IO
> from entering a hw context that's no longer backed by a hardware
> resourse. Unfreezing prior opens that window up again. Maybe it's not a
> big deal; I don't often encounter scenarios where the queue count
> changes after a reset.

If an I/O were to slip through during the brief window between unfreeze
and the subsequent freeze inside blk_mq_update_nr_hw_queues(), wouldn’t
it still fail because the NVMe queues have already been suspended earlier
in the reset path? My understanding is that when the controller reset
reduces the number of online NVMe queues, the queues that are no longer
backed by hardware remain in the suspended state. As a result, any I/O
that reaches them before nr_hw_queues is updated should be rejected in
nvme_queue_rq(). And if that’s the case, then allowing a small unfreeze
window before updating the nr_hw_queue count shouldn’t result in a deadlock.
What do you think?

Thanks,
--Nilay