All of lore.kernel.org
 help / color / mirror / Atom feed
* don't drop the queue reference in blk_mq_destroy_queue
@ 2022-10-18 13:57 Christoph Hellwig
  2022-10-18 13:57 ` [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue Christoph Hellwig
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Christoph Hellwig @ 2022-10-18 13:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

Hi all,

currently blk_mq_destroy_queue not only shuts down the queue, but also
drops a queue references, which leads to the NVMe PCIe/Apple drivers and
scsi grabbing an extra references. Fix this to help with unifying more
code between the various NVMe drivers (and to make the code a little saner
even on it's own).

Diffstat:
 block/blk-mq.c            |    4 +---
 block/bsg-lib.c           |    2 ++
 drivers/nvme/host/apple.c |    8 --------
 drivers/nvme/host/core.c  |   10 ++++++++--
 drivers/nvme/host/pci.c   |    5 -----
 drivers/scsi/scsi_scan.c  |    1 -
 drivers/ufs/core/ufshcd.c |    2 ++
 7 files changed, 13 insertions(+), 19 deletions(-)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue
  2022-10-18 13:57 don't drop the queue reference in blk_mq_destroy_queue Christoph Hellwig
@ 2022-10-18 13:57 ` Christoph Hellwig
  2022-10-18 20:44   ` Chaitanya Kulkarni
                     ` (2 more replies)
  2022-10-18 13:57 ` [PATCH 2/4] scsi: remove an extra queue reference Christoph Hellwig
                   ` (4 subsequent siblings)
  5 siblings, 3 replies; 20+ messages in thread
From: Christoph Hellwig @ 2022-10-18 13:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

The fact that blk_mq_destroy_queue also drops a queue reference leads
to various places having to grab an extra reference.  Move the call to
blk_put_queue into the callers to allow removing the extra references.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c            |  4 +---
 block/bsg-lib.c           |  2 ++
 drivers/nvme/host/apple.c |  1 +
 drivers/nvme/host/core.c  | 10 ++++++++--
 drivers/nvme/host/pci.c   |  1 +
 drivers/scsi/scsi_sysfs.c |  1 +
 drivers/ufs/core/ufshcd.c |  2 ++
 7 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 8070b6c10e8d5..ee644444e0f33 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4007,9 +4007,6 @@ void blk_mq_destroy_queue(struct request_queue *q)
 	blk_sync_queue(q);
 	blk_mq_cancel_work_sync(q);
 	blk_mq_exit_queue(q);
-
-	/* @q is and will stay empty, shutdown and put */
-	blk_put_queue(q);
 }
 EXPORT_SYMBOL(blk_mq_destroy_queue);
 
@@ -4026,6 +4023,7 @@ struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set, void *queuedata,
 	disk = __alloc_disk_node(q, set->numa_node, lkclass);
 	if (!disk) {
 		blk_mq_destroy_queue(q);
+		blk_put_queue(q);
 		return ERR_PTR(-ENOMEM);
 	}
 	set_bit(GD_OWNS_QUEUE, &disk->state);
diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index d6f5dcdce748c..435c32373cd68 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -325,6 +325,7 @@ void bsg_remove_queue(struct request_queue *q)
 
 		bsg_unregister_queue(bset->bd);
 		blk_mq_destroy_queue(q);
+		blk_put_queue(q);
 		blk_mq_free_tag_set(&bset->tag_set);
 		kfree(bset);
 	}
@@ -400,6 +401,7 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
 	return q;
 out_cleanup_queue:
 	blk_mq_destroy_queue(q);
+	blk_put_queue(q);
 out_queue:
 	blk_mq_free_tag_set(set);
 out_tag_set:
diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
index 5fc5ea196b400..42b17439dfd57 100644
--- a/drivers/nvme/host/apple.c
+++ b/drivers/nvme/host/apple.c
@@ -1508,6 +1508,7 @@ static int apple_nvme_probe(struct platform_device *pdev)
 	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;
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 059737c1a2c19..07381673170b9 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4847,6 +4847,7 @@ int nvme_alloc_admin_tag_set(struct nvme_ctrl *ctrl, struct blk_mq_tag_set *set,
 
 out_cleanup_admin_q:
 	blk_mq_destroy_queue(ctrl->fabrics_q);
+	blk_put_queue(ctrl->fabrics_q);
 out_free_tagset:
 	blk_mq_free_tag_set(ctrl->admin_tagset);
 	return ret;
@@ -4856,8 +4857,11 @@ EXPORT_SYMBOL_GPL(nvme_alloc_admin_tag_set);
 void nvme_remove_admin_tag_set(struct nvme_ctrl *ctrl)
 {
 	blk_mq_destroy_queue(ctrl->admin_q);
-	if (ctrl->ops->flags & NVME_F_FABRICS)
+	blk_put_queue(ctrl->admin_q);
+	if (ctrl->ops->flags & NVME_F_FABRICS) {
 		blk_mq_destroy_queue(ctrl->fabrics_q);
+		blk_put_queue(ctrl->fabrics_q);
+	}
 	blk_mq_free_tag_set(ctrl->admin_tagset);
 }
 EXPORT_SYMBOL_GPL(nvme_remove_admin_tag_set);
@@ -4903,8 +4907,10 @@ EXPORT_SYMBOL_GPL(nvme_alloc_io_tag_set);
 
 void nvme_remove_io_tag_set(struct nvme_ctrl *ctrl)
 {
-	if (ctrl->ops->flags & NVME_F_FABRICS)
+	if (ctrl->ops->flags & NVME_F_FABRICS) {
 		blk_mq_destroy_queue(ctrl->connect_q);
+		blk_put_queue(ctrl->connect_q);
+	}
 	blk_mq_free_tag_set(ctrl->tagset);
 }
 EXPORT_SYMBOL_GPL(nvme_remove_io_tag_set);
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index bcbef6bc5672f..16509b8d92e59 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1749,6 +1749,7 @@ static void nvme_dev_remove_admin(struct nvme_dev *dev)
 		 */
 		nvme_start_admin_queue(&dev->ctrl);
 		blk_mq_destroy_queue(dev->ctrl.admin_q);
+		blk_put_queue(dev->ctrl.admin_q);
 		blk_mq_free_tag_set(&dev->admin_tagset);
 	}
 }
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index c95177ca6ed26..1214c6f07bc64 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1478,6 +1478,7 @@ void __scsi_remove_device(struct scsi_device *sdev)
 	mutex_unlock(&sdev->state_mutex);
 
 	blk_mq_destroy_queue(sdev->request_queue);
+	blk_put_queue(sdev->request_queue);
 	kref_put(&sdev->host->tagset_refcnt, scsi_mq_free_tags);
 	cancel_work_sync(&sdev->requeue_work);
 
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 7256e6c43ca68..8ee0ac168ff2b 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9544,6 +9544,7 @@ void ufshcd_remove(struct ufs_hba *hba)
 	ufshpb_remove(hba);
 	ufs_sysfs_remove_nodes(hba->dev);
 	blk_mq_destroy_queue(hba->tmf_queue);
+	blk_put_queue(hba->tmf_queue);
 	blk_mq_free_tag_set(&hba->tmf_tag_set);
 	scsi_remove_host(hba->host);
 	/* disable interrupts */
@@ -9840,6 +9841,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 free_tmf_queue:
 	blk_mq_destroy_queue(hba->tmf_queue);
+	blk_put_queue(hba->tmf_queue);
 free_tmf_tag_set:
 	blk_mq_free_tag_set(&hba->tmf_tag_set);
 out_remove_scsi_host:
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/4] scsi: remove an extra queue reference
  2022-10-18 13:57 don't drop the queue reference in blk_mq_destroy_queue Christoph Hellwig
  2022-10-18 13:57 ` [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue Christoph Hellwig
@ 2022-10-18 13:57 ` Christoph Hellwig
  2022-10-18 20:44   ` Chaitanya Kulkarni
                     ` (2 more replies)
  2022-10-18 13:57 ` [PATCH 3/4] nvme-pci: " Christoph Hellwig
                   ` (3 subsequent siblings)
  5 siblings, 3 replies; 20+ messages in thread
