Re: [patch 1/5] Extended crashkernel command line

[Vivek Goyal <vgoyal@in.ibm.com>]

On Sun, Sep 09, 2007 at 10:39:15AM +0200, Bernhard Walle wrote:

[..]
> +
> +/*
> + * 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 parse_crashkernel_mem(char 			*cmdline,
> +				 unsigned long long 	*crash_size,
> +				 unsigned long long 	*crash_base,
> +				 unsigned long 		system_ram)
> +{
> +	char *cur = cmdline;
> +
> +	*crash_size = 0;
> +	*crash_base = 0;
> +
> +	/* 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);

"offset" seems to be optional in the new syntax. What happens if user does
not specify offset. I think crash_base will be set to zero and system will
try to reserve x amount of memory start at zero? That would fail?

I think we should add some intelligence for automatic selection of "offset"
if user has not specified one. Automatically choose a chunk of free memory.
This takes away the headache from user for selecting a right place. In fact
one "offset" might not be valid for all the systems. I remember, somebody
had reported that ACPI tables were mapped lower in the address space and
reserving memory for kdump had failed.

Choosing the "offset" automatically should help distributions where one
command line is supposed to work on all the systems.

Thanks
Vivek