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