From: Christoph Hellwig @ 2022-10-18 13:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

Now that blk_mq_destroy_queue does not release the queue reference, there
is no need for a second queue reference to be held by the scsi_device.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_scan.c  | 1 -
 drivers/scsi/scsi_sysfs.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 5d27f5196de6f..0a95fa787fdf4 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -344,7 +344,6 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	sdev->request_queue = q;
 	q->queuedata = sdev;
 	__scsi_init_queue(sdev->host, q);
-	WARN_ON_ONCE(!blk_get_queue(q));
 
 	depth = sdev->host->cmd_per_lun ?: 1;
 
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 1214c6f07bc64..c95177ca6ed26 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1478,7 +1478,6 @@ void __scsi_remove_device(struct scsi_device *sdev)
 	mutex_unlock(&sdev->state_mutex);
 
 	blk_mq_destroy_queue(sdev->request_queue);
-	blk_put_queue(sdev->request_queue);
 	kref_put(&sdev->host->tagset_refcnt, scsi_mq_free_tags);
 	cancel_work_sync(&sdev->requeue_work);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/4] nvme-pci: remove an extra queue reference
  2022-10-18 13:57 don't drop the queue reference in blk_mq_destroy_queue Christoph Hellwig
  2022-10-18 13:57 ` [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue Christoph Hellwig
  2022-10-18 13:57 ` [PATCH 2/4] scsi: remove an extra queue reference Christoph Hellwig
@ 2022-10-18 13:57 ` Christoph Hellwig
  2022-10-18 20:44   ` Chaitanya Kulkarni
  2022-10-19  8:19   ` Sagi Grimberg
  2022-10-18 13:57 ` [PATCH 4/4] nvme-apple: " Christoph Hellwig
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 20+ messages in thread
From: Christoph Hellwig @ 2022-10-18 13:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

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 nvme_dev.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/pci.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 16509b8d92e59..efea468a2a5bb 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1749,7 +1749,6 @@ static void nvme_dev_remove_admin(struct nvme_dev *dev)
 		 */
 		nvme_start_admin_queue(&dev->ctrl);
 		blk_mq_destroy_queue(dev->ctrl.admin_q);
-		blk_put_queue(dev->ctrl.admin_q);
 		blk_mq_free_tag_set(&dev->admin_tagset);
 	}
 }
@@ -1778,11 +1777,6 @@ static int nvme_pci_alloc_admin_tag_set(struct nvme_dev *dev)
 		dev->ctrl.admin_q = NULL;
 		return -ENOMEM;
 	}
-	if (!blk_get_queue(dev->ctrl.admin_q)) {
-		nvme_dev_remove_admin(dev);
-		dev->ctrl.admin_q = NULL;
-		return -ENODEV;
-	}
 	return 0;
 }
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/4] nvme-apple: remove an extra queue reference
  2022-10-18 13:57 don't drop the queue reference in blk_mq_destroy_queue Christoph Hellwig
                   ` (2 preceding siblings ...)
  2022-10-18 13:57 ` [PATCH 3/4] nvme-pci: " Christoph Hellwig
@ 2022-10-18 13:57 ` Christoph Hellwig
  2022-10-18 17:51   ` Sven Peter
                     ` (2 more replies)
  2022-10-18 15:16 ` don't drop the queue reference in blk_mq_destroy_queue Keith Busch
  2022-10-25 14:26 ` Jens Axboe
  5 siblings, 3 replies; 20+ messages in thread
From: Christoph Hellwig @ 2022-10-18 13:57 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

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.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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 42b17439dfd57..fe7f0444a8e46 100644
--- a/drivers/nvme/host/apple.c
+++ b/drivers/nvme/host/apple.c
@@ -1505,15 +1505,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.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: don't drop the queue reference in blk_mq_destroy_queue
  2022-10-18 13:57 don't drop the queue reference in blk_mq_destroy_queue Christoph Hellwig
                   ` (3 preceding siblings ...)
  2022-10-18 13:57 ` [PATCH 4/4] nvme-apple: " Christoph Hellwig
@ 2022-10-18 15:16 ` Keith Busch
  2022-10-25 14:26 ` Jens Axboe
  5 siblings, 0 replies; 20+ messages in thread
From: Keith Busch @ 2022-10-18 15:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Hector Martin, Sven Peter, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

Series looks good to me.

Reviewed-by: Keith Busch <kbusch@kernel.org>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/4] nvme-apple: remove an extra queue reference
  2022-10-18 13:57 ` [PATCH 4/4] nvme-apple: " Christoph Hellwig
