Re: [PATCH 1/3] kernel: Add a new config option to remove command line parsing

Re: [PATCH 1/3] kernel: Add a new config option to remove command line parsing

[Tim Bird]

On 04/01/2015 07:34 AM, Iulia Manda wrote:
> This patch introduces CONFIG_CMDLINE_PARSE option which conditionally compiles
> the support for parsing kernel command line arguments. The corresponding
> functions that actually do the parsing will be compiled out. 
> 
> This is used when no parameters will be specified neither at compile time nor at
> boot time.
> 
> Bloat-o-meter output (compared to the preivous version in which builtin cmdline
> was also set to 'Y'):
> 
> add/remove: 0/8 grow/shrink: 0/6 up/down: 0/-3669 (-3669)

I think this is great stuff.  (Of course, having research something like this
previously, I'm a bit biased.)

It's very nice to have the Bloat-o-meter output for these kernel size shrinking
patches.

The patch itself looks very straightforward, and I don't see any problems
in a visual inspection.

You can add a Reviewed-by: Tim Bird <tim.bird@sonymobile.com> to this patch.

I'd like to be able to point people to this on the mailing list.  In the future
can you make sure to CC: <linux-kernel@vger.kernel.org>, so there's a reference
on lkml.org I can point people to?

Thanks for this great work!
 -- Tim

[PATCH 1/3] kernel: Add a new config option to remove command line parsing

[Iulia Manda]

This patch introduces CONFIG_CMDLINE_PARSE option which conditionally
compiles the support for parsing kernel command line arguments. The
corresponding functions that actually do the parsing will be compiled out.

This is used when no parameters will be specified neither at compile time
nor at boot time.

Bloat-o-meter output (compared to the version in which builtin
cmdline is set to 'Y'):

add/remove: 0/8 grow/shrink: 0/6 up/down: 0/-3669 (-3669)
function                                     old     new   delta
load_module                                 5571    5563      -8
parse_early_param                             54      44     -10
parse_early_options                           33       5     -28
initcall_level_names                          32       -     -32
kernel_init_freeable                         413     360     -53
unknown_module_param_cb                       60       -     -60
setup_arch                                  3041    2972     -69
set_init_arg                                  73       -     -73
repair_env_string                             81       -     -81
start_kernel                                 857     759     -98
do_early_param                               117       -    -117
unknown_bootoption                           366       -    -366
parse_args                                   626       -    -626
builtin_cmdline                             2048       -   -2048

Signed-off-by: Iulia Manda <iulia.manda21@gmail.com>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
Reviewed-by: Tim Bird <tim.bird@sonymobile.com>
---
 arch/x86/Kconfig            | 15 +++++++++++++++
 include/linux/moduleparam.h | 14 ++++++++++++++
 kernel/params.c             |  2 ++
 3 files changed, 31 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 6049d58..daabbc3 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2002,8 +2002,23 @@ config COMPAT_VDSO
 	  If unsure, say N: if you are compiling your own kernel, you
 	  are unlikely to be using a buggy version of glibc.
 
+config CMDLINE_PARSE
+	bool "Enable support for command line parsing"
+	default y
+	---help---
+	  With this option set to 'Y', kernel parameters, both the ones
+	  passed at boot time and at compile time are parsed.
+
+	  If you say no here, all the kernel parameters' values will be set
+	  to their defaults at compile time, in order to make constant
+	  folding possible.
+
+	  Systems with no space constraints should leave this option set to
+	  'Y'.
+
 config CMDLINE_BOOL
 	bool "Built-in kernel command line"
+	depends on CMDLINE_PARSE
 	---help---
 	  Allow for specifying boot arguments to the kernel at
 	  build time.  On some systems (e.g. embedded ones), it is
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 1c9effa..570f70e 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -350,6 +350,7 @@ extern bool parameq(const char *name1, const char *name2);
  */
 extern bool parameqn(const char *name1, const char *name2, size_t n);
 
+#ifdef CONFIG_CMDLINE_PARSE
 /* Called on module insert or kernel boot */
 extern char *parse_args(const char *name,
 		      char *args,
@@ -359,6 +360,19 @@ extern char *parse_args(const char *name,
 		      s16 level_max,
 		      int (*unknown)(char *param, char *val,
 			      const char *doing));
+#else
+static inline char *parse_args(const char *name,
+		      char *args,
+		      const struct kernel_param *params,
+		      unsigned num,
+		      s16 level_min,
+		      s16 level_max,
+		      int (*unknown)(char *param, char *val,
+			      const char *doing))
+{
+	return NULL;
+}
+#endif
 
 /* Called by module remove. */
 #ifdef CONFIG_SYSFS
diff --git a/kernel/params.c b/kernel/params.c
index 728e05b..d3bfe47 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -93,6 +93,7 @@ static void param_check_unsafe(const struct kernel_param *kp)
 	}
 }
 
