diff mbox series

[V2,4/4] loop: allow user to set the queue depth

Message ID 20220215115104.11429-5-kch@nvidia.com (mailing list archive)
State New, archived
Headers show
Series loop: cleanup and few improvements | expand

Commit Message

Chaitanya Kulkarni Feb. 15, 2022, 11:51 a.m. UTC 
Instead of hardcoding queue depth allow user to set the hw queue depth
using module parameter. Set default value to 128 to retain the existing
behavior.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
---
 drivers/block/loop.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Jens Axboe Feb. 15, 2022, 2:54 p.m. UTC | #1 
On 2/15/22 4:51 AM, Chaitanya Kulkarni wrote:
> Instead of hardcoding queue depth allow user to set the hw queue depth
> using module parameter. Set default value to 128 to retain the existing
> behavior.

Do we want to ensure that the depth falls within a reasonable range? I
guess we don't if we just ensure that it fails gracefully. Eg have you
tried the usual sanity checks of -1, 0, 1, huge?
Chaitanya Kulkarni Feb. 15, 2022, 4:54 p.m. UTC | #2 
On 2/15/22 06:54, Jens Axboe wrote:
> On 2/15/22 4:51 AM, Chaitanya Kulkarni wrote:
>> Instead of hardcoding queue depth allow user to set the hw queue depth
>> using module parameter. Set default value to 128 to retain the existing
>> behavior.
> 
> Do we want to ensure that the depth falls within a reasonable range? I
> guess we don't if we just ensure that it fails gracefully. Eg have you
> tried the usual sanity checks of -1, 0, 1, huge?
>

Yes we should, we can error out for values < 1 see [1], but what should
be the reasonable range for any positive or huge values ? since block
layer will reduce the depth if it cannot incorporate huge values to the
ones which it can in blk_mq_alloc_set_map_and_rqs().

[1] loop hw_queue_depth parameter validation incremental patch
     untested :-

root@dev linux-block (for-next) # git diff
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index fd2184d63c11..c9a732a22767 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -85,6 +85,7 @@
  #include <linux/uaccess.h>

  #define LOOP_IDLE_WORKER_TIMEOUT (60 * HZ)
+#define LOOP_DEFAULT_HW_Q_DEPTH (128)

  static DEFINE_IDR(loop_index_idr);
  static DEFINE_MUTEX(loop_ctl_mutex);
@@ -1785,9 +1786,24 @@ module_param(max_loop, int, 0444);
  MODULE_PARM_DESC(max_loop, "Maximum number of loop devices");
  module_param(max_part, int, 0444);
  MODULE_PARM_DESC(max_part, "Maximum number of partitions per loop 
device");
-static int hw_queue_depth = 128;
-module_param_named(hw_queue_depth, hw_queue_depth, int, 0444);
+
+static int hw_queue_depth = LOOP_DEFAULT_HW_Q_DEPTH;
+
+static int loop_set_hw_queue_depth(const char *s, const struct 
kernel_param *p)
+{
+       int ret = kstrtoint(s, 10, &hw_queue_depth);
+
+       return (ret || (hw_queue_depth < 1) ? -EINVAL : 0;
+}
+
+static const struct kernel_param_ops loop_hw_qdepth_param_ops = {
+       .set    = loop_set_hw_queue_depth,
+       .get    = param_get_int,
+};
+
+device_param_cb(hw_queue_depth, &loop_hw_qdepth_param_ops, 
&hw_queue_depth, 0444);
  MODULE_PARM_DESC(hw_queue_depth, "Queue depth for each hardware queue. 
Default: 128");
+

-ck
diff mbox series

Patch

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 18b30a56bfc4..fd2184d63c11 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1785,6 +1785,9 @@  module_param(max_loop, int, 0444);
 MODULE_PARM_DESC(max_loop, "Maximum number of loop devices");
 module_param(max_part, int, 0444);
 MODULE_PARM_DESC(max_part, "Maximum number of partitions per loop device");
+static int hw_queue_depth = 128;
+module_param_named(hw_queue_depth, hw_queue_depth, int, 0444);
+MODULE_PARM_DESC(hw_queue_depth, "Queue depth for each hardware queue. Default: 128");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS_BLOCKDEV_MAJOR(LOOP_MAJOR);
 
@@ -1979,7 +1982,7 @@  static int loop_add(int i)
 
 	lo->tag_set.ops = &loop_mq_ops;
 	lo->tag_set.nr_hw_queues = 1;
-	lo->tag_set.queue_depth = 128;
+	lo->tag_set.queue_depth = hw_queue_depth;
 	lo->tag_set.numa_node = NUMA_NO_NODE;
 	lo->tag_set.cmd_size = sizeof(struct loop_cmd);
 	lo->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_STACKING |