@ 2022-10-18 17:51   ` Sven Peter
  2022-10-18 20:45   ` Chaitanya Kulkarni
  2022-10-19  8:19   ` Sagi Grimberg
  2 siblings, 0 replies; 20+ messages in thread
From: Sven Peter @ 2022-10-18 17:51 UTC (permalink / raw)
  To: hch, Jens Axboe
  Cc: Hector Martin, Keith Busch, sagi, Martin K. Petersen,
	linux-block, linux-nvme, linux-scsi

On Tue, Oct 18, 2022, at 15:57, Christoph Hellwig wrote:
> 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.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Sven Peter <sven@svenpeter.dev>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue
  2022-10-18 13:57 ` [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue Christoph Hellwig
@ 2022-10-18 20:44   ` Chaitanya Kulkarni
  2022-10-19  8:19   ` Sagi Grimberg
  2022-10-25 14:23   ` Jens Axboe
  2 siblings, 0 replies; 20+ messages in thread
From: Chaitanya Kulkarni @ 2022-10-18 20:44 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

On 10/18/22 06:57, Christoph Hellwig wrote:
> The fact that blk_mq_destroy_queue also drops a queue reference leads
> to various places having to grab an extra reference.  Move the call to
> blk_put_queue into the callers to allow removing the extra references.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Makes sense.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/4] scsi: remove an extra queue reference
  2022-10-18 13:57 ` [PATCH 2/4] scsi: remove an extra queue reference Christoph Hellwig
@ 2022-10-18 20:44   ` Chaitanya Kulkarni
  2022-10-19  1:16   ` Ming Lei
  2022-10-19  8:19   ` Sagi Grimberg
  2 siblings, 0 replies; 20+ messages in thread
From: Chaitanya Kulkarni @ 2022-10-18 20:44 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

On 10/18/22 06:57, Christoph Hellwig wrote:
> Now that blk_mq_destroy_queue does not release the queue reference, there
> is no need for a second queue reference to be held by the scsi_device.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/4] nvme-pci: remove an extra queue reference
  2022-10-18 13:57 ` [PATCH 3/4] nvme-pci: " Christoph Hellwig
@ 2022-10-18 20:44   ` Chaitanya Kulkarni
  2022-10-19  8:19   ` Sagi Grimberg
  1 sibling, 0 replies; 20+ messages in thread
From: Chaitanya Kulkarni @ 2022-10-18 20:44 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

On 10/18/22 06:57, Christoph Hellwig wrote:
> 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 nvme_dev.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/4] nvme-apple: remove an extra queue reference
  2022-10-18 13:57 ` [PATCH 4/4] nvme-apple: " Christoph Hellwig
  2022-10-18 17:51   ` Sven Peter
@ 2022-10-18 20:45   ` Chaitanya Kulkarni
  2022-10-19  8:19   ` Sagi Grimberg
  2 siblings, 0 replies; 20+ messages in thread
From: Chaitanya Kulkarni @ 2022-10-18 20:45 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

On 10/18/22 06:57, Christoph Hellwig wrote:
> 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.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/4] scsi: remove an extra queue reference
  2022-10-18 13:57 ` [PATCH 2/4] scsi: remove an extra queue reference Christoph Hellwig
  2022-10-18 20:44   ` Chaitanya Kulkarni