+#ifdef CONFIG_CMDLINE_PARSE
 static int parse_one(char *param,
 		     char *val,
 		     const char *doing,
@@ -239,6 +240,7 @@ char *parse_args(const char *doing,
 	/* All parsed OK. */
 	return NULL;
 }
+#endif
 
 /* Lazy bastard, eh? */
 #define STANDARD_PARAM_DEF(name, type, format, strtolfn)      		\
-- 
1.8.3.2

Re: [PATCH 1/3] kernel: Add a new config option to remove command line parsing

[Paul Bolle]

On Mon, 2015-05-18 at 14:50 +0300, Iulia Manda wrote:
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig

> +config CMDLINE_PARSE
> +	bool "Enable support for command line parsing"
> +	default y
> +	---help---
> +	  With this option set to 'Y', kernel parameters, both the ones
> +	  passed at boot time and at compile time are parsed.
> +
> +	  If you say no here, all the kernel parameters' values will be set
> +	  to their defaults at compile time, in order to make constant
> +	  folding possible.
> +
> +	  Systems with no space constraints should leave this option set to
> +	  'Y'.

This adds a x86 specific Kconfig option.

> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
 
> +#ifdef CONFIG_CMDLINE_PARSE
>  /* Called on module insert or kernel boot */
>  extern char *parse_args(const char *name,
>  		      char *args,
> @@ -359,6 +360,19 @@ extern char *parse_args(const char *name,
>  		      s16 level_max,
>  		      int (*unknown)(char *param, char *val,
>  			      const char *doing));
> +#else
> +static inline char *parse_args(const char *name,
> +		      char *args,
> +		      const struct kernel_param *params,
> +		      unsigned num,
> +		      s16 level_min,
> +		      s16 level_max,
> +		      int (*unknown)(char *param, char *val,
> +			      const char *doing))
> +{
> +	return NULL;
> +}
> +#endif

So this effectively disables parse_args() for all architectures, doesn't
it? If that's OK, I'd say the patch is x86 specific. Than it should
have, say, "x86:" as a prefix and it should be sent to the people and
lists taking care of x86.

But a quick grep suggests it's not OK to disable this for all other
architectures. Did I miss something with that quick grep?

Thanks,


Paul Bolle

Re: [PATCH 1/3] kernel: Add a new config option to remove command line parsing

[josh]

On Tue, May 19, 2015 at 11:35:59AM +0200, Paul Bolle wrote:
> On Mon, 2015-05-18 at 14:50 +0300, Iulia Manda wrote:
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> 
> > +config CMDLINE_PARSE
> > +	bool "Enable support for command line parsing"
> > +	default y
> > +	---help---
> > +	  With this option set to 'Y', kernel parameters, both the ones
> > +	  passed at boot time and at compile time are parsed.
> > +
> > +	  If you say no here, all the kernel parameters' values will be set
> > +	  to their defaults at compile time, in order to make constant
> > +	  folding possible.
> > +
> > +	  Systems with no space constraints should leave this option set to
> > +	  'Y'.
> 
> This adds a x86 specific Kconfig option.

Good catch.  I reviewed the Kconfig option and its usage very carefully,
but completely missed that it was in the wrong file.

This needs to go in init/Kconfig or similar.

- Josh Triplett

Re: [PATCH 1/3] kernel: Add a new config option to remove command line parsing

[Rob Landley]

On Mon, May 18, 2015 at 6:50 AM, Iulia Manda <iulia.manda21@gmail.com> wrote:
> This patch introduces CONFIG_CMDLINE_PARSE option which conditionally
> compiles the support for parsing kernel command line arguments. The
> corresponding functions that actually do the parsing will be compiled out.

Could you make it depend on EXPERT?

Thanks,

Rob