| Message ID | 20221018135720.670094-3-hch@lst.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue | expand |
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
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
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
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
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);
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(-)