scsi: core: Replace sdev->device_busy with sbitmap
authorMing Lei <ming.lei@redhat.com>
Fri, 22 Jan 2021 02:33:17 +0000 (10:33 +0800)
committerMartin K. Petersen <martin.petersen@oracle.com>
Thu, 4 Mar 2021 22:37:00 +0000 (17:37 -0500)
SCSI currently uses an atomic variable to track queue depth for each
attached device. The queue depth depends on many factors such as transport
type and device implementation. In addition, the SCSI device queue depth is
not a static entity but changes over time as a result of congestion
management.

While blk-mq currently tracks queue depth for each hctx, it can't easily be
changed to accommodate the SCSI per-device requirement.

The current approach of using an atomic variable doesn't scale well when
there are lots of CPU cores and the disk is very fast. IOPS can be
substantially impacted by the atomic in the hot path.

Replace the atomic variable sdev->device_busy with an sbitmap for tracking
the SCSI device queue depth.

It has been observed that IOPS is improved ~30% by this patchset in the
following test:

1) test machine(32 logical CPU cores)
Thread(s) per core:  2
Core(s) per socket:  8
Socket(s):           2
NUMA node(s):        2
Model name:          Intel(R) Xeon(R) Silver 4110 CPU @ 2.10GHz

2) setup scsi_debug:
modprobe scsi_debug virtual_gb=128 max_luns=1 submit_queues=32 delay=0 max_queue=256

3) fio script:
fio --rw=randread --size=128G --direct=1 --ioengine=libaio --iodepth=2048 \
--numjobs=32 --bs=4k --group_reporting=1 --group_reporting=1 --runtime=60 \
--loops=10000 --name=job1 --filename=/dev/sdN

[mkp: fix device_busy reference in mpt3sas]

Link: https://lore.kernel.org/r/20210122023317.687987-14-ming.lei@redhat.com
Link: https://lore.kernel.org/linux-block/20200119071432.18558-6-ming.lei@redhat.com/
Cc: Omar Sandoval <osandov@fb.com>
Cc: Kashyap Desai <kashyap.desai@broadcom.com>
Cc: Sumanesh Samanta <sumanesh.samanta@broadcom.com>
Cc: Ewan D. Milne <emilne@redhat.com>
Cc: Hannes Reinecke <hare@suse.de>
Tested-by: Sumanesh Samanta <sumanesh.samanta@broadcom.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
drivers/scsi/mpt3sas/mpt3sas_base.c
drivers/scsi/scsi.c
drivers/scsi/scsi_lib.c
drivers/scsi/scsi_priv.h
drivers/scsi/scsi_scan.c
drivers/scsi/scsi_sysfs.c
include/scsi/scsi_device.h

index ac066f86bb141b3708c9655d1e830f2c2ac6cff4..504ed856d479743a74af967314186d6e6c91c484 100644 (file)
@@ -3678,8 +3678,7 @@ _base_get_high_iops_msix_index(struct MPT3SAS_ADAPTER *ioc,
         * IOs on the target device is >=8.
         */
 
-       if (atomic_read(&scmd->device->device_busy) >
-           MPT3SAS_DEVICE_HIGH_IOPS_DEPTH)
+       if (scsi_device_busy(scmd->device) > MPT3SAS_DEVICE_HIGH_IOPS_DEPTH)
                return base_mod64((
                    atomic64_add_return(1, &ioc->high_iops_outstanding) /
                    MPT3SAS_HIGH_IOPS_BATCH_COUNT),
index a28d48c850cf5721d14657fc5e0b9b247aa5c0bf..e9e2f0e15ac85ece57414fa488b89434fb258120 100644 (file)
@@ -218,7 +218,7 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
 /*
  * 1024 is big enough for saturating the fast scsi LUN now
  */
-static int scsi_device_max_queue_depth(struct scsi_device *sdev)
+int scsi_device_max_queue_depth(struct scsi_device *sdev)
 {
        return max_t(int, sdev->host->can_queue, 1024);
 }
@@ -242,6 +242,8 @@ int scsi_change_queue_depth(struct scsi_device *sdev, int depth)
        if (sdev->request_queue)
                blk_set_queue_depth(sdev->request_queue, depth);
 
+       sbitmap_resize(&sdev->budget_map, sdev->queue_depth);
+
        return sdev->queue_depth;
 }
 EXPORT_SYMBOL(scsi_change_queue_depth);
index 2f8578fb89ff0fdeeefa93aab79a5fdb5d19f07a..ffe824782647660916955a23e59df4b468e26a38 100644 (file)
@@ -328,7 +328,7 @@ void scsi_device_unbusy(struct scsi_device *sdev, struct scsi_cmnd *cmd)
        if (starget->can_queue > 0)
                atomic_dec(&starget->target_busy);
 
-       atomic_dec(&sdev->device_busy);
+       sbitmap_put(&sdev->budget_map, cmd->budget_token);
        cmd->budget_token = -1;
 }
 
