Re: [PATCH 6.1.y 5/8] nvme-apple: remove an extra queue reference

[Fedor Pchelkin]

Hello,

"Heyne, Maximilian" <mheyne@xxxxxxxxx> wrote:
> From: Christoph Hellwig <hch@xxxxxx>
> 
> [ Upstream commit 941f7298c70c7668416e7845fa76eb72c07d966b ]
> 
> Now that blk_mq_destroy_queue does not release the queue reference, there
> is no need for a second admin queue reference to be held by the
> apple_nvme structure.

This patch is probably buggy in upstream.  It removes extra reference
->get, but doesn't remove the corresponding ->put which is located
inside apple_nvme_free_ctrl().

I'm reporting here currently just for the heads up - was looking at the
same nvme regression problem at 6.1.y, found this thread, and the
nvme-apple changes appeared suspicious.

nvme-apple patch is not required to fix the regression (this also holds
true for [PATCH 6.1.y 3/8] scsi: remove an extra queue reference).  Maybe
they shouldn't go to stable.

That said, the other part of the backport series FWIW looks good to me,
and I've also verified it resolves the 6.1.y regression.

Thanks.

> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Sagi Grimberg <sagi@xxxxxxxxxxx>
> Reviewed-by: Sven Peter <sven@xxxxxxxxxxxxx>
> Reviewed-by: Chaitanya Kulkarni <kch@xxxxxxxxxx>
> Reviewed-by: Keith Busch <kbusch@xxxxxxxxxx>
> Link: https://lore.kernel.org/r/20221018135720.670094-5-hch@xxxxxx
> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
> Signed-off-by: Maximilian Heyne <mheyne@xxxxxxxxx>
> ---
>  drivers/nvme/host/apple.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
> index c5fc293c22123..c84ebfcfdeb88 100644
> --- a/drivers/nvme/host/apple.c
> +++ b/drivers/nvme/host/apple.c
> @@ -1507,15 +1507,6 @@ static int apple_nvme_probe(struct platform_device *pdev)
>  		goto put_dev;
>  	}
>  
> -	if (!blk_get_queue(anv->ctrl.admin_q)) {
> -		nvme_start_admin_queue(&anv->ctrl);
> -		blk_mq_destroy_queue(anv->ctrl.admin_q);
> -		blk_put_queue(anv->ctrl.admin_q);
> -		anv->ctrl.admin_q = NULL;
> -		ret = -ENODEV;
> -		goto put_dev;
> -	}
> -
>  	nvme_reset_ctrl(&anv->ctrl);
>  	async_schedule(apple_nvme_async_probe, anv);
>  
> -- 
> 2.50.1