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

On 2/15/22 06:54, Jens Axboe wrote:

On 2/15/22 4:51 AM, Chaitanya Kulkarni wrote:

quoted

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