Different minor kexec cleanups

Different minor kexec cleanups

[Thomas Renninger]

Hi,

after some holidays and catching up, I like to re-visit the:
Pass memory ranges via e820 tables to crash kernel
work.
These are some cleanup patches on code I stumbled over while looking at
this. It would be great if these can get applied, so that my local branch
is in sync with mainline. I then like to proceed with further cleanups and
finally get to the point where a clean, split up in small patches submission
to implement above is possible.

Thanks,

       Thomas



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 1/4] kexec-tools: Cleanup: Fix indentation

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <Thomas Renninger" trenn@suse.de>
---
 kexec/arch/i386/crashdump-x86.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
index e44fceb..17c8744 100644
--- a/kexec/arch/i386/crashdump-x86.c
+++ b/kexec/arch/i386/crashdump-x86.c
@@ -559,11 +559,11 @@ static int add_memmap(struct memory_range *memmap_p, unsigned long long addr,
 		else if (addr > mend)
 			tidx = i+1;
 	}
-		/* Insert the memory region. */
-		for (j = nr_entries-1; j >= tidx; j--)
-			memmap_p[j+1] = memmap_p[j];
-		memmap_p[tidx].start = addr;
-		memmap_p[tidx].end = addr + size - 1;
+	/* Insert the memory region. */
+	for (j = nr_entries-1; j >= tidx; j--)
+		memmap_p[j+1] = memmap_p[j];
+	memmap_p[tidx].start = addr;
+	memmap_p[tidx].end = addr + size - 1;
 
 	dbgprintf("Memmap after adding segment\n");
 	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
-- 
1.7.6.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 2/4] kexec-tools: Introduce dbg_memrange() macro and make use of it

[Thomas Renninger]

There are several places where memory range arrays are printed for debug
reasons. In load_crashdump_segments() the memory type is missing.

Unify printing memory ranges in --debug case in a macro and add the first
users.

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <Thomas Renninger" trenn@suse.de>
---
 kexec/arch/i386/crashdump-x86.c    |    5 +----
 kexec/arch/i386/kexec-x86-common.c |    6 +-----
 kexec/arch/ppc/crashdump-powerpc.c |    8 +-------
 kexec/kexec.h                      |   11 +++++++++++
 4 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
index 17c8744..9b5a7cd 100644
--- a/kexec/arch/i386/crashdump-x86.c
+++ b/kexec/arch/i386/crashdump-x86.c
@@ -959,10 +959,7 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
 
 	get_backup_area(info, mem_range, nr_ranges);
 
