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

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

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