2007-09-20 17:19:26

by Bernhard Walle

[permalink] [raw]
Subject: [patch 1/7] Extended crashkernel command line

This is the generic part of the patch. It adds a parse_crashkernel() function
in kernel/kexec.c that is called by the architecture specific code that
actually reserves the memory. That function takes the whole command line and
looks itself for "crashkernel=" in it.

If there are multiple occurrences, then the last one is taken. The advantage
is that if you have a bootloader like lilo or elilo which allows you to append
a command line parameter but not to remove one (like in GRUB), then you can add
another crashkernel value for testing at the boot command line and this one
overwrites the command line in the configuration then.


Signed-off-by: Bernhard Walle <[email protected]>

---
include/linux/kexec.h | 2
kernel/kexec.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 143 insertions(+)

--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -179,6 +179,8 @@ extern note_buf_t *crash_notes;
extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
extern unsigned int vmcoreinfo_size;
extern unsigned int vmcoreinfo_max_size;
+int __init parse_crashkernel(char *cmdline, unsigned long long system_ram,
+ unsigned long long *crash_size, unsigned long long *crash_base);


#else /* !CONFIG_KEXEC */
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1146,6 +1146,147 @@ static int __init crash_notes_memory_ini
}
module_init(crash_notes_memory_init)

+
+/*
+ * parsing the "crashkernel" commandline
+ *
+ * this code is intended to be called from architecture specific code
+ */
+
+
+/*
+ * This function parses command lines in the format
+ *
+ * crashkernel=<ramsize-range>:<size>[,...][@<base>]
+ *
+ * The function returns 0 on success and -EINVAL on failure.
+ */
+static int __init parse_crashkernel_mem(char *cmdline,
+ unsigned long system_ram,
+ unsigned long long *crash_size,
+ unsigned long long *crash_base)
+{
+ char *cur = cmdline;
+
+ /* for each entry of the comma-separated list */
+ do {
+ unsigned long long start = 0, end = ULLONG_MAX;
+ unsigned long long size = -1;
+
+ /* get the start of the range */
+ start = memparse(cur, &cur);
+ if (*cur != '-') {
+ printk(KERN_WARNING "crashkernel: '-' expected\n");
+ return -EINVAL;
+ }
+ cur++;
+
+ /* if no ':' is here, than we read the end */
+ if (*cur != ':') {
+ end = memparse(cur, &cur);
+ if (end <= start) {
+ printk(KERN_WARNING "crashkernel: end <= start\n");
+ return -EINVAL;
+ }
+ }
+
+ if (*cur != ':') {
+ printk(KERN_WARNING "crashkernel: ':' expected\n");
+ return -EINVAL;
+ }
+ cur++;
+
+ size = memparse(cur, &cur);
+ if (size < 0) {
+ printk(KERN_WARNING "crashkernel: invalid size\n");
+ return -EINVAL;
+ }
+
+ /* match ? */
+ if (system_ram >= start && system_ram <= end) {
+ *crash_size = size;
+ break;
+ }
+ } while (*cur++ == ',');
+
+ if (*crash_size > 0) {
+ while (*cur != ' ' && *cur != '@')
+ cur++;
+ if (*cur == '@')
+ *crash_base = memparse(cur+1, &cur);
+ }
+
+ return 0;
+}
+
+/*
+ * That function parses "simple" (old) crashkernel command lines like
+ *
+ * crashkernel=size[@base]
+ *
+ * It returns 0 on success and -EINVAL on failure.
+ */
+static int __init parse_crashkernel_simple(char *cmdline,
+ unsigned long long *crash_size,
+ unsigned long long *crash_base)
+{
+ char *cur = cmdline;
+
+ *crash_size = memparse(cmdline, &cur);
+ if (cmdline == cur)
+ return -EINVAL;
+
+ if (*cur == '@')
+ *crash_base = memparse(cur+1, &cur);
+
+ return 0;
+}
+
+/*
+ * That function is the entry point for command line parsing and should be
+ * called from the arch-specific code.
+ */
+int __init parse_crashkernel(char *cmdline,
+ unsigned long long system_ram,
+ unsigned long long *crash_size,
+ unsigned long long *crash_base)
+{
+ char *p = cmdline, *ck_cmdline = NULL;
+ char *first_colon, *first_space;
+
+ *crash_size = 0;
+ *crash_base = 0;
+
+ /* find crashkernel and use the last one if there are more */
+ p = strstr(p, "crashkernel=");
+ while (p) {
+ ck_cmdline = p;
+ p = strstr(p+1, "crashkernel=");
+ }
+
+ if (!ck_cmdline)
+ return -EINVAL;
+
+ ck_cmdline += 12; /* strlen("crashkernel=") */
+
+ /*
+ * if the commandline contains a ':', then that's the extended
+ * syntax -- if not, it must be the classic syntax
+ */
+ first_colon = strchr(ck_cmdline, ':');
+ first_space = strchr(ck_cmdline, ' ');
+ if (first_colon && (!first_space || first_colon < first_space))
+ return parse_crashkernel_mem(ck_cmdline, system_ram,
+ crash_size, crash_base);
+ else
+ return parse_crashkernel_simple(ck_cmdline, crash_size,
+ crash_base);
+
+ return 0;
+}
+
+
+
void crash_save_vmcoreinfo(void)
{
u32 *buf;

--


2007-09-22 22:58:30

by Oleg Verych

[permalink] [raw]
Subject: Re: [patch 1/7] Extended crashkernel command line

* Thu, 20 Sep 2007 19:18:46 +0200

[]
> extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
> extern unsigned int vmcoreinfo_size;
> extern unsigned int vmcoreinfo_max_size;
> +int __init parse_crashkernel(char *cmdline, unsigned long long system_ram,
> + unsigned long long *crash_size, unsigned long long *crash_base);

(BTW, why `system_ram' is `unsigned long' in parse_crashkernel_mem() but
`unsigned long long' in parse_crashkernel()?)

> +static int __init parse_crashkernel_mem(char *cmdline,
> + unsigned long system_ram,
> + unsigned long long *crash_size,
> + unsigned long long *crash_base)
> +{
> + char *cur = cmdline;
> +
> + /* for each entry of the comma-separated list */
> + do {
> + unsigned long long start = 0, end = ULLONG_MAX;
> + unsigned long long size = -1;
[]

What is the point of not using `ulong' and `u64'?

What about another names?

+int __init get_crashkernel_params(u64 *memsize, u64 *addrbase, char *cmdline, u64 ram);
_____

2007-09-23 20:19:55

by Bernhard Walle

[permalink] [raw]
Subject: Re: [patch 1/7] Extended crashkernel command line

* Oleg Verych <[email protected]> [2007-09-23 01:14]:
> * Thu, 20 Sep 2007 19:18:46 +0200
>
> []
> > extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
> > extern unsigned int vmcoreinfo_size;
> > extern unsigned int vmcoreinfo_max_size;
> > +int __init parse_crashkernel(char *cmdline, unsigned long long system_ram,
> > + unsigned long long *crash_size, unsigned long long *crash_base);
>
> (BTW, why `system_ram' is `unsigned long' in parse_crashkernel_mem() but
> `unsigned long long' in parse_crashkernel()?)

Because that's a bug, thanks for spotting that out. I will sent an
updated version of this patch until the issue below has been
clarified:

> What is the point of not using `ulong' and `u64'?
>
> What about another names?
>
> +int __init get_crashkernel_params(u64 *memsize, u64 *addrbase, char *cmdline, u64 ram);

Andrew, what's your opinion on this? Whould I resend the patch with
shorter type names?


Thanks,
Bernhard

2007-09-23 20:59:47

by Oleg Verych

[permalink] [raw]
Subject: Re: [patch 1/7] Extended crashkernel command line

On Sun, Sep 23, 2007 at 10:19:43PM +0200, Bernhard Walle wrote:
[]
> > +int __init get_crashkernel_params(u64 *memsize, u64 *addrbase, char *cmdline, u64 ram);
>
> Andrew, what's your opinion on this? Whould I resend the patch with
> shorter type names?

Also, maybe it will be better to extend linux/lib/cmdline.c->{get_range(), get_range()}
to handle that stuff? And maybe new functionality can be built up-on the current one:

- crashkernel=512M-2G:64M,2G-:128M@offset
+ crashkernel=512M-2G,64M,2G-,128M,,offset
?

OK, OK, you are already using and maintaining new syntax, but still.
_____
(`Mail-Followup-To' ignored)

2007-09-23 21:04:47

by Bernhard Walle

[permalink] [raw]
Subject: Re: [patch 1/7] Extended crashkernel command line

* Oleg Verych <[email protected]> [2007-09-23 23:15]:
>
> - crashkernel=512M-2G:64M,2G-:128M@offset
> + crashkernel=512M-2G,64M,2G-,128M,,offset
> ?

I don't like this syntax.


Thanks,
Bernhard