-	dbgprintf("CRASH MEMORY RANGES\n");
-
-	for(i = 0; i < nr_ranges; ++i)
-		dbgprintf("%016Lx-%016Lx\n", mem_range[i].start, mem_range[i].end);
+	dbg_memrange("CRASH MEMORY RANGES", &mem_range, nr_ranges);
 
 	/*
 	 * if the core type has not been set on command line, set it here
diff --git a/kexec/arch/i386/kexec-x86-common.c b/kexec/arch/i386/kexec-x86-common.c
index ed6c950..015dcb6 100644
--- a/kexec/arch/i386/kexec-x86-common.c
+++ b/kexec/arch/i386/kexec-x86-common.c
@@ -477,11 +477,7 @@ int get_memory_ranges(struct memory_range **range, int *ranges,
 			mem_max = end;
 	}
 
-	dbgprintf("MEMORY RANGES\n");
-	for (i = 0; i < *ranges; i++) {
-		dbgprintf("%016Lx-%016Lx (%d)\n", (*range)[i].start,
-			  (*range)[i].end, (*range)[i].type);
-	}
+	dbg_memrange("MEMORY RANGES", range, *ranges);
 
 	return ret;
 }
diff --git a/kexec/arch/ppc/crashdump-powerpc.c b/kexec/arch/ppc/crashdump-powerpc.c
index eee5b37..feb3cfd 100644
--- a/kexec/arch/ppc/crashdump-powerpc.c
+++ b/kexec/arch/ppc/crashdump-powerpc.c
@@ -236,13 +236,7 @@ static int get_crash_memory_ranges(struct memory_range **range, int *ranges)
 	*ranges = memory_ranges;
 
 	int j;
-	dbgprintf("CRASH MEMORY RANGES\n");
-	for (j = 0; j < *ranges; j++) {
-		start = crash_memory_range[j].start;
-		end = crash_memory_range[j].end;
-		dbgprintf("%016Lx-%016Lx\n", start, end);
-	}
-
+	dbg_memrange("MEMORY RANGES", &crash_memory_range, *ranges);
 	return 0;
 
 err:
diff --git a/kexec/kexec.h b/kexec/kexec.h
index 715b568..eea8c50 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -121,6 +121,17 @@ do { \
 		fprintf(stderr, __VA_ARGS__); \
 } while(0)
 
+#define dbg_memrange(title, range, nr) \
+do { \
+ int i; \
+ dbgprintf(title "\n"); \
+ for (i = 0; i < nr; i++) { \
+	 dbgprintf("%016Lx-%016Lx (%d)\n", (*range)[i].start,	\
+		   (*range)[i].end, (*range)[i].type);		\
+ } \
+ } while (0)
+
+
 struct kexec_segment {
 	const void *buf;
 	size_t bufsz;
-- 
1.7.6.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 3/4] kexec-tools: Fix possible overflows and make use of dbg_memrange() macro

[Thomas Renninger]

add_memmap() will add another memrange, therefore we need an additional
array entry and need to check for
if (nr_entries >= CRASH_MAX_MEMMAP_NR - 1)

Same for delete_memmap: If a region has to be split an additional region is
added first, so we again have to check for:
if (nr_entries >= CRASH_MAX_MEMMAP_NR - 1)

In add_memmap we know the amount of range entries. No need to check for the
ugly:
-               if (mstart == 0 && mend == 0)
-                       break;
condition, just let the loop go until nr_entries.

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <Thomas Renninger" trenn@suse.de>
---
 kexec/arch/i386/crashdump-x86.c |   35 ++++++++---------------------------
 1 files changed, 8 insertions(+), 27 deletions(-)

diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
index 9b5a7cd..7fd1c5b 100644
--- a/kexec/arch/i386/crashdump-x86.c
+++ b/kexec/arch/i386/crashdump-x86.c
@@ -545,14 +545,12 @@ static int add_memmap(struct memory_range *memmap_p, unsigned long long addr,
 		else
 			nr_entries++;
 	}
-	if (nr_entries == CRASH_MAX_MEMMAP_NR)
+	if (nr_entries >= CRASH_MAX_MEMMAP_NR - 1)
 		return -1;
 
-	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
+	for (i = 0; i < nr_entries;  i++) {
 		mstart = memmap_p[i].start;
 		mend = memmap_p[i].end;
-		if (mstart == 0 && mend == 0)
-			break;
 		if (mstart <= (addr+size-1) && mend >=addr)
 			/* Overlapping region. */
 			return -1;
@@ -565,16 +563,8 @@ static int add_memmap(struct memory_range *memmap_p, unsigned long long addr,
 	memmap_p[tidx].start = addr;
 	memmap_p[tidx].end = addr + size - 1;
 
-	dbgprintf("Memmap after adding segment\n");
-	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
-		mstart = memmap_p[i].start;
-		mend = memmap_p[i].end;
-		if (mstart == 0 && mend == 0)
-			break;
-		dbgprintf("%016llx - %016llx\n",
-			mstart, mend);
-	}
-
+	nr_entries++;
+	dbg_memrange("Memmap after adding segment", &memmap_p, nr_entries);
 	return 0;
 }
 
@@ -600,8 +590,7 @@ static int delete_memmap(struct memory_range *memmap_p, unsigned long long addr,
 		else
 			nr_entries++;
 	}
-	if (nr_entries == CRASH_MAX_MEMMAP_NR)
-		/* List if full */
+	if (nr_entries >= CRASH_MAX_MEMMAP_NR - 1)
 		return -1;
 
 	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
@@ -643,25 +632,17 @@ static int delete_memmap(struct memory_range *memmap_p, unsigned long long addr,
 		for (j = nr_entries-1; j > tidx; j--)
 			memmap_p[j+1] = memmap_p[j];
 		memmap_p[tidx+1] = temp_region;
+		nr_entries++;
 	}
 	if ((operation == -1) && tidx >=0) {
 		/* Delete the exact match memory region. */
 		for (j = i+1; j < CRASH_MAX_MEMMAP_NR; j++)
 			memmap_p[j-1] = memmap_p[j];
 		memmap_p[j-1].start = memmap_p[j-1].end = 0;
+		nr_entries--;
 	}
 
-	dbgprintf("Memmap after deleting segment\n");
-	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
-		mstart = memmap_p[i].start;
-		mend = memmap_p[i].end;
-		if (mstart == 0 && mend == 0) {
-			break;
-		}
-		dbgprintf("%016llx - %016llx\n",
-			mstart, mend);
-	}
-
+	dbg_memrange("Memmap after deleting segment", &memmap_p, nr_entries);
 	return 0;
 }
 
-- 
1.7.6.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 4/4] kexec-tools: Add cgroup_disable=memory to crash kernel parameters

[Thomas Renninger]

cgroup memory subsystem pre-allocates quite some memory at boot up.
Example dmesg:
allocated 469762048 bytes of page_cgroup
please try 'cgroup_disable=memory' option if you don't want memory cgroups

Due to maxcpu=1 it is by far not that much than in a productive kernel.
It still seem to be around 4M statically as no Numa overhead exists with
only one active CPU. It is still worth to disable cgroup memory
pre-allocating in crash kernel environment.

Signed-off-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Thomas Renninger <Thomas Renninger" trenn@suse.de>
---
 kexec/arch/i386/crashdump-x86.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
index 7fd1c5b..e6d60a9 100644
--- a/kexec/arch/i386/crashdump-x86.c
+++ b/kexec/arch/i386/crashdump-x86.c
@@ -757,6 +757,23 @@ static int cmdline_add_elfcorehdr(char *cmdline, unsigned long addr)
 	return 0;
 }
 
+static void cmdline_disable_cgroup(char *cmdline)
+{
+	int cmdlen;
+	char str[30] = " cgroup_disable=memory";
+
+	cmdlen = strlen(cmdline) + strlen(str);
+	if (cmdlen > (COMMAND_LINE_SIZE - 1)) {
+		/* Not fatal if cgroup memory is not disabled */
+		dbgprintf(
+		  "Command line overflow, cgroup memory not disabled\n");
+		return;
+	}
+	strcat(cmdline, str);
+
+	dbgprintf("Command line after adding adding cgroup_disable=memory\n");
+	dbgprintf("%s\n", cmdline);
+}
 
 /*
  * This routine is specific to i386 architecture to maintain the
@@ -1037,6 +1054,11 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
 		end = mem_range[i].end;
 		cmdline_add_memmap_acpi(mod_cmdline, start, end);
 	}
+	/* 
+	 * Always add disabling cgroup_diable=memory param at the end,
+	 * it's not critical if it falls off
+	 */
+	cmdline_disable_cgroup(mod_cmdline);
 	return 0;
 }
 
-- 
1.7.6.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/4] kexec-tools: Cleanup: Fix indentation

[Simon Horman]

On Wed, May 22, 2013 at 10:57:33AM +0200, Thomas Renninger wrote:
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> Signed-off-by: Thomas Renninger <Thomas Renninger" trenn@suse.de>
> ---
>  kexec/arch/i386/crashdump-x86.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
> index e44fceb..17c8744 100644
> --- a/kexec/arch/i386/crashdump-x86.c
> +++ b/kexec/arch/i386/crashdump-x86.c
> @@ -559,11 +559,11 @@ static int add_memmap(struct memory_range *memmap_p, unsigned long long addr,
>  		else if (addr > mend)
>  			tidx = i+1;
>  	}
> -		/* Insert the memory region. */
> -		for (j = nr_entries-1; j >= tidx; j--)
> -			memmap_p[j+1] = memmap_p[j];
> -		memmap_p[tidx].start = addr;
> -		memmap_p[tidx].end = addr + size - 1;
> +	/* Insert the memory region. */
> +	for (j = nr_entries-1; j >= tidx; j--)
> +		memmap_p[j+1] = memmap_p[j];

While you are at it could you also put spaces around the '-' and '+' in
the lines above?

