On 4/10/23 14:13, Chaitanya Kulkarni wrote: > 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); INT_MAX ? That is a little large... Why not using CONFIG_NR_CPUS ? Or CONFIG_NR_CPUS * 2 to leave some room for experiments. > +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_bs = 512; > -module_param_named(bs, g_bs, int, 0444); > +NULL_PARAM(bs, 1, PAGE_SIZE); Allowing min=1 here does not make sense. This should be 512. And you could add a "bool is_power_of_2" argument to null_param_store_int() to check that the value is a power of 2. > +device_param_cb(bs, &null_bs_param_ops, &g_bs, 0444); > MODULE_PARM_DESC(bs, "Block size (in bytes)"); [...] > 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); INT_MAX here is a little silly. Way too large to make any sense in my opinion. NVMe allows up to 65536, why not use the same value here ? > +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;