@ 2022-10-19  1:16   ` Ming Lei
  2022-10-19  1:23     ` Ming Lei
  2022-10-19  8:19   ` Sagi Grimberg
  2 siblings, 1 reply; 20+ messages in thread
From: Ming Lei @ 2022-10-19  1:16 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Hector Martin, Sven Peter, Keith Busch,
	Sagi Grimberg, Martin K. Petersen, linux-block, linux-nvme,
	linux-scsi

On Tue, Oct 18, 2022 at 03:57:18PM +0200, Christoph Hellwig wrote:
> Now that blk_mq_destroy_queue does not release the queue reference, there
> is no need for a second queue reference to be held by the scsi_device.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/scsi_scan.c  | 1 -
>  drivers/scsi/scsi_sysfs.c | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 5d27f5196de6f..0a95fa787fdf4 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -344,7 +344,6 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
>  	sdev->request_queue = q;
>  	q->queuedata = sdev;
>  	__scsi_init_queue(sdev->host, q);
> -	WARN_ON_ONCE(!blk_get_queue(q));
>  
>  	depth = sdev->host->cmd_per_lun ?: 1;
>  
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 1214c6f07bc64..c95177ca6ed26 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1478,7 +1478,6 @@ void __scsi_remove_device(struct scsi_device *sdev)
>  	mutex_unlock(&sdev->state_mutex);
>  
>  	blk_mq_destroy_queue(sdev->request_queue);
> -	blk_put_queue(sdev->request_queue);

The above put is counter-pair of blk_get_queue() in scsi_alloc_sdev, and
the original blk_put_queue() in blk_mq_destroy_queue() is counter-pair of
the initial get in blk_alloc_queue().

Now blk_put_queue() is moved out of blk_mq_destroy_queue(), I am wondering
how the scsi queue lifetime can work correctly with this patch? Or is there
bug in current scsi code?


Thanks,
Ming


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/4] scsi: remove an extra queue reference
  2022-10-19  1:16   ` Ming Lei
@ 2022-10-19  1:23     ` Ming Lei
  0 siblings, 0 replies; 20+ messages in thread
From: Ming Lei @ 2022-10-19  1:23 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Hector Martin, Sven Peter, Keith Busch,
	Sagi Grimberg, Martin K. Petersen, linux-block, linux-nvme,
	linux-scsi

On Wed, Oct 19, 2022 at 09:16:57AM +0800, Ming Lei wrote:
> On Tue, Oct 18, 2022 at 03:57:18PM +0200, Christoph Hellwig wrote:
> > Now that blk_mq_destroy_queue does not release the queue reference, there
> > is no need for a second queue reference to be held by the scsi_device.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  drivers/scsi/scsi_scan.c  | 1 -
> >  drivers/scsi/scsi_sysfs.c | 1 -
> >  2 files changed, 2 deletions(-)
> > 
> > diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> > index 5d27f5196de6f..0a95fa787fdf4 100644
> > --- a/drivers/scsi/scsi_scan.c
> > +++ b/drivers/scsi/scsi_scan.c
> > @@ -344,7 +344,6 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
> >  	sdev->request_queue = q;
> >  	q->queuedata = sdev;
> >  	__scsi_init_queue(sdev->host, q);
> > -	WARN_ON_ONCE(!blk_get_queue(q));
> >  
> >  	depth = sdev->host->cmd_per_lun ?: 1;
> >  
> > diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> > index 1214c6f07bc64..c95177ca6ed26 100644
> > --- a/drivers/scsi/scsi_sysfs.c
> > +++ b/drivers/scsi/scsi_sysfs.c
> > @@ -1478,7 +1478,6 @@ void __scsi_remove_device(struct scsi_device *sdev)
> >  	mutex_unlock(&sdev->state_mutex);
> >  
> >  	blk_mq_destroy_queue(sdev->request_queue);
> > -	blk_put_queue(sdev->request_queue);
> 
> The above put is counter-pair of blk_get_queue() in scsi_alloc_sdev, and
> the original blk_put_queue() in blk_mq_destroy_queue() is counter-pair of
> the initial get in blk_alloc_queue().
> 
> Now blk_put_queue() is moved out of blk_mq_destroy_queue(), I am wondering
> how the scsi queue lifetime can work correctly with this patch? Or is there
> bug in current scsi code?

oops, the above blk_put_queue() is actually added in the 1st patch, so
this patch is fine, sorry for the noise.


thanks,
Ming


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue
  2022-10-18 13:57 ` [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue Christoph Hellwig
  2022-10-18 20:44   ` Chaitanya Kulkarni
@ 2022-10-19  8:19   ` Sagi Grimberg
  2022-10-25 14:23   ` Jens Axboe
  2 siblings, 0 replies; 20+ messages in thread
From: Sagi Grimberg @ 2022-10-19  8:19 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Martin K. Petersen,
	linux-block, linux-nvme, linux-scsi

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/4] scsi: remove an extra queue reference
  2022-10-18 13:57 ` [PATCH 2/4] scsi: remove an extra queue reference Christoph Hellwig
  2022-10-18 20:44   ` Chaitanya Kulkarni
  2022-10-19  1:16   ` Ming Lei
@ 2022-10-19  8:19   ` Sagi Grimberg
  2 siblings, 0 replies; 20+ messages in thread
From: Sagi Grimberg @ 2022-10-19  8:19 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Martin K. Petersen,
	linux-block, linux-nvme, linux-scsi

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/4] nvme-pci: remove an extra queue reference
  2022-10-18 13:57 ` [PATCH 3/4] nvme-pci: " Christoph Hellwig
  2022-10-18 20:44   ` Chaitanya Kulkarni
@ 2022-10-19  8:19   ` Sagi Grimberg
  1 sibling, 0 replies; 20+ messages in thread
From: Sagi Grimberg @ 2022-10-19  8:19 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Martin K. Petersen,
	linux-block, linux-nvme, linux-scsi

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/4] nvme-apple: remove an extra queue reference
  2022-10-18 13:57 ` [PATCH 4/4] nvme-apple: " Christoph Hellwig
  2022-10-18 17:51   ` Sven Peter
  2022-10-18 20:45   ` Chaitanya Kulkarni
@ 2022-10-19  8:19   ` Sagi Grimberg
  2 siblings, 0 replies; 20+ messages in thread
