All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chaitanya Kulkarni <kch@nvidia.com>
To: <linux-block@vger.kernel.org>
Cc: <axboe@kernel.dk>, <kch@nvidia.com>,
	<damien.lemoal@opensource.wdc.com>, <johannes.thumshirn@wdc.com>,
	<ming.lei@redhat.com>, <bvanassche@acm.org>,
	<shinichiro.kawasaki@wdc.com>, <vincent.fu@samsung.com>
Subject: [PATCH V2 1/1] null_blk: add moddule parameter check
Date: Sun, 9 Apr 2023 22:13:52 -0700	[thread overview]
Message-ID: <20230410051352.36856-2-kch@nvidia.com> (raw)
In-Reply-To: <20230410051352.36856-1-kch@nvidia.com>

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@nvidia.com>
---
 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);
+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;
-- 
2.29.0


  reply	other threads:[~2023-04-10  5:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-10  5:13 [PATCH V2 0/1 null_blk: add modparam checks Chaitanya Kulkarni
2023-04-10  5:13 ` Chaitanya Kulkarni [this message]
2023-04-10  5:58   ` [PATCH V2 1/1] null_blk: add moddule parameter check Damien Le Moal
2023-04-10 17:47   ` Nitesh Shetty
2023-04-11  5:29     ` Damien Le Moal
2023-04-11  6:40       ` Chaitanya Kulkarni
2023-04-11  7:02         ` Damien Le Moal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230410051352.36856-2-kch@nvidia.com \
    --to=kch@nvidia.com \
    --cc=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=johannes.thumshirn@wdc.com \
    --cc=linux-block@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=shinichiro.kawasaki@wdc.com \
    --cc=vincent.fu@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.