diff mbox series

[v2] scsi: core: Fix sbitmap depth in scsi_realloc_sdev_budget_map()

Message ID 1647423870-143867-1-git-send-email-john.garry@huawei.com (mailing list archive)
State Accepted
Headers show
Series [v2] scsi: core: Fix sbitmap depth in scsi_realloc_sdev_budget_map() | expand

Commit Message

John Garry March 16, 2022, 9:44 a.m. UTC
In commit edb854a3680b ("scsi: core: Reallocate device's budget map on
queue depth change"), the sbitmap for the device budget map may be
reallocated after the slave device depth is configured.

When the sbitmap is reallocated we use the result from
scsi_device_max_queue_depth() for the sbitmap size, but don't resize to
match the actual device queue depth.

Fix by resizing the sbitmap after reallocating the budget sbitmap. We do
this instead of init'ing the sbitmap to the device queue depth as the user
may want to change the queue depth later via sysfs or other.

Fixes: edb854a3680b ("scsi: core: Reallocate device's budget map on queue depth change")
Signed-off-by: John Garry <john.garry@huawei.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
---
Changes since v1 (apart from sending as a separate patch):
- Add fixes and RB tag (thanks)
- mention in commit message why we don't init sbitmap at queue depth

Comments

Damien Le Moal March 16, 2022, 11:38 a.m. UTC | #1
On 3/16/22 18:44, John Garry wrote:
> In commit edb854a3680b ("scsi: core: Reallocate device's budget map on
> queue depth change"), the sbitmap for the device budget map may be
> reallocated after the slave device depth is configured.
> 
> When the sbitmap is reallocated we use the result from
> scsi_device_max_queue_depth() for the sbitmap size, but don't resize to
> match the actual device queue depth.
> 
> Fix by resizing the sbitmap after reallocating the budget sbitmap. We do
> this instead of init'ing the sbitmap to the device queue depth as the user
> may want to change the queue depth later via sysfs or other.
> 
> Fixes: edb854a3680b ("scsi: core: Reallocate device's budget map on queue depth change")
> Signed-off-by: John Garry <john.garry@huawei.com>
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> ---
> Changes since v1 (apart from sending as a separate patch):
> - Add fixes and RB tag (thanks)
> - mention in commit message why we don't init sbitmap at queue depth
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index f4e6c68ac99e..2ef78083f1ef 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -223,6 +223,8 @@ static int scsi_realloc_sdev_budget_map(struct scsi_device *sdev,
>  	int ret;
>  	struct sbitmap sb_backup;
>  
> +	depth = min_t(unsigned int, depth, scsi_device_max_queue_depth(sdev));
> +
>  	/*
>  	 * realloc if new shift is calculated, which is caused by setting
>  	 * up one new default queue depth after calling ->slave_configure
> @@ -245,6 +247,9 @@ static int scsi_realloc_sdev_budget_map(struct scsi_device *sdev,
>  				scsi_device_max_queue_depth(sdev),
>  				new_shift, GFP_KERNEL,
>  				sdev->request_queue->node, false, true);
> +	if (!ret)
> +		sbitmap_resize(&sdev->budget_map, depth);
> +
>  	if (need_free) {
>  		if (ret)
>  			sdev->budget_map = sb_backup;

Tested-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Bart Van Assche March 16, 2022, 2:29 p.m. UTC | #2
On 3/16/22 02:44, John Garry wrote:
> In commit edb854a3680b ("scsi: core: Reallocate device's budget map on
> queue depth change"), the sbitmap for the device budget map may be
> reallocated after the slave device depth is configured.
> 
> When the sbitmap is reallocated we use the result from
> scsi_device_max_queue_depth() for the sbitmap size, but don't resize to
> match the actual device queue depth.
> 
> Fix by resizing the sbitmap after reallocating the budget sbitmap. We do
> this instead of init'ing the sbitmap to the device queue depth as the user
> may want to change the queue depth later via sysfs or other.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Martin K. Petersen March 30, 2022, 3:45 a.m. UTC | #3
John,

> In commit edb854a3680b ("scsi: core: Reallocate device's budget map on
> queue depth change"), the sbitmap for the device budget map may be
> reallocated after the slave device depth is configured.

Applied to 5.18/scsi-staging, thanks!
Martin K. Petersen April 7, 2022, 1:35 p.m. UTC | #4
On Wed, 16 Mar 2022 17:44:30 +0800, John Garry wrote:

> In commit edb854a3680b ("scsi: core: Reallocate device's budget map on
> queue depth change"), the sbitmap for the device budget map may be
> reallocated after the slave device depth is configured.
> 
> When the sbitmap is reallocated we use the result from
> scsi_device_max_queue_depth() for the sbitmap size, but don't resize to
> match the actual device queue depth.
> 
> [...]

Applied to 5.18/scsi-fixes, thanks!

[1/1] scsi: core: Fix sbitmap depth in scsi_realloc_sdev_budget_map()
      https://git.kernel.org/mkp/scsi/c/eaba83b5b850
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index f4e6c68ac99e..2ef78083f1ef 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -223,6 +223,8 @@  static int scsi_realloc_sdev_budget_map(struct scsi_device *sdev,
 	int ret;
 	struct sbitmap sb_backup;
 
+	depth = min_t(unsigned int, depth, scsi_device_max_queue_depth(sdev));
+
 	/*
 	 * realloc if new shift is calculated, which is caused by setting
 	 * up one new default queue depth after calling ->slave_configure
@@ -245,6 +247,9 @@  static int scsi_realloc_sdev_budget_map(struct scsi_device *sdev,
 				scsi_device_max_queue_depth(sdev),
 				new_shift, GFP_KERNEL,
 				sdev->request_queue->node, false, true);
+	if (!ret)
+		sbitmap_resize(&sdev->budget_map, depth);
+
 	if (need_free) {
 		if (ret)
 			sdev->budget_map = sb_backup;