2016-11-15 02:00:23

by Paul Gortmaker

[permalink] [raw]
Subject: [PATCH -RFC] moduleparam: introduce core_param_named macro for non-modular code

We have the case where module_param_named() in file "foo.c" for
parameter myparam translates that into the bootarg for the
non-modular use case as "foo.myparam=..."

The problem exists where the use case with the filename and the
dot prefix is established, but the code is then realized to be 100%
non-modular, or is converted to non-modular. Both of the existing
macros like core_param() or setup_param() do not append such a
prefix, so a straight conversion to either will break the existing
use cases.

Similarly, trying to embed a hard coded "foo." prefix on the name
fails cpp syntax due to the special nature of "." in code. So we add
this parallel variant for the modular --> non-modular transition to
preserve existing and documented use cases with such a prefix.

Cc: Jessica Yu <[email protected]>
Cc: Rusty Russell <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---

[Marking this RFC since I don't like the fact that it still requires
non-modular code to use moduleparam.h -- one possible fix for that is
to consider moving non-modular macros to a new param.h or similar. ]

include/linux/moduleparam.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 52666d90ca94..4f2b92345eb5 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -269,6 +269,23 @@ static inline void kernel_param_unlock(struct module *mod)
__module_param_call("", name, &param_ops_##type, &var, perm, -1, 0)

/**
+ * core_param_named - define a module compat core kernel parameter.
+ * @name: the name of the cmdline and sysfs parameter (often the same as var)
+ * @var: the variable
+ * @type: the type of the parameter
+ * @perm: visibility in sysfs
+ *
+ * core_param_named is just like module_param_named(), but cannot be modular
+ * and it _does_ add a prefix (such as "printk."). This is for compatibility
+ * with module_param_named(), and it exists to provide boot arg compatibility
+ * with code that was previously using the modular version with the prefix.
+ */
+#define core_param_named(name, var, type, perm) \
+ param_check_##type(name, &(var)); \
+ __module_param_call(KBUILD_MODNAME ".", name, &param_ops_##type,\
+ &var, perm, -1, 0)
+
+/**
* core_param_unsafe - same as core_param but taints kernel
*/
#define core_param_unsafe(name, var, type, perm) \
--
2.10.1


2016-11-16 02:50:59

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH -RFC] moduleparam: introduce core_param_named macro for non-modular code

Paul Gortmaker <[email protected]> writes:
> We have the case where module_param_named() in file "foo.c" for
> parameter myparam translates that into the bootarg for the
> non-modular use case as "foo.myparam=..."
>
> The problem exists where the use case with the filename and the
> dot prefix is established, but the code is then realized to be 100%
> non-modular, or is converted to non-modular. Both of the existing
> macros like core_param() or setup_param() do not append such a
> prefix, so a straight conversion to either will break the existing
> use cases.

IMHO you should keep using moduleparam.

I originally called everything simply param(), but there was a name
clash.

Linus' answer was basically that "everything is a module, even if it's
not a .ko". And it's his tree, so he must be right!

Cheers,
Rusty.

2016-11-21 07:37:50

by Jessica Yu

[permalink] [raw]
Subject: Re: moduleparam: introduce core_param_named macro for non-modular code

+++ Paul Gortmaker [14/11/16 21:00 -0500]:
>We have the case where module_param_named() in file "foo.c" for
>parameter myparam translates that into the bootarg for the
>non-modular use case as "foo.myparam=..."
>
>The problem exists where the use case with the filename and the
>dot prefix is established, but the code is then realized to be 100%
>non-modular, or is converted to non-modular. Both of the existing
>macros like core_param() or setup_param() do not append such a
>prefix, so a straight conversion to either will break the existing
>use cases.
>
>Similarly, trying to embed a hard coded "foo." prefix on the name
>fails cpp syntax due to the special nature of "." in code. So we add
>this parallel variant for the modular --> non-modular transition to
>preserve existing and documented use cases with such a prefix.

Hm, I'm not convinced we need a core_ counterpart to module_param_named
(that's nearly identical), when module_param_named already implements
all of the above. Plenty of non-modular code already use it (e.g.
workqueue, printk), and a prefix is automatically supplied (which can be
overridden) in the non-modular case. That should already meet your
requirements, no?

>Cc: Jessica Yu <[email protected]>
>Cc: Rusty Russell <[email protected]>
>Signed-off-by: Paul Gortmaker <[email protected]>
>---
>
>[Marking this RFC since I don't like the fact that it still requires
> non-modular code to use moduleparam.h -- one possible fix for that is
> to consider moving non-modular macros to a new param.h or similar. ]
>
> include/linux/moduleparam.h | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
>diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
>index 52666d90ca94..4f2b92345eb5 100644
>--- a/include/linux/moduleparam.h
>+++ b/include/linux/moduleparam.h
>@@ -269,6 +269,23 @@ static inline void kernel_param_unlock(struct module *mod)
> __module_param_call("", name, &param_ops_##type, &var, perm, -1, 0)
>
> /**
>+ * core_param_named - define a module compat core kernel parameter.
>+ * @name: the name of the cmdline and sysfs parameter (often the same as var)
>+ * @var: the variable
>+ * @type: the type of the parameter
>+ * @perm: visibility in sysfs
>+ *
>+ * core_param_named is just like module_param_named(), but cannot be modular
>+ * and it _does_ add a prefix (such as "printk."). This is for compatibility
>+ * with module_param_named(), and it exists to provide boot arg compatibility
>+ * with code that was previously using the modular version with the prefix.
>+ */
>+#define core_param_named(name, var, type, perm) \
>+ param_check_##type(name, &(var)); \
>+ __module_param_call(KBUILD_MODNAME ".", name, &param_ops_##type,\
>+ &var, perm, -1, 0)
>+
>+/**
> * core_param_unsafe - same as core_param but taints kernel
> */
> #define core_param_unsafe(name, var, type, perm) \
>--
>2.10.1
>

2016-11-21 15:38:07

by Paul Gortmaker

[permalink] [raw]
Subject: Re: moduleparam: introduce core_param_named macro for non-modular code

[Re: moduleparam: introduce core_param_named macro for non-modular code] On 21/11/2016 (Mon 02:37) Jessica Yu wrote:

> +++ Paul Gortmaker [14/11/16 21:00 -0500]:
> >We have the case where module_param_named() in file "foo.c" for
> >parameter myparam translates that into the bootarg for the
> >non-modular use case as "foo.myparam=..."
> >
> >The problem exists where the use case with the filename and the
> >dot prefix is established, but the code is then realized to be 100%
> >non-modular, or is converted to non-modular. Both of the existing
> >macros like core_param() or setup_param() do not append such a
> >prefix, so a straight conversion to either will break the existing
> >use cases.
> >
> >Similarly, trying to embed a hard coded "foo." prefix on the name
> >fails cpp syntax due to the special nature of "." in code. So we add
> >this parallel variant for the modular --> non-modular transition to
> >preserve existing and documented use cases with such a prefix.
>
> Hm, I'm not convinced we need a core_ counterpart to module_param_named
> (that's nearly identical), when module_param_named already implements
> all of the above. Plenty of non-modular code already use it (e.g.

That above sentence was one of the things I was trying to fix, i.e. get
better clarity by not using "module" anything in code that is
non-modular. There are other advantages besides clarity too.

> workqueue, printk), and a prefix is automatically supplied (which can be
> overridden) in the non-modular case. That should already meet your
> requirements, no?

Well, it "works" but it isn't IMHO ideal. Anyway for now I will just
try and catch new instances and get them to use the non-modular ones
for non-modular cases before their use case becomes established, and
leave the existing ones with the prefix alone.

Paul.
--

>
> >Cc: Jessica Yu <[email protected]>
> >Cc: Rusty Russell <[email protected]>
> >Signed-off-by: Paul Gortmaker <[email protected]>
> >---
> >
> >[Marking this RFC since I don't like the fact that it still requires
> >non-modular code to use moduleparam.h -- one possible fix for that is
> >to consider moving non-modular macros to a new param.h or similar. ]
> >
> >include/linux/moduleparam.h | 17 +++++++++++++++++
> >1 file changed, 17 insertions(+)
> >
> >diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> >index 52666d90ca94..4f2b92345eb5 100644
> >--- a/include/linux/moduleparam.h
> >+++ b/include/linux/moduleparam.h
> >@@ -269,6 +269,23 @@ static inline void kernel_param_unlock(struct module *mod)
> > __module_param_call("", name, &param_ops_##type, &var, perm, -1, 0)
> >
> >/**
> >+ * core_param_named - define a module compat core kernel parameter.
> >+ * @name: the name of the cmdline and sysfs parameter (often the same as var)
> >+ * @var: the variable
> >+ * @type: the type of the parameter
> >+ * @perm: visibility in sysfs
> >+ *
> >+ * core_param_named is just like module_param_named(), but cannot be modular
> >+ * and it _does_ add a prefix (such as "printk."). This is for compatibility
> >+ * with module_param_named(), and it exists to provide boot arg compatibility
> >+ * with code that was previously using the modular version with the prefix.
> >+ */
> >+#define core_param_named(name, var, type, perm) \
> >+ param_check_##type(name, &(var)); \
> >+ __module_param_call(KBUILD_MODNAME ".", name, &param_ops_##type,\
> >+ &var, perm, -1, 0)
> >+
> >+/**
> > * core_param_unsafe - same as core_param but taints kernel
> > */
> >#define core_param_unsafe(name, var, type, perm) \
> >--
> >2.10.1
> >