From: Sagi Grimberg @ 2022-10-19  8:19 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: Hector Martin, Sven Peter, Keith Busch, Martin K. Petersen,
	linux-block, linux-nvme, linux-scsi

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue
  2022-10-18 13:57 ` [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue Christoph Hellwig
  2022-10-18 20:44   ` Chaitanya Kulkarni
  2022-10-19  8:19   ` Sagi Grimberg
@ 2022-10-25 14:23   ` Jens Axboe
  2022-10-25 14:25     ` Jens Axboe
  2 siblings, 1 reply; 20+ messages in thread
From: Jens Axboe @ 2022-10-25 14:23 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Hector Martin, Sven Peter, Keith Busch, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 059737c1a2c19..07381673170b9 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -4847,6 +4847,7 @@ int nvme_alloc_admin_tag_set(struct nvme_ctrl *ctrl, struct blk_mq_tag_set *set,
>  
>  out_cleanup_admin_q:
>  	blk_mq_destroy_queue(ctrl->fabrics_q);
> +	blk_put_queue(ctrl->fabrics_q);
>  out_free_tagset:
>  	blk_mq_free_tag_set(ctrl->admin_tagset);
>  	return ret;
This is wrong and doesn't apply because it got fixed upstream:

commit 4739824e2d7878dcea88397a6758e31e3c5c124e
Author: Dan Carpenter <error27@gmail.com>
Date:   Sat Oct 15 11:25:56 2022 +0300

    nvme: fix error pointer dereference in error handling

Can you respin this series so we can get it applied?

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue
  2022-10-25 14:23   ` Jens Axboe
@ 2022-10-25 14:25     ` Jens Axboe
  0 siblings, 0 replies; 20+ messages in thread
From: Jens Axboe @ 2022-10-25 14:25 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Hector Martin, Sven Peter, Keith Busch, Sagi Grimberg,
	Martin K. Petersen, linux-block, linux-nvme, linux-scsi

On 10/25/22 8:23 AM, Jens Axboe wrote:
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 059737c1a2c19..07381673170b9 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -4847,6 +4847,7 @@ int nvme_alloc_admin_tag_set(struct nvme_ctrl *ctrl, struct blk_mq_tag_set *set,
>>  
>>  out_cleanup_admin_q:
>>  	blk_mq_destroy_queue(ctrl->fabrics_q);
>> +	blk_put_queue(ctrl->fabrics_q);
>>  out_free_tagset:
>>  	blk_mq_free_tag_set(ctrl->admin_tagset);
>>  	return ret;
> This is wrong and doesn't apply because it got fixed upstream:
> 
> commit 4739824e2d7878dcea88397a6758e31e3c5c124e
> Author: Dan Carpenter <error27@gmail.com>
> Date:   Sat Oct 15 11:25:56 2022 +0300
> 
>     nvme: fix error pointer dereference in error handling
> 
> Can you respin this series so we can get it applied?

I just made the trivial edit in the patch itself, rest was
fine.

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: don't drop the queue reference in blk_mq_destroy_queue
  2022-10-18 13:57 don't drop the queue reference in blk_mq_destroy_queue Christoph Hellwig
                   ` (4 preceding siblings ...)
  2022-10-18 15:16 ` don't drop the queue reference in blk_mq_destroy_queue Keith Busch
@ 2022-10-25 14:26 ` Jens Axboe
  5 siblings, 0 replies; 20+ messages in thread
From: Jens Axboe @ 2022-10-25 14:26 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Sagi Grimberg, linux-scsi, Keith Busch, linux-block, Sven Peter,
	Hector Martin, Martin K. Petersen, linux-nvme

On Tue, 18 Oct 2022 15:57:16 +0200, Christoph Hellwig wrote:
> currently blk_mq_destroy_queue not only shuts down the queue, but also
> drops a queue references, which leads to the NVMe PCIe/Apple drivers and
> scsi grabbing an extra references. Fix this to help with unifying more
> code between the various NVMe drivers (and to make the code a little saner
> even on it's own).
> 
> Diffstat:
>  block/blk-mq.c            |    4 +---
>  block/bsg-lib.c           |    2 ++
>  drivers/nvme/host/apple.c |    8 --------
>  drivers/nvme/host/core.c  |   10 ++++++++--
>  drivers/nvme/host/pci.c   |    5 -----
>  drivers/scsi/scsi_scan.c  |    1 -
>  drivers/ufs/core/ufshcd.c |    2 ++
>  7 files changed, 13 insertions(+), 19 deletions(-)
> 
> [...]

Applied, thanks!

[1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue
      commit: 2b3f056f72e56fa07df69b4705e0b46a6c08e77c
[2/4] scsi: remove an extra queue reference
      commit: dc917c361422388f0d39d3f0dc2bc5a188c01156
[3/4] nvme-pci: remove an extra queue reference
      commit: 7dcebef90d35de13a326f765dd787538880566f9
[4/4] nvme-apple: remove an extra queue reference
      commit: 941f7298c70c7668416e7845fa76eb72c07d966b

Best regards,
-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-10-25 14:26 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-18 13:57 don't drop the queue reference in blk_mq_destroy_queue Christoph Hellwig
2022-10-18 13:57 ` [PATCH 1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue Christoph Hellwig
2022-10-18 20:44   ` Chaitanya Kulkarni
2022-10-19  8:19   ` Sagi Grimberg
2022-10-25 14:23   ` Jens Axboe
2022-10-25 14:25     ` Jens Axboe
2022-10-18 13:57 ` [PATCH 2/4] scsi: remove an extra queue reference Christoph Hellwig
2022-10-18 20:44   ` Chaitanya Kulkarni
2022-10-19  1:16   ` Ming Lei
2022-10-19  1:23     ` Ming Lei
2022-10-19  8:19   ` Sagi Grimberg
2022-10-18 13:57 ` [PATCH 3/4] nvme-pci: " Christoph Hellwig
2022-10-18 20:44   ` Chaitanya Kulkarni
2022-10-19  8:19   ` Sagi Grimberg
2022-10-18 13:57 ` [PATCH 4/4] nvme-apple: " Christoph Hellwig
2022-10-18 17:51   ` Sven Peter
2022-10-18 20:45   ` Chaitanya Kulkarni
2022-10-19  8:19   ` Sagi Grimberg
2022-10-18 15:16 ` don't drop the queue reference in blk_mq_destroy_queue Keith Busch
2022-10-25 14:26 ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.