CC: Russell King <[email protected]>, Paul Mackerras <[email protected]>, Geert Uytterhoeven <[email protected]>, Andi Kleen <[email protected]>, [email protected], [email protected], [email protected], [email protected]
[ CC'ing of arch maintainers where I've made non-trivial changes ]
Hello. The following is outcome of talking with David Woodhouse about
the need in various parts of the kernel to parse the command line very
early and set some options based on what we read. The result is
__early_param("arg", fn) based very heavily on the macro of the same name
in the arm kernel. The following is the core of these changes, adding the
macro, struct and externs to <linux/init.h>, the parser to init/main.c
and converting console= to this format. As a follow on to this thread are
patches against all arches (vs 2.6.5-rc2) to use the global define of
saved_command_line, add the appropriate bits to
arch/$(ARCH)/kernel/vmlinux.lds.S and in some cases, convert params
from the old arch-specific variant to the new __early_param way.
---
linux-2.6-early_setup-trini/include/linux/init.h | 14 ++++++
linux-2.6-early_setup-trini/init/main.c | 53 ++++++++++++++++++++++-
linux-2.6-early_setup-trini/kernel/printk.c | 2
3 files changed, 67 insertions(+), 2 deletions(-)
diff -puN include/linux/init.h~core include/linux/init.h
--- linux-2.6-early_setup/include/linux/init.h~core 2004-03-24 16:15:04.645141825 -0700
+++ linux-2.6-early_setup-trini/include/linux/init.h 2004-03-24 16:15:04.651140474 -0700
@@ -66,6 +66,20 @@ typedef void (*exitcall_t)(void);
extern initcall_t __con_initcall_start, __con_initcall_end;
extern initcall_t __security_initcall_start, __security_initcall_end;
+
+/*
+ * Early command line parameters.
+ */
+struct early_params {
+ const char *arg;
+ int (*fn)(char *p);
+};
+extern struct early_params __early_begin, __early_end;
+extern void parse_early_options(char **cmdline_p);
+
+#define __early_param(name,fn) \
+static struct early_params __early_##fn __attribute_used__ \
+__attribute__((__section__("__early_param"))) = { name, fn }
#endif
#ifndef MODULE
diff -puN init/main.c~core init/main.c
--- linux-2.6-early_setup/init/main.c~core 2004-03-24 16:15:04.647141375 -0700
+++ linux-2.6-early_setup-trini/init/main.c 2004-03-24 16:15:04.652140249 -0700
@@ -43,6 +43,7 @@
#include <linux/efi.h>
#include <linux/unistd.h>
+#include <asm/setup.h>
#include <asm/io.h>
#include <asm/bugs.h>
@@ -111,6 +112,10 @@ extern void time_init(void);
void (*late_time_init)(void);
extern void softirq_init(void);
+/* Stuff for the command line. */
+char saved_command_line[COMMAND_LINE_SIZE]; /* For /proc */
+static char tmp_command_line[COMMAND_LINE_SIZE]; /* Parsed. */
+
static char *execute_command;
/* Setup configured maximum number of CPUs to activate */
@@ -396,13 +401,59 @@ static void noinline rest_init(void)
}
/*
+ * Initial parsing of the command line. We destructivly
+ * scan the pointer, and take out any params for which we have
+ * an early handler for.
+ */
+void __init parse_early_options(char **cmdline_p)
+{
+ char *from = *cmdline_p; /* Original. */
+ char c = ' ', *to = tmp_command_line; /* Parsed. */
+ int len = 0;
+
+ /* Save it, if we need to. */
+ if (*cmdline_p != saved_command_line)
+ memcpy(saved_command_line, *cmdline_p, COMMAND_LINE_SIZE);
+ saved_command_line[COMMAND_LINE_SIZE - 1] = '\0';
+
+ for (;;) {
+ if (c == ' ') {
+ struct early_params *p;
+
+ for (p = &__early_begin; p < &__early_end; p++) {
+ int len = strlen(p->arg);
+
+ if (memcmp(from, p->arg, len) == 0) {
+ if (to != *cmdline_p)
+ to -= 1;
+ from += len;
+ p->fn(from);
+
+ while (*from != ' ' && *from != '\0')
+ from++;
+ break;
+ }
+ }
+ }
+ c = *from++;
+ if (!c)
+ break;
+ if (COMMAND_LINE_SIZE <= ++len)
+ break;
+ *to++ = c;
+ }
+
+ *to = '\0';
+ *cmdline_p = tmp_command_line;
+}
+
+/*
* Activate the first processor.
*/
asmlinkage void __init start_kernel(void)
{
char * command_line;
- extern char saved_command_line[];
extern struct kernel_param __start___param[], __stop___param[];
/*
* Interrupts are still disabled. Do necessary setups, then
diff -puN kernel/printk.c~core kernel/printk.c
--- linux-2.6-early_setup/kernel/printk.c~core 2004-03-24 16:15:04.649140924 -0700
+++ linux-2.6-early_setup-trini/kernel/printk.c 2004-03-24 16:15:04.653140024 -0700
@@ -149,7 +149,7 @@ static int __init console_setup(char *st
return 1;
}
-__setup("console=", console_setup);
+__early_param("console=", console_setup);
/**
* add_preferred_console - add a device to the list of preferred consoles.
_
[email protected] wrote:
>
> Hello. The following is outcome of talking with David Woodhouse about
> the need in various parts of the kernel to parse the command line very
> early and set some options based on what we read. The result is
> __early_param("arg", fn) based very heavily on the macro of the same name
> in the arm kernel. The following is the core of these changes, adding the
> macro, struct and externs to <linux/init.h>, the parser to init/main.c
> and converting console= to this format. As a follow on to this thread are
> patches against all arches (vs 2.6.5-rc2) to use the global define of
> saved_command_line, add the appropriate bits to
> arch/$(ARCH)/kernel/vmlinux.lds.S and in some cases, convert params
> from the old arch-specific variant to the new __early_param way.
I don't recall seeing this requirement mentioned before, and that's a ton
of patches you have there.
Please tell us a little more about why we need these patches. (Apart from
what seems to be a moderate amount of code consolidation).
Also, what is different between __setup and __early_setup? Why is it not
possible to make __setup run sufficiently early for whatever application is
requiring these changes?
Thanks.
On Wed, 2004-03-24 at 19:55 -0800, Andrew Morton wrote:
> Please tell us a little more about why we need these patches. (Apart from
> what seems to be a moderate amount of code consolidation).
A lot of command line options need checking before we get out of
setup_arch() into start_kernel() where parse_cmdline() is currently
called.
In particular, the only thing stopping us from registering real
permanent consoles from the moment we hit setup_arch() is the fact that
we haven't yet handled 'console=' on the command line, and we end up
enabling the first console registered as if there's no console=
argument, _despite_ the fact that there _is_ such an argument.
> Also, what is different between __setup and __early_setup? Why is it not
> possible to make __setup run sufficiently early for whatever application is
> requiring these changes?
Drivers may require allocation (bootmem not slab). We want to run before
that's feasible -- before 'mem=', by definition :)
There _are_ a lot of patches but most of them are trivial and were
separated just for cleanliness. Where they're non-trivial that's because
there's real consolidation.
--
dwmw2
On Fri, Mar 26, 2004 at 11:29:35AM +0000, David Woodhouse wrote:
> Drivers may require allocation (bootmem not slab). We want to run before
> that's feasible -- before 'mem=', by definition :)
It definitely wants to be after mem= since mem= modifies the memory
layout before bootmem is initialised (and its required to be parsed
before bootmem is initialised, so bootmem knows where the memory is.)
So we require, in order:
- mem= to be parsed
- bootmem to be initialised
- drivers which want to allocate from bootmem to then be
initialised
- setup rest of the kernel
- final command line parsing
which gives us three stages of command line parsing.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
On Fri, 2004-03-26 at 21:03 +0000, Russell King wrote:
> So we require, in order:
>
> - mem= to be parsed
> - bootmem to be initialised
> - drivers which want to allocate from bootmem to then be
> initialised
> - setup rest of the kernel
> - final command line parsing
>
> which gives us three stages of command line parsing.
No, because the existing command line parsing doesn't get to use the
slab, so there's no need for us to add that final item in your list.
Leave it out, and you have what Tom's already implemented. The current
__setup() calls are allowed to use bootmem but not slab, and the
__early_param() calls aren't even allowed to use bootmem.
--
dwmw2
On Thu, 2004-03-25 at 10:57, [email protected] wrote:
> +void __init parse_early_options(char **cmdline_p)
Please, don't do it this way.
__setup() has icky semantics which are only used in three places (ide,
ppc64's eeh, and um's eth). The string is a prefix and the rest of the
parameter is handed to the fn as an arg. Meaning misspellings aren't
usually caught, and accidental name reuse is hard to catch.
Secondly, we already have a parser in the kernel.
I like the idea of cleaning up saved_command_line crap: ideally
the archs would just assign to a global "command_line" var, and
anyone wanting to write to it would make their own copies.
How's this version, instead? If you agree, I'll produce a merged
version.
Thanks!
Rusty.
Name: Merge __early_param() with __setup code
Author: Rusty Russell
Status: Tested on 2.6.5-rc3
The __early_param implementation added yet another commandline parser
to the kernel. Try just overloading the __setup() code insteadm and
fix up the semantics (require exact match, hand args in two pieces, fn
returns void).
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.6.5-rc3/arch/i386/kernel/head.S working-2.6.5-rc3-early_param-with-setup/arch/i386/kernel/head.S
--- linux-2.6.5-rc3/arch/i386/kernel/head.S 2004-03-30 16:14:04.000000000 +1000
+++ working-2.6.5-rc3-early_param-with-setup/arch/i386/kernel/head.S 2004-03-31 13:38:31.000000000 +1000
@@ -19,6 +19,7 @@
#include <asm/thread_info.h>
#include <asm/asm_offsets.h>
#include <asm/setup.h>
+#include <asm/param.h>
/*
* References to members of the new_cpu_data structure.
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.6.5-rc3/include/asm-i386/param.h working-2.6.5-rc3-early_param-with-setup/include/asm-i386/param.h
--- linux-2.6.5-rc3/include/asm-i386/param.h 2004-03-12 07:57:19.000000000 +1100
+++ working-2.6.5-rc3-early_param-with-setup/include/asm-i386/param.h 2004-03-31 13:38:31.000000000 +1000
@@ -18,5 +23,6 @@
#endif
#define MAXHOSTNAMELEN 64 /* max length of hostname */
+#define COMMAND_LINE_SIZE 256
#endif
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.6.5-rc3/include/asm-i386/setup.h working-2.6.5-rc3-early_param-with-setup/include/asm-i386/setup.h
--- linux-2.6.5-rc3/include/asm-i386/setup.h 2004-03-30 16:14:30.000000000 +1000
+++ working-2.6.5-rc3-early_param-with-setup/include/asm-i386/setup.h 2004-03-31 13:38:31.000000000 +1000
@@ -17,7 +17,6 @@
#define MAX_NONPAE_PFN (1 << 20)
#define PARAM_SIZE 2048
-#define COMMAND_LINE_SIZE 256
#define OLD_CL_MAGIC_ADDR 0x90020
#define OLD_CL_MAGIC 0xA33F
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.6.5-rc3/include/linux/init.h working-2.6.5-rc3-early_param-with-setup/include/linux/init.h
--- linux-2.6.5-rc3/include/linux/init.h 2004-03-30 16:14:35.000000000 +1000
+++ working-2.6.5-rc3-early_param-with-setup/include/linux/init.h 2004-03-31 14:14:21.000000000 +1000
@@ -106,26 +106,38 @@ extern initcall_t __security_initcall_st
struct obs_kernel_param {
const char *str;
- int (*setup_func)(char *);
+ union {
+ int (*setup_func)(char *);
+ void (*param_func)(char *);
+ } u;
+ int early;
};
-/* OBSOLETE: see moduleparam.h for the right way. */
-#define __setup_param(str, unique_id, fn) \
+/* These only when builtin: see moduleparam.h for the normal way. */
+/* Called if str matches prefix: fn returns 1 if we're the right one. */
+#define __setup(prefix, fn) \
+ __setup_param(prefix, fn, {.setup_func = fn}, 0)
+
+/* Called v. early on when option matches str: fn gets string after =,
+ * or NULL. */
+#define __early_param(str, fn) \
+ __setup_param(str, fn, {.param_func = fn}, 1)
+
+#define __setup_param(str, unique_id, fn_init, early) \
static char __setup_str_##unique_id[] __initdata = str; \
static struct obs_kernel_param __setup_##unique_id \
__attribute_used__ \
__attribute__((__section__(".init.setup"))) \
- = { __setup_str_##unique_id, fn }
-
-#define __setup_null_param(str, unique_id) \
- __setup_param(str, unique_id, NULL)
+ = { __setup_str_##unique_id, fn_init, early }
-#define __setup(str, fn) \
- __setup_param(str, fn, fn)
+#define __obsolete_setup2(str, unique_id) \
+ __setup_param(str, unique_id, {.setup_func = NULL}, 0)
+/* Does nothing, but will warn user if it's used. */
#define __obsolete_setup(str) \
- __setup_null_param(str, __LINE__)
+ __obsolete_setup2(str, __LINE__)
+void __init parse_early_options(const char *saved_command_line);
#endif /* __ASSEMBLY__ */
/**
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.6.5-rc3/init/main.c working-2.6.5-rc3-early_param-with-setup/init/main.c
--- linux-2.6.5-rc3/init/main.c 2004-03-30 16:14:35.000000000 +1000
+++ working-2.6.5-rc3-early_param-with-setup/init/main.c 2004-03-31 14:14:26.000000000 +1000
@@ -157,10 +157,13 @@ static int __init obsolete_checksetup(ch
do {
int n = strlen(p->str);
if (!strncmp(line, p->str, n)) {
- if (!p->setup_func) {
+ /* Already done in handle_early_option? */
+ if (p->early)
+ return 1;
+ if (!p->u.setup_func) {
printk(KERN_WARNING "Parameter %s is obsolete, ignored\n", p->str);
return 1;
- } else if (p->setup_func(line + n))
+ } else if (p->u.setup_func(line + n))
return 1;
}
p++;
@@ -393,7 +397,31 @@ static void noinline rest_init(void)
kernel_thread(init, NULL, CLONE_FS | CLONE_SIGHAND);
unlock_kernel();
cpu_idle();
-}
+}
+
+/* Check for early options. */
+static int __init early_option(char *param, char *val)
+{
+ struct obs_kernel_param *p;
+ extern struct obs_kernel_param __setup_start, __setup_end;
+
+ for (p = &__setup_start; p < &__setup_end; p++)
+ if (p->early && strcmp(param, p->str) == 0)
+ p->u.param_func(val);
+
+ /* We accept everything at this stage. */
+ return 0;
+}
+
+/* Arch code calls this early on. */
+void __init parse_early_options(const char *saved_command_line)
+{
+ static char __initdata command_line[COMMAND_LINE_SIZE];
+ strcpy(command_line, saved_command_line);
+
+ /* All fall through to handle_early_option. */
+ parse_args("early options", command_line, NULL, 0, early_option);
+}
/*
* Activate the first processor.
--
Anyone who quotes me in their signature is an idiot -- Rusty Russell
On Wed, Mar 31, 2004 at 02:23:05PM +1000, Rusty Russell wrote:
> On Thu, 2004-03-25 at 10:57, [email protected] wrote:
> > +void __init parse_early_options(char **cmdline_p)
>
> Please, don't do it this way.
>
> __setup() has icky semantics which are only used in three places (ide,
> ppc64's eeh, and um's eth). The string is a prefix and the rest of the
> parameter is handed to the fn as an arg. Meaning misspellings aren't
> usually caught, and accidental name reuse is hard to catch.
>
> Secondly, we already have a parser in the kernel.
>
> I like the idea of cleaning up saved_command_line crap: ideally
> the archs would just assign to a global "command_line" var, and
> anyone wanting to write to it would make their own copies.
>
> How's this version, instead? If you agree, I'll produce a merged
> version.
My first concern is can parse_args & co really be run so very early on ?
Also:
> +/* Arch code calls this early on. */
> +void __init parse_early_options(const char *saved_command_line)
> +{
> + static char __initdata command_line[COMMAND_LINE_SIZE];
> + strcpy(command_line, saved_command_line);
Really should be:
/* i386 goes right to saved_command_line */
if (*cmdline_p != saved_command_line)
memcpy(saved_command_line, *cmdline_p, COMMAND_LINE_SIZE);
/* ensure NUL terminated. */
saved_command_line[COMMAND_LINE_SIZE - 1] = '\0';
--
Tom Rini
http://gate.crashing.org/~trini/
On Thu, 2004-04-01 at 02:13, Tom Rini wrote:
> My first concern is can parse_args & co really be run so very early on ?
If you can run normal C code, yes. It doesn't require any
initialization.
> Also:
> > +/* Arch code calls this early on. */
> > +void __init parse_early_options(const char *saved_command_line)
> > +{
> > + static char __initdata command_line[COMMAND_LINE_SIZE];
> > + strcpy(command_line, saved_command_line);
>
> Really should be:
> /* i386 goes right to saved_command_line */
> if (*cmdline_p != saved_command_line)
> memcpy(saved_command_line, *cmdline_p, COMMAND_LINE_SIZE);
> /* ensure NUL terminated. */
> saved_command_line[COMMAND_LINE_SIZE - 1] = '\0';
Why? Other than the nul term (which I agree with), I didn't understand
that code.
Get the archs to pass some command line in. eg. i386 can do:
- parse_cmdline_early(cmdline_p);
+ parse_early_options(*cmdline_p);
I feel that anyone destroying command lines should do their own
saving, and we should head that way...
Thanks,
Rusty.
--
Anyone who quotes me in their signature is an idiot -- Rusty Russell
On Thu, Apr 01, 2004 at 11:51:41AM +1000, Rusty Russell wrote:
> On Thu, 2004-04-01 at 02:13, Tom Rini wrote:
> > My first concern is can parse_args & co really be run so very early on ?
>
> If you can run normal C code, yes. It doesn't require any
> initialization.
No malloc'ing, etc? OK. Then yes, please create some sort of merged
patchset (I don't know how Andrew wants to handle it, maybe just replace
the core patch?)
> > Also:
> > > +/* Arch code calls this early on. */
> > > +void __init parse_early_options(const char *saved_command_line)
> > > +{
> > > + static char __initdata command_line[COMMAND_LINE_SIZE];
> > > + strcpy(command_line, saved_command_line);
> >
> > Really should be:
> > /* i386 goes right to saved_command_line */
> > if (*cmdline_p != saved_command_line)
> > memcpy(saved_command_line, *cmdline_p, COMMAND_LINE_SIZE);
> > /* ensure NUL terminated. */
> > saved_command_line[COMMAND_LINE_SIZE - 1] = '\0';
>
> Why? Other than the nul term (which I agree with), I didn't understand
> that code.
>
> Get the archs to pass some command line in. eg. i386 can do:
>
> - parse_cmdline_early(cmdline_p);
> + parse_early_options(*cmdline_p);
>
> I feel that anyone destroying command lines should do their own
> saving, and we should head that way...
memcpy(src, src, size) isn't good, is it? On i386 we start out with the
commandline in saved_command_line. Everyone else I believe has a local
var we point cmdline_p at, which gets copied into saved_command_line.
--
Tom Rini
http://gate.crashing.org/~trini/
On Thu, 2004-04-01 at 12:24, Tom Rini wrote:
> > > > +/* Arch code calls this early on. */
> > > > +void __init parse_early_options(const char *saved_command_line)
> > > > +{
> > > > + static char __initdata command_line[COMMAND_LINE_SIZE];
> > > > + strcpy(command_line, saved_command_line);
> memcpy(src, src, size) isn't good, is it? On i386 we start out with the
> commandline in saved_command_line. Everyone else I believe has a local
> var we point cmdline_p at, which gets copied into saved_command_line.
Read again. Not accessing global vars at all: it's a (probably badly
named) parameter.
Rusty.
--
Anyone who quotes me in their signature is an idiot -- Rusty Russell