Rusty,
I'm looking at porting my "enable tracepoints in module load" patches
and one of the comments you gave me (long ago) was to not have:
trace_foo=1
but to just have:
trace_foo
as a parameter name. I went and implemented this but discovered that the
functions that allow no arguments are hard coded in the params.c file.
I changed this to allow other "set" functions to be given no arguments,
and even noticed that a few already exist in the kernel. So I'm sending
you this patch set that implements a modification to the parameter
parsing to allow other kernel_param_ops to not bother with arguments
passed in.
What do you think?
-- Steve
Steven Rostedt (1):
tracing: Enable tracepoints via module parameters
Steven Rostedt (Red Hat) (3):
module: Add flag to allow mod params to have no arguments
module: Add NOARG flag for ops with param_set_bool_enable_only() set function
module/lsm: Have apparmor module parameters work with no args
----
include/linux/ftrace_event.h | 4 +++
include/linux/moduleparam.h | 13 +++++++-
include/trace/ftrace.h | 23 ++++++++++++--
kernel/module.c | 1 +
kernel/params.c | 6 ++--
kernel/trace/trace_events.c | 71 ++++++++++++++++++++++++++++++++++++++++++
security/apparmor/lsm.c | 2 ++
7 files changed, 115 insertions(+), 5 deletions(-)
---------------------------
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 120d57a..0395182 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -164,6 +164,8 @@ void tracing_record_cmdline(struct task_struct *tsk);
struct event_filter;
+extern struct kernel_param_ops ftrace_mod_ops;
+
enum trace_reg {
TRACE_REG_REGISTER,
TRACE_REG_UNREGISTER,
@@ -202,6 +204,7 @@ enum {
TRACE_EVENT_FL_NO_SET_FILTER_BIT,
TRACE_EVENT_FL_IGNORE_ENABLE_BIT,
TRACE_EVENT_FL_WAS_ENABLED_BIT,
+ TRACE_EVENT_FL_MOD_ENABLE_BIT,
};
/*
@@ -220,6 +223,7 @@ enum {
TRACE_EVENT_FL_NO_SET_FILTER = (1 << TRACE_EVENT_FL_NO_SET_FILTER_BIT),
TRACE_EVENT_FL_IGNORE_ENABLE = (1 << TRACE_EVENT_FL_IGNORE_ENABLE_BIT),
TRACE_EVENT_FL_WAS_ENABLED = (1 << TRACE_EVENT_FL_WAS_ENABLED_BIT),
+ TRACE_EVENT_FL_MOD_ENABLE = (1 << TRACE_EVENT_FL_MOD_ENABLE_BIT),
};
struct ftrace_event_call {
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 27d9da3..c3eb102 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -36,7 +36,18 @@ static const char __UNIQUE_ID(name)[] \
struct kernel_param;
+/*
+ * Flags available for kernel_param_ops
+ *
+ * NOARG - the parameter allows for no argument (foo instead of foo=1)
+ */
+enum {
+ KERNEL_PARAM_FL_NOARG = (1 << 0)
+};
+
struct kernel_param_ops {
+ /* How the ops should behave */
+ unsigned int flags;
/* Returns 0, or -errno. arg is in kp->arg. */
int (*set)(const char *val, const struct kernel_param *kp);
/* Returns length written or -errno. Buffer is 4k (ie. be short!) */
@@ -187,7 +198,7 @@ struct kparam_array
/* Obsolete - use module_param_cb() */
#define module_param_call(name, set, get, arg, perm) \
static struct kernel_param_ops __param_ops_##name = \
- { (void *)set, (void *)get }; \
+ { 0, (void *)set, (void *)get }; \
__module_param_call(MODULE_PARAM_PREFIX, \
name, &__param_ops_##name, arg, \
(perm) + sizeof(__check_old_set_param(set))*0, -1)
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 41a6643..d6029ed 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -17,6 +17,7 @@
*/
#include <linux/ftrace_event.h>
+#include <linux/module.h>
/*
* DECLARE_EVENT_CLASS can be used to add a generic function
@@ -577,6 +578,22 @@ static inline void ftrace_test_probe_##call(void) \
#undef __get_dynamic_array
#undef __get_str
+/*
+ * Add ftrace trace points in modules to be set by module
+ * parameters. This adds "trace_##call" as a module parameter.
+ * The user could enable trace points on module load with:
+ * trace_##call=1 as a module parameter.
+ */
+#undef ftrace_mod_params
+#ifdef MODULE
+#define ftrace_mod_params(call) \
+ module_param_cb(trace_##call, &ftrace_mod_ops, \
+ &event_##call, 0644); \
+ MODULE_INFO(tracepoint, #call)
+#else
+#define ftrace_mod_params(call)
+#endif
+
#undef TP_printk
#define TP_printk(fmt, args...) "\"" fmt "\", " __stringify(args)
@@ -604,7 +621,8 @@ static struct ftrace_event_call __used event_##call = { \
.print_fmt = print_fmt_##template, \
}; \
static struct ftrace_event_call __used \
-__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
+__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call;\
+ftrace_mod_params(call)
#undef DEFINE_EVENT_PRINT
#define DEFINE_EVENT_PRINT(template, call, proto, args, print) \
@@ -618,7 +636,8 @@ static struct ftrace_event_call __used event_##call = { \
.print_fmt = print_fmt_##call, \
}; \
static struct ftrace_event_call __used \
-__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
+__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call; \
+ftrace_mod_params(call)
#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
diff --git a/kernel/module.c b/kernel/module.c
index 2069158..4eb26b6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -136,6 +136,7 @@ static int param_set_bool_enable_only(const char *val,
}
static const struct kernel_param_ops param_ops_bool_enable_only = {
+ .flags = KERNEL_PARAM_FL_NOARG,
.set = param_set_bool_enable_only,
.get = param_get_bool,
};
diff --git a/kernel/params.c b/kernel/params.c
index 440e65d..27a0af9 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -103,8 +103,8 @@ static int parse_one(char *param,
|| params[i].level > max_level)
return 0;
/* No one handled NULL, so do it here. */
- if (!val && params[i].ops->set != param_set_bool
- && params[i].ops->set != param_set_bint)
+ if (!val &&
+ !(params[i].ops->flags & KERNEL_PARAM_FL_NOARG))
return -EINVAL;
pr_debug("handling %s with %p\n", param,
params[i].ops->set);
@@ -320,6 +320,7 @@ int param_get_bool(char *buffer, const struct kernel_param *kp)
EXPORT_SYMBOL(param_get_bool);
struct kernel_param_ops param_ops_bool = {
+ .flags = KERNEL_PARAM_FL_NOARG,
.set = param_set_bool,
.get = param_get_bool,
};
@@ -370,6 +371,7 @@ int param_set_bint(const char *val, const struct kernel_param *kp)
EXPORT_SYMBOL(param_set_bint);
struct kernel_param_ops param_ops_bint = {
+ .flags = KERNEL_PARAM_FL_NOARG,
.set = param_set_bint,
.get = param_get_int,
};
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 29a7ebc..5bd3f51 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1839,10 +1839,32 @@ trace_create_file_ops(struct module *mod)
return file_ops;
}
+static void
+enable_event_on_trace_array(struct trace_array *tr,
+ struct ftrace_event_call *call)
+{
+ struct ftrace_event_file *file;
+ int ret;
+
+ call->flags &= ~TRACE_EVENT_FL_MOD_ENABLE;
+
+ /* find event on top trace array */
+ list_for_each_entry(file, &tr->events, list) {
+ if (file->event_call == call) {
+ ret = ftrace_event_enable_disable(file, 1);
+ if (ret < 0)
+ pr_warning("unable to enable tracepoint %s",
+ call->name);
+ return;
+ }
+ }
+}
+
static void trace_module_add_events(struct module *mod)
{
struct ftrace_module_file_ops *file_ops = NULL;
struct ftrace_event_call **call, **start, **end;
+ struct trace_array *tr = NULL;
start = mod->trace_events;
end = mod->trace_events + mod->num_trace_events;
@@ -1857,6 +1879,12 @@ static void trace_module_add_events(struct module *mod)
for_each_event(call, start, end) {
__register_event(*call, mod);
__add_event_to_tracers(*call, file_ops);
+ /* If the module tracepoint parameter was set */
+ if ((*call)->flags & TRACE_EVENT_FL_MOD_ENABLE) {
+ if (!tr)
+ tr = top_trace_array();
+ enable_event_on_trace_array(tr, *call);
+ }
}
}
@@ -2569,6 +2597,49 @@ early_initcall(event_trace_memsetup);
core_initcall(event_trace_enable);
fs_initcall(event_trace_init);
+/* Allow modules to load with enabled trace points */
+int ftrace_mod_param_set(const char *val, const struct kernel_param *kp)
+{
+ struct ftrace_event_call *call = kp->arg;
+
+ /* This is set like param_set_bool() */
+
+ /* No equals means "set"... */
+ if (!val)
+ val = "1";
+
+ /* One of =[yYnN01] */
+ switch (val[0]) {
+ case 'y': case 'Y': case '1':
+ break;
+ case 'n': case 'N': case '0':
+ /* Do nothing */
+ return 0;
+ default:
+ return -EINVAL;
+ }
+
+ /* Set flag to tell ftrace to enable this event on init */
+ call->flags = TRACE_EVENT_FL_MOD_ENABLE;
+
+ return 0;
+}
+
+int ftrace_mod_param_get(char *buffer, const struct kernel_param *kp)
+{
+ struct ftrace_event_call *call = kp->arg;
+
+ return sprintf(buffer, "%d",
+ !!(call->flags & TRACE_EVENT_FL_MOD_ENABLE));
+}
+
+struct kernel_param_ops ftrace_mod_ops = {
+ .flags = KERNEL_PARAM_FL_NOARG,
+ .set = ftrace_mod_param_set,
+ .get = ftrace_mod_param_get,
+};
+EXPORT_SYMBOL(ftrace_mod_ops);
+
#ifdef CONFIG_FTRACE_STARTUP_TEST
static DEFINE_SPINLOCK(test_spinlock);
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 2e2a0dd..e3a704c 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -666,6 +666,7 @@ static int param_set_aabool(const char *val, const struct kernel_param *kp);
static int param_get_aabool(char *buffer, const struct kernel_param *kp);
#define param_check_aabool param_check_bool
static struct kernel_param_ops param_ops_aabool = {
+ .flags = KERNEL_PARAM_FL_NOARG,
.set = param_set_aabool,
.get = param_get_aabool
};
@@ -682,6 +683,7 @@ static int param_set_aalockpolicy(const char *val, const struct kernel_param *kp
static int param_get_aalockpolicy(char *buffer, const struct kernel_param *kp);
#define param_check_aalockpolicy param_check_bool
static struct kernel_param_ops param_ops_aalockpolicy = {
+ .flags = KERNEL_PARAM_FL_NOARG,
.set = param_set_aalockpolicy,
.get = param_get_aalockpolicy
};
On Tue, 13 Aug 2013 17:02:28 -0400
Steven Rostedt <[email protected]> wrote:
> Rusty,
>
> I'm looking at porting my "enable tracepoints in module load" patches
> and one of the comments you gave me (long ago) was to not have:
>
> trace_foo=1
>
> but to just have:
>
> trace_foo
>
> as a parameter name. I went and implemented this but discovered that the
> functions that allow no arguments are hard coded in the params.c file.
>
> I changed this to allow other "set" functions to be given no arguments,
> and even noticed that a few already exist in the kernel. So I'm sending
> you this patch set that implements a modification to the parameter
> parsing to allow other kernel_param_ops to not bother with arguments
> passed in.
>
> What do you think?
>
> -- Steve
>
> Steven Rostedt (1):
> tracing: Enable tracepoints via module parameters
>
OK, this is what I get for using my scripts along with manually sending
out patches via quilt. I only wanted to send out the three patches
below, but then used my scripts to make this header. The above patch
commit (along with the complete change set below) was not what I
intended on sending :-p
-- Steve
> Steven Rostedt (Red Hat) (3):
> module: Add flag to allow mod params to have no arguments
> module: Add NOARG flag for ops with param_set_bool_enable_only() set function
> module/lsm: Have apparmor module parameters work with no args
>
> ----
> include/linux/ftrace_event.h | 4 +++
> include/linux/moduleparam.h | 13 +++++++-
> include/trace/ftrace.h | 23 ++++++++++++--
> kernel/module.c | 1 +
> kernel/params.c | 6 ++--
> kernel/trace/trace_events.c | 71 ++++++++++++++++++++++++++++++++++++++++++
> security/apparmor/lsm.c | 2 ++
> 7 files changed, 115 insertions(+), 5 deletions(-)
> ---------------------------
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index 120d57a..0395182 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -164,6 +164,8 @@ void tracing_record_cmdline(struct task_struct *tsk);
>
> struct event_filter;
>
> +extern struct kernel_param_ops ftrace_mod_ops;
> +
> enum trace_reg {
> TRACE_REG_REGISTER,
> TRACE_REG_UNREGISTER,
> @@ -202,6 +204,7 @@ enum {
> TRACE_EVENT_FL_NO_SET_FILTER_BIT,
> TRACE_EVENT_FL_IGNORE_ENABLE_BIT,
> TRACE_EVENT_FL_WAS_ENABLED_BIT,
> + TRACE_EVENT_FL_MOD_ENABLE_BIT,
> };
>
> /*
> @@ -220,6 +223,7 @@ enum {
> TRACE_EVENT_FL_NO_SET_FILTER = (1 << TRACE_EVENT_FL_NO_SET_FILTER_BIT),
> TRACE_EVENT_FL_IGNORE_ENABLE = (1 << TRACE_EVENT_FL_IGNORE_ENABLE_BIT),
> TRACE_EVENT_FL_WAS_ENABLED = (1 << TRACE_EVENT_FL_WAS_ENABLED_BIT),
> + TRACE_EVENT_FL_MOD_ENABLE = (1 << TRACE_EVENT_FL_MOD_ENABLE_BIT),
> };
>
> struct ftrace_event_call {
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index 27d9da3..c3eb102 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -36,7 +36,18 @@ static const char __UNIQUE_ID(name)[] \
>
> struct kernel_param;
>
> +/*
> + * Flags available for kernel_param_ops
> + *
> + * NOARG - the parameter allows for no argument (foo instead of foo=1)
> + */
> +enum {
> + KERNEL_PARAM_FL_NOARG = (1 << 0)
> +};
> +
> struct kernel_param_ops {
> + /* How the ops should behave */
> + unsigned int flags;
> /* Returns 0, or -errno. arg is in kp->arg. */
> int (*set)(const char *val, const struct kernel_param *kp);
> /* Returns length written or -errno. Buffer is 4k (ie. be short!) */
> @@ -187,7 +198,7 @@ struct kparam_array
> /* Obsolete - use module_param_cb() */
> #define module_param_call(name, set, get, arg, perm) \
> static struct kernel_param_ops __param_ops_##name = \
> - { (void *)set, (void *)get }; \
> + { 0, (void *)set, (void *)get }; \
> __module_param_call(MODULE_PARAM_PREFIX, \
> name, &__param_ops_##name, arg, \
> (perm) + sizeof(__check_old_set_param(set))*0, -1)
> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> index 41a6643..d6029ed 100644
> --- a/include/trace/ftrace.h
> +++ b/include/trace/ftrace.h
> @@ -17,6 +17,7 @@
> */
>
> #include <linux/ftrace_event.h>
> +#include <linux/module.h>
>
> /*
> * DECLARE_EVENT_CLASS can be used to add a generic function
> @@ -577,6 +578,22 @@ static inline void ftrace_test_probe_##call(void) \
> #undef __get_dynamic_array
> #undef __get_str
>
> +/*
> + * Add ftrace trace points in modules to be set by module
> + * parameters. This adds "trace_##call" as a module parameter.
> + * The user could enable trace points on module load with:
> + * trace_##call=1 as a module parameter.
> + */
> +#undef ftrace_mod_params
> +#ifdef MODULE
> +#define ftrace_mod_params(call) \
> + module_param_cb(trace_##call, &ftrace_mod_ops, \
> + &event_##call, 0644); \
> + MODULE_INFO(tracepoint, #call)
> +#else
> +#define ftrace_mod_params(call)
> +#endif
> +
> #undef TP_printk
> #define TP_printk(fmt, args...) "\"" fmt "\", " __stringify(args)
>
> @@ -604,7 +621,8 @@ static struct ftrace_event_call __used event_##call = { \
> .print_fmt = print_fmt_##template, \
> }; \
> static struct ftrace_event_call __used \
> -__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
> +__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call;\
> +ftrace_mod_params(call)
>
> #undef DEFINE_EVENT_PRINT
> #define DEFINE_EVENT_PRINT(template, call, proto, args, print) \
> @@ -618,7 +636,8 @@ static struct ftrace_event_call __used event_##call = { \
> .print_fmt = print_fmt_##call, \
> }; \
> static struct ftrace_event_call __used \
> -__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
> +__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call; \
> +ftrace_mod_params(call)
>
> #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 2069158..4eb26b6 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -136,6 +136,7 @@ static int param_set_bool_enable_only(const char *val,
> }
>
> static const struct kernel_param_ops param_ops_bool_enable_only = {
> + .flags = KERNEL_PARAM_FL_NOARG,
> .set = param_set_bool_enable_only,
> .get = param_get_bool,
> };
> diff --git a/kernel/params.c b/kernel/params.c
> index 440e65d..27a0af9 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -103,8 +103,8 @@ static int parse_one(char *param,
> || params[i].level > max_level)
> return 0;
> /* No one handled NULL, so do it here. */
> - if (!val && params[i].ops->set != param_set_bool
> - && params[i].ops->set != param_set_bint)
> + if (!val &&
> + !(params[i].ops->flags & KERNEL_PARAM_FL_NOARG))
> return -EINVAL;
> pr_debug("handling %s with %p\n", param,
> params[i].ops->set);
> @@ -320,6 +320,7 @@ int param_get_bool(char *buffer, const struct kernel_param *kp)
> EXPORT_SYMBOL(param_get_bool);
>
> struct kernel_param_ops param_ops_bool = {
> + .flags = KERNEL_PARAM_FL_NOARG,
> .set = param_set_bool,
> .get = param_get_bool,
> };
> @@ -370,6 +371,7 @@ int param_set_bint(const char *val, const struct kernel_param *kp)
> EXPORT_SYMBOL(param_set_bint);
>
> struct kernel_param_ops param_ops_bint = {
> + .flags = KERNEL_PARAM_FL_NOARG,
> .set = param_set_bint,
> .get = param_get_int,
> };
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 29a7ebc..5bd3f51 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1839,10 +1839,32 @@ trace_create_file_ops(struct module *mod)
> return file_ops;
> }
>
> +static void
> +enable_event_on_trace_array(struct trace_array *tr,
> + struct ftrace_event_call *call)
> +{
> + struct ftrace_event_file *file;
> + int ret;
> +
> + call->flags &= ~TRACE_EVENT_FL_MOD_ENABLE;
> +
> + /* find event on top trace array */
> + list_for_each_entry(file, &tr->events, list) {
> + if (file->event_call == call) {
> + ret = ftrace_event_enable_disable(file, 1);
> + if (ret < 0)
> + pr_warning("unable to enable tracepoint %s",
> + call->name);
> + return;
> + }
> + }
> +}
> +
> static void trace_module_add_events(struct module *mod)
> {
> struct ftrace_module_file_ops *file_ops = NULL;
> struct ftrace_event_call **call, **start, **end;
> + struct trace_array *tr = NULL;
>
> start = mod->trace_events;
> end = mod->trace_events + mod->num_trace_events;
> @@ -1857,6 +1879,12 @@ static void trace_module_add_events(struct module *mod)
> for_each_event(call, start, end) {
> __register_event(*call, mod);
> __add_event_to_tracers(*call, file_ops);
> + /* If the module tracepoint parameter was set */
> + if ((*call)->flags & TRACE_EVENT_FL_MOD_ENABLE) {
> + if (!tr)
> + tr = top_trace_array();
> + enable_event_on_trace_array(tr, *call);
> + }
> }
> }
>
> @@ -2569,6 +2597,49 @@ early_initcall(event_trace_memsetup);
> core_initcall(event_trace_enable);
> fs_initcall(event_trace_init);
>
> +/* Allow modules to load with enabled trace points */
> +int ftrace_mod_param_set(const char *val, const struct kernel_param *kp)
> +{
> + struct ftrace_event_call *call = kp->arg;
> +
> + /* This is set like param_set_bool() */
> +
> + /* No equals means "set"... */
> + if (!val)
> + val = "1";
> +
> + /* One of =[yYnN01] */
> + switch (val[0]) {
> + case 'y': case 'Y': case '1':
> + break;
> + case 'n': case 'N': case '0':
> + /* Do nothing */
> + return 0;
> + default:
> + return -EINVAL;
> + }
> +
> + /* Set flag to tell ftrace to enable this event on init */
> + call->flags = TRACE_EVENT_FL_MOD_ENABLE;
> +
> + return 0;
> +}
> +
> +int ftrace_mod_param_get(char *buffer, const struct kernel_param *kp)
> +{
> + struct ftrace_event_call *call = kp->arg;
> +
> + return sprintf(buffer, "%d",
> + !!(call->flags & TRACE_EVENT_FL_MOD_ENABLE));
> +}
> +
> +struct kernel_param_ops ftrace_mod_ops = {
> + .flags = KERNEL_PARAM_FL_NOARG,
> + .set = ftrace_mod_param_set,
> + .get = ftrace_mod_param_get,
> +};
> +EXPORT_SYMBOL(ftrace_mod_ops);
> +
> #ifdef CONFIG_FTRACE_STARTUP_TEST
>
> static DEFINE_SPINLOCK(test_spinlock);
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 2e2a0dd..e3a704c 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -666,6 +666,7 @@ static int param_set_aabool(const char *val, const struct kernel_param *kp);
> static int param_get_aabool(char *buffer, const struct kernel_param *kp);
> #define param_check_aabool param_check_bool
> static struct kernel_param_ops param_ops_aabool = {
> + .flags = KERNEL_PARAM_FL_NOARG,
> .set = param_set_aabool,
> .get = param_get_aabool
> };
> @@ -682,6 +683,7 @@ static int param_set_aalockpolicy(const char *val, const struct kernel_param *kp
> static int param_get_aalockpolicy(char *buffer, const struct kernel_param *kp);
> #define param_check_aalockpolicy param_check_bool
> static struct kernel_param_ops param_ops_aalockpolicy = {
> + .flags = KERNEL_PARAM_FL_NOARG,
> .set = param_set_aalockpolicy,
> .get = param_get_aalockpolicy
> };
On Tue, Aug 13, 2013 at 6:02 PM, Steven Rostedt <[email protected]> wrote:
> Rusty,
>
> I'm looking at porting my "enable tracepoints in module load" patches
> and one of the comments you gave me (long ago) was to not have:
>
> trace_foo=1
>
> but to just have:
>
> trace_foo
>
> as a parameter name. I went and implemented this but discovered that the
> functions that allow no arguments are hard coded in the params.c file.
>
> I changed this to allow other "set" functions to be given no arguments,
> and even noticed that a few already exist in the kernel. So I'm sending
> you this patch set that implements a modification to the parameter
> parsing to allow other kernel_param_ops to not bother with arguments
> passed in.
>
> What do you think?
so in kcmdline we would have modulename.param instead of modulename.param=1?
I guess we need to update kmod then, because currently we ignore and
treat this case as a wrong token. From a quick look, allowing it in
kmod would be as simple as removing a condition check.
Lucas De Marchi
On Tue, 13 Aug 2013 20:34:58 -0300
Lucas De Marchi <[email protected]> wrote:
> so in kcmdline we would have modulename.param instead of modulename.param=1?
>
> I guess we need to update kmod then, because currently we ignore and
> treat this case as a wrong token. From a quick look, allowing it in
> kmod would be as simple as removing a condition check.
>
> Lucas De Marchi
Note, both will still work. And it didn't change much. Today, anything
that uses "module_param()" with bool type (a quick git grep shows 570
users), already do not require a value.
Randomly looking at one... drivers/input/mouse/synaptics_i2c.c, you can
just do:
insmod synaptics_i2c.ko no_filter
no need to add a "=1" to that.
But anything else will still require a value. I just want to allow
other parameters that act like a boolean to not require one.
-- Steve
On Tue, Aug 13, 2013 at 9:17 PM, Steven Rostedt <[email protected]> wrote:
> On Tue, 13 Aug 2013 20:34:58 -0300
> Lucas De Marchi <[email protected]> wrote:
>
>
>> so in kcmdline we would have modulename.param instead of modulename.param=1?
>>
>> I guess we need to update kmod then, because currently we ignore and
>> treat this case as a wrong token. From a quick look, allowing it in
>> kmod would be as simple as removing a condition check.
>>
>> Lucas De Marchi
>
> Note, both will still work. And it didn't change much. Today, anything
> that uses "module_param()" with bool type (a quick git grep shows 570
> users), already do not require a value.
>
> Randomly looking at one... drivers/input/mouse/synaptics_i2c.c, you can
> just do:
>
> insmod synaptics_i2c.ko no_filter
>
> no need to add a "=1" to that.
>
> But anything else will still require a value. I just want to allow
> other parameters that act like a boolean to not require one.
true... but currently "modprobe synaptics_i2c" doesn't get the
parameter correctly from kernel command line if it doesn't have a
value. And I agree this not something that changed but rather a bug in
kmod waiting to be fixed.
Lucas De Marchi
On Tue, Aug 13, 2013 at 10:00 PM, Lucas De Marchi
<[email protected]> wrote:
> On Tue, Aug 13, 2013 at 9:17 PM, Steven Rostedt <[email protected]> wrote:
>> On Tue, 13 Aug 2013 20:34:58 -0300
>> Lucas De Marchi <[email protected]> wrote:
>>
>>
>>> so in kcmdline we would have modulename.param instead of modulename.param=1?
>>>
>>> I guess we need to update kmod then, because currently we ignore and
>>> treat this case as a wrong token. From a quick look, allowing it in
>>> kmod would be as simple as removing a condition check.
>>>
>>> Lucas De Marchi
>>
>> Note, both will still work. And it didn't change much. Today, anything
>> that uses "module_param()" with bool type (a quick git grep shows 570
>> users), already do not require a value.
>>
>> Randomly looking at one... drivers/input/mouse/synaptics_i2c.c, you can
>> just do:
>>
>> insmod synaptics_i2c.ko no_filter
>>
>> no need to add a "=1" to that.
>>
>> But anything else will still require a value. I just want to allow
>> other parameters that act like a boolean to not require one.
>
> true... but currently "modprobe synaptics_i2c" doesn't get the
> parameter correctly from kernel command line if it doesn't have a
> value. And I agree this not something that changed but rather a bug in
> kmod waiting to be fixed.
And it's fixed now with a proper test added.
thanks
Lucas De Marchi