Skip to content

Commit

Permalink
scsi: core: Reallocate device's budget map on queue depth change
Browse files Browse the repository at this point in the history
[ Upstream commit edb854a ]

We currently use ->cmd_per_lun as initial queue depth for setting up the
budget_map. Martin Wilck reported that it is common for the queue_depth to
be subsequently updated in slave_configure() based on detected hardware
characteristics.

As a result, for some drivers, the static host template settings for
cmd_per_lun and can_queue won't actually get used in practice. And if the
default values are used to allocate the budget_map, memory may be consumed
unnecessarily.

Fix the issue by reallocating the budget_map after ->slave_configure()
returns. At that time the device queue_depth should accurately reflect what
the hardware needs.

Link: https://lore.kernel.org/r/20220127153733.409132-1-ming.lei@redhat.com
Cc: Bart Van Assche <bvanassche@acm.org>
Reported-by: Martin Wilck <martin.wilck@suse.com>
Suggested-by: Martin Wilck <martin.wilck@suse.com>
Tested-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
Ming Lei authored and gregkh committed Feb 23, 2022
1 parent 46756cc commit f2c5866
Showing 1 changed file with 50 additions and 5 deletions.
55 changes: 50 additions & 5 deletions drivers/scsi/scsi_scan.c
Expand Up @@ -198,6 +198,48 @@ static void scsi_unlock_floptical(struct scsi_device *sdev,
SCSI_TIMEOUT, 3, NULL);
}

static int scsi_realloc_sdev_budget_map(struct scsi_device *sdev,
unsigned int depth)
{
int new_shift = sbitmap_calculate_shift(depth);
bool need_alloc = !sdev->budget_map.map;
bool need_free = false;
int ret;
struct sbitmap sb_backup;

/*
* realloc if new shift is calculated, which is caused by setting
* up one new default queue depth after calling ->slave_configure
*/
if (!need_alloc && new_shift != sdev->budget_map.shift)
need_alloc = need_free = true;

if (!need_alloc)
return 0;

/*
* Request queue has to be frozen for reallocating budget map,
* and here disk isn't added yet, so freezing is pretty fast
*/
if (need_free) {
blk_mq_freeze_queue(sdev->request_queue);
sb_backup = sdev->budget_map;
}
ret = sbitmap_init_node(&sdev->budget_map,
scsi_device_max_queue_depth(sdev),
new_shift, GFP_KERNEL,
sdev->request_queue->node, false, true);
if (need_free) {
if (ret)
sdev->budget_map = sb_backup;
else
sbitmap_free(&sb_backup);
ret = 0;
blk_mq_unfreeze_queue(sdev->request_queue);
}
return ret;
}

/**
* scsi_alloc_sdev - allocate and setup a scsi_Device
* @starget: which target to allocate a &scsi_device for
Expand Down Expand Up @@ -291,11 +333,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
* default device queue depth to figure out sbitmap shift
* since we use this queue depth most of times.
*/
if (sbitmap_init_node(&sdev->budget_map,
scsi_device_max_queue_depth(sdev),
sbitmap_calculate_shift(depth),
GFP_KERNEL, sdev->request_queue->node,
false, true)) {
if (scsi_realloc_sdev_budget_map(sdev, depth)) {
put_device(&starget->dev);
kfree(sdev);
goto out;
Expand Down Expand Up @@ -1001,6 +1039,13 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
}
return SCSI_SCAN_NO_RESPONSE;
}

/*
* The queue_depth is often changed in ->slave_configure.
* Set up budget map again since memory consumption of
* the map depends on actual queue depth.
*/
scsi_realloc_sdev_budget_map(sdev, sdev->queue_depth);
}

if (sdev->scsi_level >= SCSI_3)
Expand Down

0 comments on commit f2c5866

Please sign in to comment.