@@ -1257,19 +1257,20 @@ scsi_device_state_check(struct scsi_device *sdev, struct request *req)
 }
 
 /*
- * scsi_dev_queue_ready: if we can send requests to sdev, return 1 else
- * return 0.
- *
- * Called with the queue_lock held.
+ * scsi_dev_queue_ready: if we can send requests to sdev, assign one token
+ * and return the token else return -1.
  */
 static inline int scsi_dev_queue_ready(struct request_queue *q,
                                  struct scsi_device *sdev)
 {
-       unsigned int busy;
+       int token;
 
-       busy = atomic_inc_return(&sdev->device_busy) - 1;
+       token = sbitmap_get(&sdev->budget_map);
        if (atomic_read(&sdev->device_blocked)) {
-               if (busy)
+               if (token < 0)
+                       goto out;
+
+               if (scsi_device_busy(sdev) > 1)
                        goto out_dec;
 
                /*
@@ -1281,13 +1282,12 @@ static inline int scsi_dev_queue_ready(struct request_queue *q,
                                   "unblocking device at zero depth\n"));
        }
 
-       if (busy >= sdev->queue_depth)
-               goto out_dec;
-
-       return 1;
+       return token;
 out_dec:
-       atomic_dec(&sdev->device_busy);
-       return 0;
+       if (token >= 0)
+               sbitmap_put(&sdev->budget_map, token);
+out:
+       return -1;
 }
 
 /*
@@ -1612,15 +1612,16 @@ static void scsi_mq_put_budget(struct request_queue *q, int budget_token)
 {
        struct scsi_device *sdev = q->queuedata;
 
-       atomic_dec(&sdev->device_busy);
+       sbitmap_put(&sdev->budget_map, budget_token);
 }
 
 static int scsi_mq_get_budget(struct request_queue *q)
 {
        struct scsi_device *sdev = q->queuedata;
+       int token = scsi_dev_queue_ready(q, sdev);
 
-       if (scsi_dev_queue_ready(q, sdev))
-               return 0;
+       if (token >= 0)
+               return token;
 
        atomic_inc(&sdev->restarts);
 
index 180636d54982d0db670ecaf1b3b158f1eca08956..30b35002d2f8974a338ef050072931d65cc4b217 100644 (file)
@@ -5,6 +5,7 @@
 #include <linux/device.h>
 #include <linux/async.h>
 #include <scsi/scsi_device.h>
+#include <linux/sbitmap.h>
 
 struct request_queue;
 struct request;
@@ -182,6 +183,8 @@ static inline void scsi_dh_add_device(struct scsi_device *sdev) { }
 static inline void scsi_dh_release_device(struct scsi_device *sdev) { }
 #endif
 
+extern int scsi_device_max_queue_depth(struct scsi_device *sdev);
+
 /* 
  * internal scsi timeout functions: for use by mid-layer and transport
  * classes.
index 9af50e6f94c4c483ea26fc560bc758885b2397ed..9f1b7f3c650a01753db9a40c58e0c61a83feb343 100644 (file)
@@ -215,6 +215,7 @@ static void scsi_unlock_floptical(struct scsi_device *sdev,
 static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
                                           u64 lun, void *hostdata)
 {
+       unsigned int depth;
        struct scsi_device *sdev;
        int display_failure_msg = 1, ret;
        struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
@@ -276,8 +277,25 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
        WARN_ON_ONCE(!blk_get_queue(sdev->request_queue));
        sdev->request_queue->queuedata = sdev;
 
-       scsi_change_queue_depth(sdev, sdev->host->cmd_per_lun ?
-                                       sdev->host->cmd_per_lun : 1);
+       depth = sdev->host->cmd_per_lun ?: 1;
+
+       /*
+        * Use .can_queue as budget map's depth because we have to
+        * support adjusting queue depth from sysfs. Meantime use
+        * 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)) {
+               put_device(&starget->dev);
+               kfree(sdev);
+               goto out;
+       }
+
+       scsi_change_queue_depth(sdev, depth);
 
        scsi_sysfs_device_initialize(sdev);
 
@@ -979,6 +997,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
                scsi_attach_vpd(sdev);
 
        sdev->max_queue_depth = sdev->queue_depth;
+       WARN_ON_ONCE(sdev->max_queue_depth > sdev->budget_map.depth);
        sdev->sdev_bflags = *bflags;
 
        /*
index 0840e44140dee63778a1b6e3ba9ec2bda1166c60..7fb2f70e97c8ecb9f70f784bdee4ee1ceaf8911d 100644 (file)
@@ -477,6 +477,8 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
        /* NULL queue means the device can't be used */
        sdev->request_queue = NULL;
 
+       sbitmap_free(&sdev->budget_map);
+
        mutex_lock(&sdev->inquiry_mutex);
        vpd_pg0 = rcu_replace_pointer(sdev->vpd_pg0, vpd_pg0,
                                       lockdep_is_held(&sdev->inquiry_mutex));
index dd0b9f690a26e51ed0ce6121452dcfaf0299629c..05c7c320ef32fe2bd23691366de8cae14e719a5c 100644 (file)
@@ -8,6 +8,7 @@
 #include <linux/blkdev.h>
 #include <scsi/scsi.h>
 #include <linux/atomic.h>
+#include <linux/sbitmap.h>
 
 struct device;
 struct request_queue;
@@ -106,7 +107,7 @@ struct scsi_device {
        struct list_head    siblings;   /* list of all devices on this host */
        struct list_head    same_target_siblings; /* just the devices sharing same target id */
 
-       atomic_t device_busy;           /* commands actually active on LLDD */
+       struct sbitmap budget_map;
        atomic_t device_blocked;        /* Device returned QUEUE_FULL. */
 
        atomic_t restarts;
@@ -592,7 +593,7 @@ static inline int scsi_device_supports_vpd(struct scsi_device *sdev)
 
 static inline int scsi_device_busy(struct scsi_device *sdev)
 {
-       return atomic_read(&sdev->device_busy);
+       return sbitmap_weight(&sdev->budget_map);
 }
 
 #define MODULE_ALIAS_SCSI_DEVICE(type) \