On 23/04/09 10:13PM, Chaitanya Kulkarni wrote:
Move null_param_store_int() function to the top of the code so that
it can be accessed by the new NULL_PARAM() macro. The macro takes
name of the module parameter, minimum value, and maximum value as
input, then creates specific callback functions and kernel_param
ops for each module parameter. This macro ultimately calls the
null_param_store_int() function, which returns an error if the
user-provided value is out of bounds(min,max).
To prevent negative values from being set by the user for following
list of module parameters, uses NULL_PARM() macro to add user input
validation:
submit_queues
poll_queues
queue_mode
gb
max_sectors
irqmode
hw_qdepth
bs
This commit improves the organization and safety of the code, making
it easier to maintain and preventing users from accidentally setting
negative values for the specified parameters.
Signed-off-by: Chaitanya Kulkarni <kch@xxxxxxxxxx>
---
drivers/block/null_blk/main.c | 85 +++++++++++++++++------------------
1 file changed, 41 insertions(+), 44 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index bc2c58724df3..fcb2ce4f9dc8 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -77,6 +77,33 @@ enum {
NULL_IRQ_TIMER = 2,
};
+static int null_param_store_int(const char *str, int *val, int min, int max)
+{
+ int ret, new_val;
+
+ ret = kstrtoint(str, 10, &new_val);
+ if (ret)
+ return -EINVAL;
+
+ if (new_val < min || new_val > max)
+ return -EINVAL;
+
+ *val = new_val;
+ return 0;
+}
+
+#define NULL_PARAM(_name, _min, _max) \
+static int null_param_##_name##_set(const char *s, \
+ const struct kernel_param *kp) \
+{ \
+ return null_param_store_int(s, kp->arg, _min, _max); \
+} \
+ \
+static const struct kernel_param_ops null_##_name##_param_ops = { \
+ .set = null_param_##_name##_set, \
+ .get = param_get_int, \
+}
+
static bool g_virt_boundary = false;
module_param_named(virt_boundary, g_virt_boundary, bool, 0444);
MODULE_PARM_DESC(virt_boundary, "Require a virtual boundary for the device. Default: False");
@@ -86,11 +113,13 @@ module_param_named(no_sched, g_no_sched, int, 0444);
MODULE_PARM_DESC(no_sched, "No io scheduler");
static int g_submit_queues = 1;
-module_param_named(submit_queues, g_submit_queues, int, 0444);
+NULL_PARAM(submit_queues, 1, INT_MAX);
+device_param_cb(submit_queues, &null_submit_queues_param_ops, &g_submit_queues, 0444);
MODULE_PARM_DESC(submit_queues, "Number of submission queues");
static int g_poll_queues = 1;
-module_param_named(poll_queues, g_poll_queues, int, 0444);
+NULL_PARAM(poll_queues, 1, num_online_cpus());
+device_param_cb(poll_queues, &null_poll_queues_param_ops, &g_poll_queues, 0444);
MODULE_PARM_DESC(poll_queues, "Number of IOPOLL submission queues");
static int g_home_node = NUMA_NO_NODE;
@@ -116,45 +145,23 @@ MODULE_PARM_DESC(init_hctx, "Fault injection to fail hctx init. init_hctx=<inter
#endif
static int g_queue_mode = NULL_Q_MQ;
-
-static int null_param_store_val(const char *str, int *val, int min, int max)
-{
- int ret, new_val;
-
- ret = kstrtoint(str, 10, &new_val);
- if (ret)
- return -EINVAL;
-
- if (new_val < min || new_val > max)
- return -EINVAL;
-
- *val = new_val;
- return 0;
-}
-
-static int null_set_queue_mode(const char *str, const struct kernel_param *kp)
-{
- return null_param_store_val(str, &g_queue_mode, NULL_Q_BIO, NULL_Q_MQ);
-}
-
-static const struct kernel_param_ops null_queue_mode_param_ops = {
- .set = null_set_queue_mode,
- .get = param_get_int,
-};
-
+NULL_PARAM(queue_mode, NULL_Q_BIO, NULL_Q_MQ);
device_param_cb(queue_mode, &null_queue_mode_param_ops, &g_queue_mode, 0444);
MODULE_PARM_DESC(queue_mode, "Block interface to use (0=bio,1=rq,2=multiqueue)");
static int g_gb = 250;
-module_param_named(gb, g_gb, int, 0444);
+NULL_PARAM(gb, 1, INT_MAX);
This value gets converted to mb, for dev->size calculation in
null_alloc_dev. I think either there should be a type conversion or
this module parameter max value can be reduced to smaller value.
+device_param_cb(gb, &null_gb_param_ops, &g_gb, 0444);
MODULE_PARM_DESC(gb, "Size in GB");
static int g_bs = 512;
-module_param_named(bs, g_bs, int, 0444);
+NULL_PARAM(bs, 1, PAGE_SIZE);
+device_param_cb(bs, &null_bs_param_ops, &g_bs, 0444);
MODULE_PARM_DESC(bs, "Block size (in bytes)");
static int g_max_sectors;
-module_param_named(max_sectors, g_max_sectors, int, 0444);
+NULL_PARAM(max_sectors, 1, INT_MAX);
+device_param_cb(max_sectors, &null_max_sectors_param_ops, &g_max_sectors, 0444);
MODULE_PARM_DESC(max_sectors, "Maximum size of a command (in 512B sectors)");
static unsigned int nr_devices = 1;
@@ -174,18 +181,7 @@ module_param_named(shared_tag_bitmap, g_shared_tag_bitmap, bool, 0444);
MODULE_PARM_DESC(shared_tag_bitmap, "Use shared tag bitmap for all submission queues for blk-mq");
static int g_irqmode = NULL_IRQ_SOFTIRQ;
-
-static int null_set_irqmode(const char *str, const struct kernel_param *kp)
-{
- return null_param_store_val(str, &g_irqmode, NULL_IRQ_NONE,
- NULL_IRQ_TIMER);
-}
-
-static const struct kernel_param_ops null_irqmode_param_ops = {
- .set = null_set_irqmode,
- .get = param_get_int,
-};
-
+NULL_PARAM(irqmode, NULL_IRQ_NONE, NULL_IRQ_TIMER);
device_param_cb(irqmode, &null_irqmode_param_ops, &g_irqmode, 0444);
MODULE_PARM_DESC(irqmode, "IRQ completion handler. 0-none, 1-softirq, 2-timer");
@@ -194,7 +190,8 @@ module_param_named(completion_nsec, g_completion_nsec, ulong, 0444);
MODULE_PARM_DESC(completion_nsec, "Time in ns to complete a request in hardware. Default: 10,000ns");
static int g_hw_queue_depth = 64;
-module_param_named(hw_queue_depth, g_hw_queue_depth, int, 0444);
+NULL_PARAM(hw_qdepth, 1, INT_MAX);
+device_param_cb(hw_queue_depth, &null_hw_qdepth_param_ops, &g_hw_queue_depth, 0444);
MODULE_PARM_DESC(hw_queue_depth, "Queue depth for each hardware queue. Default: 64");
static bool g_use_per_node_hctx;
--
Regards,
Nitesh Shetty
2.29.0
