On 3/15/22 03:39, 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.
>
> Signed-off-by: John Garry <[email protected]>
> ---
> drivers/scsi/scsi_scan.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> 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);
Hmm ... why to call both sbitmap_init_node() and sbitmap_resize()
instead of combining both calls into a single call with the proper depth?
Thanks,
Bart.
On 15/03/2022 14:33, Bart Van Assche wrote:
>> 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);
>
> Hmm ... why to call both sbitmap_init_node() and sbitmap_resize()
> instead of combining both calls into a single call with the proper depth?
Hi Bart,
Is the user wants to change the queue depth later via sysfs we do not
reallocate the sbitmap then. So we need to ensure that the size we
reallocate here will satisfy the scsi device max depth. I'm referencing
scsi_change_queue_depth() for this.
Thanks,
John