Commit edb854a3 authored by Ming Lei's avatar Ming Lei Committed by Martin K. Petersen
Browse files

scsi: core: Reallocate device's budget map on queue depth change 

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: default avatarMartin Wilck <martin.wilck@suse.com>
Suggested-by: default avatarMartin Wilck <martin.wilck@suse.com>
Tested-by: default avatarMartin Wilck <mwilck@suse.com>
Reviewed-by: default avatarMartin Wilck <mwilck@suse.com>
Reviewed-by: default avatarBart Van Assche <bvanassche@acm.org>
Signed-off-by: default avatarMing Lei <ming.lei@redhat.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 936bd034

drivers/scsi/scsi_scan.c

+50 −5
Original line number Diff line number Diff line
@@ -214,6 +214,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
@@ -306,11 +348,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;
@@ -1017,6 +1055,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)