[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20130410191416.GE6602@redhat.com>
Date: Wed, 10 Apr 2013 15:14:16 -0400
From: Vivek Goyal <vgoyal@...hat.com>
To: Yinghai Lu <yinghai@...nel.org>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...e.hu>,
"H. Peter Anvin" <hpa@...or.com>, WANG Chao <chaowang@...hat.com>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
linux-kernel@...r.kernel.org,
HATAYAMA Daisuke <d.hatayama@...fujitsu.com>
Subject: Re: [PATCH v4 3/4] x86, kdump: Change crashkernel_high/low= to
crashkernel=,high/low
On Tue, Apr 09, 2013 at 01:01:50PM -0700, Yinghai Lu wrote:
> Per hpa, use crashkernel=X,high crashkernel=Y,low instead of
> crashkernel_hign=X crashkernel_low=Y. As that could be extensible.
>
> -v2: according to Vivek, change delimiter to ;
> -v3: let hign and low only handle simple form and it conforms to
> description in kernel-parameters.txt
> still keep crashkernel=X override any crashkernel=X,high
> crashkernel=Y,low
> -v4: update get_last_crashkernel returning and add more strict
> checking in parse_crashkernel_simple() found by HATAYAMA.
> -v5: Change delimiter back to , according to HPA.
> also separate parse_suffix from parse_simper according to vivek.
> so we can avoid @pos in that path.
>
> Cc: HATAYAMA Daisuke <d.hatayama@...fujitsu.com>
> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
This one looks good to me. Looks like you posted this one independet of
previous series.
Can you repost the final version in a series again. I will do some
more testing and ack it.
Thanks
Vivek
>
> ---
> Documentation/kernel-parameters.txt | 10 +--
> arch/x86/kernel/setup.c | 6 +-
> kernel/kexec.c | 103 +++++++++++++++++++++++++++++++-----
> 3 files changed, 98 insertions(+), 21 deletions(-)
>
> Index: linux-2.6/Documentation/kernel-parameters.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/kernel-parameters.txt
> +++ linux-2.6/Documentation/kernel-parameters.txt
> @@ -603,16 +603,16 @@ bytes respectively. Such letter suffixes
> a memory unit (amount[KMG]). See also
> Documentation/kdump/kdump.txt for an example.
>
> - crashkernel_high=size[KMG]
> + crashkernel=size[KMG],high
> [KNL, x86_64] range could be above 4G. Allow kernel
> to allocate physical memory region from top, so could
> be above 4G if system have more than 4G ram installed.
> Otherwise memory region will be allocated below 4G, if
> available.
> It will be ignored if crashkernel=X is specified.
> - crashkernel_low=size[KMG]
> - [KNL, x86_64] range under 4G. When crashkernel_high= is
> - passed, kernel could allocate physical memory region
> + crashkernel=size[KMG],low
> + [KNL, x86_64] range under 4G. When crashkernel=X,high
> + is passed, kernel could allocate physical memory region
> above 4G, that cause second kernel crash on system
> that require some amount of low memory, e.g. swiotlb
> requires at least 64M+32K low memory. Kernel would
> @@ -620,7 +620,7 @@ bytes respectively. Such letter suffixes
> This one let user to specify own low range under 4G
> for second kernel instead.
> 0: to disable low allocation.
> - It will be ignored when crashkernel_high=X is not used
> + It will be ignored when crashkernel=X,high is not used
> or memory reserved is below 4G.
>
> cs89x0_dma= [HW,NET]
> Index: linux-2.6/arch/x86/kernel/setup.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/setup.c
> +++ linux-2.6/arch/x86/kernel/setup.c
> @@ -526,7 +526,7 @@ static void __init reserve_crashkernel_l
> int ret;
>
> total_low_mem = memblock_mem_size(1UL<<(32-PAGE_SHIFT));
> - /* crashkernel_low=YM */
> + /* crashkernel=Y,low */
> ret = parse_crashkernel_low(boot_command_line, total_low_mem,
> &low_size, &base);
> if (ret != 0) {
> @@ -539,7 +539,7 @@ static void __init reserve_crashkernel_l
> low_size = swiotlb_size_or_default() + (8UL<<20);
> auto_set = true;
> } else {
> - /* passed with crashkernel_low=0 ? */
> + /* passed with crashkernel=0,low ? */
> if (!low_size)
> return;
> }
> @@ -579,7 +579,7 @@ static void __init reserve_crashkernel(v
> ret = parse_crashkernel(boot_command_line, total_mem,
> &crash_size, &crash_base);
> if (ret != 0 || crash_size <= 0) {
> - /* crashkernel_high=XM */
> + /* crashkernel=X,high */
> ret = parse_crashkernel_high(boot_command_line, total_mem,
> &crash_size, &crash_base);
> if (ret != 0 || crash_size <= 0)
> Index: linux-2.6/kernel/kexec.c
> ===================================================================
> --- linux-2.6.orig/kernel/kexec.c
> +++ linux-2.6/kernel/kexec.c
> @@ -1368,35 +1368,108 @@ static int __init parse_crashkernel_simp
> return 0;
> }
>
> +#define SUFFIX_HIGH 0
> +#define SUFFIX_LOW 1
> +#define SUFFIX_NULL 2
> +static __initdata char *suffix_tbl[] = {
> + [SUFFIX_HIGH] = ",high",
> + [SUFFIX_LOW] = ",low",
> + [SUFFIX_NULL] = NULL,
> +};
> +
> /*
> - * That function is the entry point for command line parsing and should be
> - * called from the arch-specific code.
> + * That function parses "suffix" crashkernel command lines like
> + *
> + * crashkernel=size,[high|low]
> + *
> + * It returns 0 on success and -EINVAL on failure.
> */
> +static int __init parse_crashkernel_suffix(char *cmdline,
> + unsigned long long *crash_size,
> + unsigned long long *crash_base,
> + const char *suffix)
> +{
> + char *cur = cmdline;
> +
> + *crash_size = memparse(cmdline, &cur);
> + if (cmdline == cur) {
> + pr_warn("crashkernel: memory value expected\n");
> + return -EINVAL;
> + }
> +
> + /* check with suffix */
> + if (!strncmp(cur, suffix, strlen(suffix)))
> + return 0;
> +
> + pr_warn("crashkernel: unrecognized char\n");
> + return -EINVAL;
> +}
> +
> +static __init char *get_last_crashkernel(char *cmdline,
> + const char *name,
> + const char *suffix)
> +{
> + char *p = cmdline, *ck_cmdline = NULL;
> +
> + /* find crashkernel and use the last one if there are more */
> + p = strstr(p, name);
> + while (p) {
> + char *end_p = strchr(p, ' ');
> + char *q;
> +
> + if (!end_p)
> + end_p = p + strlen(p);
> +
> + if (!suffix) {
> + int i;
> +
> + /* skip the one with any known suffix */
> + for (i = 0; suffix_tbl[i]; i++) {
> + q = end_p - strlen(suffix_tbl[i]);
> + if (!strncmp(q, suffix_tbl[i],
> + strlen(suffix_tbl[i])))
> + goto next;
> + }
> + ck_cmdline = p;
> + } else {
> + q = end_p - strlen(suffix);
> + if (!strncmp(q, suffix, strlen(suffix)))
> + ck_cmdline = p;
> + }
> +next:
> + p = strstr(p+1, name);
> + }
> +
> + if (!ck_cmdline)
> + return NULL;
> +
> + return ck_cmdline;
> +}
> +
> static int __init __parse_crashkernel(char *cmdline,
> unsigned long long system_ram,
> unsigned long long *crash_size,
> unsigned long long *crash_base,
> - const char *name)
> + const char *name,
> + const char *suffix)
> {
> - char *p = cmdline, *ck_cmdline = NULL;
> char *first_colon, *first_space;
> + char *ck_cmdline;
>
> BUG_ON(!crash_size || !crash_base);
> *crash_size = 0;
> *crash_base = 0;
>
> - /* find crashkernel and use the last one if there are more */
> - p = strstr(p, name);
> - while (p) {
> - ck_cmdline = p;
> - p = strstr(p+1, name);
> - }
> + ck_cmdline = get_last_crashkernel(cmdline, name, suffix);
>
> if (!ck_cmdline)
> return -EINVAL;
>
> ck_cmdline += strlen(name);
>
> + if (suffix)
> + return parse_crashkernel_suffix(ck_cmdline, crash_size,
> + crash_base, suffix);
> /*
> * if the commandline contains a ':', then that's the extended
> * syntax -- if not, it must be the classic syntax
> @@ -1413,13 +1486,17 @@ static int __init __parse_crashkernel(ch
> 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)
> {
> return __parse_crashkernel(cmdline, system_ram, crash_size, crash_base,
> - "crashkernel=");
> + "crashkernel=", NULL);
> }
>
> int __init parse_crashkernel_high(char *cmdline,
> @@ -1428,7 +1505,7 @@ int __init parse_crashkernel_high(char *
> unsigned long long *crash_base)
> {
> return __parse_crashkernel(cmdline, system_ram, crash_size, crash_base,
> - "crashkernel_high=");
> + "crashkernel=", suffix_tbl[SUFFIX_HIGH]);
> }
>
> int __init parse_crashkernel_low(char *cmdline,
> @@ -1437,7 +1514,7 @@ int __init parse_crashkernel_low(char *c
> unsigned long long *crash_base)
> {
> return __parse_crashkernel(cmdline, system_ram, crash_size, crash_base,
> - "crashkernel_low=");
> + "crashkernel=", suffix_tbl[SUFFIX_LOW]);
> }
>
> static void update_vmcoreinfo_note(void)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists