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: 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>

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 3520b9384428941a7ae9a06371b2810f66978d13..f4e6c68ac99eddad151716e36e075347ddb54d1d 100644 (file)
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -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)