Commit e152a05f authored by Bart Van Assche's avatar Bart Van Assche Committed by Jens Axboe

loop: Improve the hw_queue_depth kernel module parameter implementation

Make the following minor changes which were reported by colleagues
while reviewing this code:
- Remove the parentheses from around the LOOP_DEFAULT_HW_Q_DEPTH
  definition since these are superfluous.
- Accept other number formats than decimal, e.g. hexadecimal.
- Do not set hw_queue_depth to an out-of-range value, even if that value
  won't be used.
- Use the LOOP_DEFAULT_HW_Q_DEPTH macro in the kernel module parameter
  description to prevent that the description gets out of sync.

This patch has been tested as follows:

 # modprobe -r loop
 # modprobe loop hw_queue_depth=-1
 modprobe: ERROR: could not insert 'loop': Invalid argument
 # modprobe loop hw_queue_depth=0
 modprobe: ERROR: could not insert 'loop': Invalid argument
 # modprobe loop hw_queue_depth=1; cat /sys/module/loop/parameters/hw_queue_depth
 1
 # modprobe -r loop; modprobe loop; cat /sys/module/loop/parameters/hw_queue_depth hw_queue_depth=0x10
 16
 # modprobe -r loop; modprobe loop; cat /sys/module/loop/parameters/hw_queue_depth hw_queue_depth=128
 128
 # modprobe -r loop; modprobe loop hw_queue_depth=129; cat /sys/module/loop/parameters/hw_queue_depth
 129
 # modprobe -r loop; modprobe loop hw_queue_depth=$((1<<32))
 modprobe: ERROR: could not insert 'loop': Numerical result out of range

See also commit ef44c508 ("loop: allow user to set the queue
depth").

Cc: Chaitanya Kulkarni <kch@nvidia.com>
Cc: Himanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: default avatarBart Van Assche <bvanassche@acm.org>
Reviewed-by: default avatarChaitanya Kulkarni <kch@nvidia.com>
Link: https://lore.kernel.org/r/20230130211347.832110-1-bvanassche@acm.orgSigned-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 2d97930d
...@@ -90,7 +90,7 @@ struct loop_cmd { ...@@ -90,7 +90,7 @@ struct loop_cmd {
}; };
#define LOOP_IDLE_WORKER_TIMEOUT (60 * HZ) #define LOOP_IDLE_WORKER_TIMEOUT (60 * HZ)
#define LOOP_DEFAULT_HW_Q_DEPTH (128) #define LOOP_DEFAULT_HW_Q_DEPTH 128
static DEFINE_IDR(loop_index_idr); static DEFINE_IDR(loop_index_idr);
static DEFINE_MUTEX(loop_ctl_mutex); static DEFINE_MUTEX(loop_ctl_mutex);
...@@ -1792,9 +1792,15 @@ static int hw_queue_depth = LOOP_DEFAULT_HW_Q_DEPTH; ...@@ -1792,9 +1792,15 @@ 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) static int loop_set_hw_queue_depth(const char *s, const struct kernel_param *p)
{ {
int ret = kstrtoint(s, 10, &hw_queue_depth); int qd, ret;
return (ret || (hw_queue_depth < 1)) ? -EINVAL : 0; ret = kstrtoint(s, 0, &qd);
if (ret < 0)
return ret;
if (qd < 1)
return -EINVAL;
hw_queue_depth = qd;
return 0;
} }
static const struct kernel_param_ops loop_hw_qdepth_param_ops = { static const struct kernel_param_ops loop_hw_qdepth_param_ops = {
...@@ -1803,7 +1809,7 @@ static const struct kernel_param_ops loop_hw_qdepth_param_ops = { ...@@ -1803,7 +1809,7 @@ static const struct kernel_param_ops loop_hw_qdepth_param_ops = {
}; };
device_param_cb(hw_queue_depth, &loop_hw_qdepth_param_ops, &hw_queue_depth, 0444); 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"); MODULE_PARM_DESC(hw_queue_depth, "Queue depth for each hardware queue. Default: " __stringify(LOOP_DEFAULT_HW_Q_DEPTH));
MODULE_LICENSE("GPL"); MODULE_LICENSE("GPL");
MODULE_ALIAS_BLOCKDEV_MAJOR(LOOP_MAJOR); MODULE_ALIAS_BLOCKDEV_MAJOR(LOOP_MAJOR);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment