[OPW,kernel,2/3] linux: Add macros that define and declare a core_param variable
diff mbox

Message ID 913906f1c59e6da1b7de48d9363bc44f36f3e4bc.1425391406.git.iulia.manda21@gmail.com
State New, archived
Headers show

Commit Message

Iulia Manda March 3, 2015, 2:13 p.m. UTC
This patch introduces two macros for kernel command line arguments subsequently
defined as core parameters:
* DECLARE_CORE_PARAM declares a constant variable.
* DEFINE_CORE_PARAM is a wrapper around core_param which, in addition, defines a
  variable with a default value.

Because at some point we need to access the address of var, it can not be const.
This is why we need to add a dummy variable that does not affect the rest of the
code and only provides us with a valid address of __var, knowing that in this
case that address will never be used. This way the variable's value is known at
compile time and we can rely on GCC to be able to constant fold it.

Signed-off-by: Iulia Manda <iulia.manda21@gmail.com>
---
 include/linux/init.h        |  8 ++++++++
 include/linux/moduleparam.h | 11 +++++++++++
 2 files changed, 19 insertions(+)

Comments

Josh Triplett March 3, 2015, 6:11 p.m. UTC | #1
On Tue, Mar 03, 2015 at 04:13:44PM +0200, Iulia Manda wrote:
> This patch introduces two macros for kernel command line arguments subsequently
> defined as core parameters:
> * DECLARE_CORE_PARAM declares a constant variable.
> * DEFINE_CORE_PARAM is a wrapper around core_param which, in addition, defines a
>   variable with a default value.
> 
> Because at some point we need to access the address of var, it can not be const.
> This is why we need to add a dummy variable that does not affect the rest of the
> code and only provides us with a valid address of __var, knowing that in this
> case that address will never be used. This way the variable's value is known at
> compile time and we can rely on GCC to be able to constant fold it.

I still don't think we need the dummy variable and the call to
core_param in the !CONFIG_CMDLINE_PARSE case.

> Signed-off-by: Iulia Manda <iulia.manda21@gmail.com>
> ---
>  include/linux/init.h        |  8 ++++++++
>  include/linux/moduleparam.h | 11 +++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 2df8e8d..ef32825 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -155,6 +155,14 @@ int __init init_rootfs(void);
>  
>  extern void (*late_time_init)(void);
>  
> +#ifdef CONFIG_CMDLINE_PARSE
> +#define DECLARE_CORE_PARAM(name) \
> +        bool name
> +#else
> +#define DECLARE_CORE_PARAM(name) \
> +        const bool name
> +#endif

This shouldn't be defined in init.h; it should be in moduleparam.h next
to DEFINE_CORE_PARAM.  For consistency with DEFINE_CORE_PARAM, the
variable name should be called "var", since "name" refers to the name of
the command line option (which may differ).  Also, it shouldn't be
hardcoded to bool, and it needs an "extern".

Actually, that raises a problematic issue.  "extern const" doesn't allow
for constant folding; I think the only constant folding you're getting
in commit #3 of this series is in init/main.c, not in the various other
files that reference initcall_debug.

On that basis, I wonder if it'd make more sense to make
DECLARE_CORE_PARAM (with !CONFIG_CMDLINE_PARSE) turn into "static const
type var = val;", and then have DEFINE_CORE_PARAM compile to nothing at
all in that case.  That'll work for initcall_debug; the only minor
annoyance is that for a parameter kept entirely in the same file without
an associated header, you still have to use both DECLARE_CORE_PARAM and
DEFINE_CORE_PARAM, rather than just the latter.  That seems OK though.

>  extern bool initcall_debug;
>  
>  #endif
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index f97397c..4e1e2f2 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -310,6 +310,17 @@ static inline void __kernel_param_unlock(void)
>  #define core_param(name, var, type, perm)				\
>  	param_check_##type(name, &(var));				\
>  	__module_param_call("", name, &param_ops_##type, &var, perm, -1, 0)
> +
> +#ifdef CONFIG_CMDLINE_PARSE
> +#define DEFINE_CORE_PARAM(name, var, val, type, perm)			\
> +        type var = val;                                                 \
> +        core_param(name, var, type, perm);
> +#else
> +#define DEFINE_CORE_PARAM(name, var, val, type, perm)			\
> +        const type var = val;                                           \
> +        type __##var;                                                   \
> +        core_param(name, __##var, type, perm);
> +#endif

In the #else cases here, you should drop the __##var and core_param.
Or, as suggested above, you could define it to nothing and declare a
static const in DECLARE_CORE_PARAM.

>  #endif /* !MODULE */
>  
>  /**
> -- 
> 1.9.1
> 
> -- 
> You received this message because you are subscribed to the Google Groups "opw-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to opw-kernel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Patch
diff mbox

diff --git a/include/linux/init.h b/include/linux/init.h
index 2df8e8d..ef32825 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -155,6 +155,14 @@  int __init init_rootfs(void);
 
 extern void (*late_time_init)(void);
 
+#ifdef CONFIG_CMDLINE_PARSE
+#define DECLARE_CORE_PARAM(name) \
+        bool name
+#else
+#define DECLARE_CORE_PARAM(name) \
+        const bool name
+#endif
+
 extern bool initcall_debug;
 
 #endif
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index f97397c..4e1e2f2 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -310,6 +310,17 @@  static inline void __kernel_param_unlock(void)
 #define core_param(name, var, type, perm)				\
 	param_check_##type(name, &(var));				\
 	__module_param_call("", name, &param_ops_##type, &var, perm, -1, 0)
+
+#ifdef CONFIG_CMDLINE_PARSE
+#define DEFINE_CORE_PARAM(name, var, val, type, perm)			\
+        type var = val;                                                 \
+        core_param(name, var, type, perm);
+#else
+#define DEFINE_CORE_PARAM(name, var, val, type, perm)			\
+        const type var = val;                                           \
+        type __##var;                                                   \
+        core_param(name, __##var, type, perm);
+#endif
 #endif /* !MODULE */
 
 /**