> +	memmap_p[tidx].start = addr;
> +	memmap_p[tidx].end = addr + size - 1;
>  
>  	dbgprintf("Memmap after adding segment\n");
>  	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
> -- 
> 1.7.6.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 3/4] kexec-tools: Fix possible overflows and make use of dbg_memrange() macro

[Simon Horman]

On Wed, May 22, 2013 at 10:57:35AM +0200, Thomas Renninger wrote:
> add_memmap() will add another memrange, therefore we need an additional
> array entry and need to check for
> if (nr_entries >= CRASH_MAX_MEMMAP_NR - 1)
> 
> Same for delete_memmap: If a region has to be split an additional region is
> added first, so we again have to check for:
> if (nr_entries >= CRASH_MAX_MEMMAP_NR - 1)
> 
> In add_memmap we know the amount of range entries. No need to check for the
> ugly:
> -               if (mstart == 0 && mend == 0)
> -                       break;
> condition, just let the loop go until nr_entries.
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> Signed-off-by: Thomas Renninger <Thomas Renninger" trenn@suse.de>

This patch seems fine, however, the second Signed-off-by line seems to be a
malformed duplicate of the first.

> ---
>  kexec/arch/i386/crashdump-x86.c |   35 ++++++++---------------------------
>  1 files changed, 8 insertions(+), 27 deletions(-)
> 
> diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
> index 9b5a7cd..7fd1c5b 100644
> --- a/kexec/arch/i386/crashdump-x86.c
> +++ b/kexec/arch/i386/crashdump-x86.c
> @@ -545,14 +545,12 @@ static int add_memmap(struct memory_range *memmap_p, unsigned long long addr,
>  		else
>  			nr_entries++;
>  	}
> -	if (nr_entries == CRASH_MAX_MEMMAP_NR)
> +	if (nr_entries >= CRASH_MAX_MEMMAP_NR - 1)
>  		return -1;
>  
> -	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
> +	for (i = 0; i < nr_entries;  i++) {
>  		mstart = memmap_p[i].start;
>  		mend = memmap_p[i].end;
> -		if (mstart == 0 && mend == 0)
> -			break;
>  		if (mstart <= (addr+size-1) && mend >=addr)
>  			/* Overlapping region. */
>  			return -1;
> @@ -565,16 +563,8 @@ static int add_memmap(struct memory_range *memmap_p, unsigned long long addr,
>  	memmap_p[tidx].start = addr;
>  	memmap_p[tidx].end = addr + size - 1;
>  
> -	dbgprintf("Memmap after adding segment\n");
> -	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
> -		mstart = memmap_p[i].start;
> -		mend = memmap_p[i].end;
> -		if (mstart == 0 && mend == 0)
> -			break;
> -		dbgprintf("%016llx - %016llx\n",
> -			mstart, mend);
> -	}
> -
> +	nr_entries++;
> +	dbg_memrange("Memmap after adding segment", &memmap_p, nr_entries);
>  	return 0;
>  }
>  
> @@ -600,8 +590,7 @@ static int delete_memmap(struct memory_range *memmap_p, unsigned long long addr,
>  		else
>  			nr_entries++;
>  	}
> -	if (nr_entries == CRASH_MAX_MEMMAP_NR)
> -		/* List if full */
> +	if (nr_entries >= CRASH_MAX_MEMMAP_NR - 1)
>  		return -1;
>  
>  	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
> @@ -643,25 +632,17 @@ static int delete_memmap(struct memory_range *memmap_p, unsigned long long addr,
>  		for (j = nr_entries-1; j > tidx; j--)
>  			memmap_p[j+1] = memmap_p[j];
>  		memmap_p[tidx+1] = temp_region;
> +		nr_entries++;
>  	}
>  	if ((operation == -1) && tidx >=0) {
>  		/* Delete the exact match memory region. */
>  		for (j = i+1; j < CRASH_MAX_MEMMAP_NR; j++)
>  			memmap_p[j-1] = memmap_p[j];
>  		memmap_p[j-1].start = memmap_p[j-1].end = 0;
> +		nr_entries--;
>  	}
>  
> -	dbgprintf("Memmap after deleting segment\n");
> -	for (i = 0; i < CRASH_MAX_MEMMAP_NR;  i++) {
> -		mstart = memmap_p[i].start;
> -		mend = memmap_p[i].end;
> -		if (mstart == 0 && mend == 0) {
> -			break;
> -		}
> -		dbgprintf("%016llx - %016llx\n",
> -			mstart, mend);
> -	}
> -
> +	dbg_memrange("Memmap after deleting segment", &memmap_p, nr_entries);
>  	return 0;
>  }
>  
> -- 
> 1.7.6.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/4] kexec-tools: Introduce dbg_memrange() macro and make use of it

