[PATCH V5 7/7] nvme-core: warn on allocating admin tag set with existing queue

[Maurizio Lombardi]

On Tue May 19, 2026 at 10:52 AM CEST, Hannes Reinecke wrote:
> On 5/14/26 10:32, Maurizio Lombardi wrote:
>> Currently, nvme_alloc_admin_tag_set() silently drops and releases
>> the existing admin_q if it called on a controller that already
>> had one (e.g., during a controller reset).
>> 
>> However, transport drivers should not be reallocating the admin tag
>> set and queue during a reset. Dropping the old queue and allocating
>> a new one destroys user-configured timeouts and may race against
>> nvme_admin_timeout_store()
>> 
>> Since all transport drivers are now expected to preserve the admin queue
>> across resets, calling nvme_alloc_admin_tag_set() when ctrl->admin_q
>> is already populated is a bug.
>> 
>> Remove the silent cleanup and replace it with a WARN_ON_ONCE() to
>> explicitly catch any transport drivers that violate this lifecycle rule
>> 
>> Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
>> Reviewed-by: Christoph Hellwig <hch at lst.de>
>> Reviewed-by: Daniel Wagner <dwagner at suse.de>
>> Signed-off-by: Maurizio Lombardi <mlombard at redhat.com>
>> ---
>>   drivers/nvme/host/core.c | 7 +------
>>   1 file changed, 1 insertion(+), 6 deletions(-)
>> 
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index e6bb1f5e9657..f23a957c6e8f 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -4893,12 +4893,7 @@ int nvme_alloc_admin_tag_set(struct nvme_ctrl *ctrl, struct blk_mq_tag_set *set,
>>   	if (ret)
>>   		return ret;
>>   
>> -	/*
>> -	 * If a previous admin queue exists (e.g., from before a reset),
>> -	 * put it now before allocating a new one to avoid orphaning it.
>> -	 */
>> -	if (ctrl->admin_q)
>> -		blk_put_queue(ctrl->admin_q);
>> +	WARN_ON_ONCE(ctrl->admin_q);
>>   
>>   	ctrl->admin_q = blk_mq_alloc_queue(set, NULL, NULL);
>>   	if (IS_ERR(ctrl->admin_q)) {
>
> Why don't you return an error here?
> The WARN_ON() now introduces a memory leak (at best); I'd rather
> not do that here as we can easily recover here.

Ok if I just do the following?

if (WARN_ON_ONCE(ctrl->admin_q))
	blk_put_queue(ctrl->admin_q);


Maurizio