[Simon Horman]

On Wed, May 22, 2013 at 10:57:34AM +0200, Thomas Renninger wrote:
> There are several places where memory range arrays are printed for debug
> reasons. In load_crashdump_segments() the memory type is missing.
> 
> Unify printing memory ranges in --debug case in a macro and add the first
> users.
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> Signed-off-by: Thomas Renninger <Thomas Renninger" trenn@suse.de>

This patch seems fine, but the second signed-off-by line seems to
be a malformed duplicate of the first one.

> ---
>  kexec/arch/i386/crashdump-x86.c    |    5 +----
>  kexec/arch/i386/kexec-x86-common.c |    6 +-----
>  kexec/arch/ppc/crashdump-powerpc.c |    8 +-------
>  kexec/kexec.h                      |   11 +++++++++++
>  4 files changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
> index 17c8744..9b5a7cd 100644
> --- a/kexec/arch/i386/crashdump-x86.c
> +++ b/kexec/arch/i386/crashdump-x86.c
> @@ -959,10 +959,7 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
>  
>  	get_backup_area(info, mem_range, nr_ranges);
>  
> -	dbgprintf("CRASH MEMORY RANGES\n");
> -
> -	for(i = 0; i < nr_ranges; ++i)
> -		dbgprintf("%016Lx-%016Lx\n", mem_range[i].start, mem_range[i].end);
> +	dbg_memrange("CRASH MEMORY RANGES", &mem_range, nr_ranges);
>  
>  	/*
>  	 * if the core type has not been set on command line, set it here
> diff --git a/kexec/arch/i386/kexec-x86-common.c b/kexec/arch/i386/kexec-x86-common.c
> index ed6c950..015dcb6 100644
> --- a/kexec/arch/i386/kexec-x86-common.c
> +++ b/kexec/arch/i386/kexec-x86-common.c
> @@ -477,11 +477,7 @@ int get_memory_ranges(struct memory_range **range, int *ranges,
>  			mem_max = end;
>  	}
>  
> -	dbgprintf("MEMORY RANGES\n");
> -	for (i = 0; i < *ranges; i++) {
> -		dbgprintf("%016Lx-%016Lx (%d)\n", (*range)[i].start,
> -			  (*range)[i].end, (*range)[i].type);
> -	}
> +	dbg_memrange("MEMORY RANGES", range, *ranges);
>  
>  	return ret;
>  }
> diff --git a/kexec/arch/ppc/crashdump-powerpc.c b/kexec/arch/ppc/crashdump-powerpc.c
> index eee5b37..feb3cfd 100644
> --- a/kexec/arch/ppc/crashdump-powerpc.c
> +++ b/kexec/arch/ppc/crashdump-powerpc.c
> @@ -236,13 +236,7 @@ static int get_crash_memory_ranges(struct memory_range **range, int *ranges)
>  	*ranges = memory_ranges;
>  
>  	int j;
> -	dbgprintf("CRASH MEMORY RANGES\n");
> -	for (j = 0; j < *ranges; j++) {
> -		start = crash_memory_range[j].start;
> -		end = crash_memory_range[j].end;
> -		dbgprintf("%016Lx-%016Lx\n", start, end);
> -	}
> -
> +	dbg_memrange("MEMORY RANGES", &crash_memory_range, *ranges);
>  	return 0;
>  
>  err:
> diff --git a/kexec/kexec.h b/kexec/kexec.h
> index 715b568..eea8c50 100644
> --- a/kexec/kexec.h
> +++ b/kexec/kexec.h
> @@ -121,6 +121,17 @@ do { \
>  		fprintf(stderr, __VA_ARGS__); \
>  } while(0)
>  
> +#define dbg_memrange(title, range, nr) \
> +do { \
> + int i; \
> + dbgprintf(title "\n"); \
> + for (i = 0; i < nr; i++) { \
> +	 dbgprintf("%016Lx-%016Lx (%d)\n", (*range)[i].start,	\
> +		   (*range)[i].end, (*range)[i].type);		\
> + } \
> + } while (0)
> +
> +
>  struct kexec_segment {
>  	const void *buf;
>  	size_t bufsz;
> -- 
> 1.7.6.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/4] kexec-tools: Add cgroup_disable=memory to crash kernel parameters

[Simon Horman]

On Wed, May 22, 2013 at 10:57:36AM +0200, Thomas Renninger wrote:
> cgroup memory subsystem pre-allocates quite some memory at boot up.
> Example dmesg:
> allocated 469762048 bytes of page_cgroup
> please try 'cgroup_disable=memory' option if you don't want memory cgroups
> 
> Due to maxcpu=1 it is by far not that much than in a productive kernel.
> It still seem to be around 4M statically as no Numa overhead exists with
> only one active CPU. It is still worth to disable cgroup memory
> pre-allocating in crash kernel environment.
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> Signed-off-by: Thomas Renninger <Thomas Renninger" trenn@suse.de>

I'm not really sure this kind of logic should be hard-coded into
kexec-tools. --append should allow callers of kexec to add this
option if it is appropriate.

> ---
>  kexec/arch/i386/crashdump-x86.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
> index 7fd1c5b..e6d60a9 100644
> --- a/kexec/arch/i386/crashdump-x86.c
> +++ b/kexec/arch/i386/crashdump-x86.c
> @@ -757,6 +757,23 @@ static int cmdline_add_elfcorehdr(char *cmdline, unsigned long addr)
>  	return 0;
>  }
>  
> +static void cmdline_disable_cgroup(char *cmdline)
> +{
> +	int cmdlen;
> +	char str[30] = " cgroup_disable=memory";
> +
> +	cmdlen = strlen(cmdline) + strlen(str);
> +	if (cmdlen > (COMMAND_LINE_SIZE - 1)) {
> +		/* Not fatal if cgroup memory is not disabled */
> +		dbgprintf(
> +		  "Command line overflow, cgroup memory not disabled\n");
> +		return;
> +	}
> +	strcat(cmdline, str);
> +
> +	dbgprintf("Command line after adding adding cgroup_disable=memory\n");
> +	dbgprintf("%s\n", cmdline);
> +}
>  
>  /*
>   * This routine is specific to i386 architecture to maintain the
> @@ -1037,6 +1054,11 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
>  		end = mem_range[i].end;
>  		cmdline_add_memmap_acpi(mod_cmdline, start, end);
>  	}
> +	/* 
> +	 * Always add disabling cgroup_diable=memory param at the end,
> +	 * it's not critical if it falls off
> +	 */
> +	cmdline_disable_cgroup(mod_cmdline);
>  	return 0;
>  }
>  
> -- 
> 1.7.6.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 4/4] kexec-tools: Add cgroup_disable=memory to crash kernel parameters

[Vivek Goyal]

On Sun, May 26, 2013 at 10:20:21PM +0900, Simon Horman wrote:
> On Wed, May 22, 2013 at 10:57:36AM +0200, Thomas Renninger wrote:
> > cgroup memory subsystem pre-allocates quite some memory at boot up.
> > Example dmesg:
> > allocated 469762048 bytes of page_cgroup
> > please try 'cgroup_disable=memory' option if you don't want memory cgroups
> > 
> > Due to maxcpu=1 it is by far not that much than in a productive kernel.
> > It still seem to be around 4M statically as no Numa overhead exists with
> > only one active CPU. It is still worth to disable cgroup memory
> > pre-allocating in crash kernel environment.
> > 
> > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > Signed-off-by: Thomas Renninger <Thomas Renninger" trenn@suse.de>
> 
> I'm not really sure this kind of logic should be hard-coded into
> kexec-tools. --append should allow callers of kexec to add this
> option if it is appropriate.

Agreed. cgroup_disable=memory is better not hard coded here. In F19
default config we have cgroup_disable=memory specified which is passed
to kexec-tools using --append option.

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec