[PATCH 0/3] makedumpfile: calculate the size of cyclic buffer automatically.

[PATCH 0/3] makedumpfile: calculate the size of cyclic buffer automatically.

[Atsushi Kumagai]

Hello,

I made the patch set based on v1.5.1-beta to calculate the size of cyclic 
buffer automatically.

In v1.5.0, users have to specify the buffer size depending on system memory
size with --cyclic-buffer option in order to get decent performance.

This patch set avoids the inconvenience above by choosing the lesser value
of the two below as the size of cyclic buffer automatically:

  a. the size enough for storing the 1st/2nd bitmap for the whole of vmcore
  b. 80% of free memory (as safety limit)

Additionally, this patch set remove --cyclic-buffer option because I think
it has been already unnecessary.

If there is no comments, I'll merge this patch set into v1.5.1-rc.


Thanks
Atsushi Kumagai

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

[PATCH 1/3] Add get_free_memory_size() to get the amount of free memory.

[Atsushi Kumagai]

get_free_memory_size() reads /proc/meminfo and consider "MemFree:"
the amount of free memory.

Signed-off-by: Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp>
---
 makedumpfile.c | 34 ++++++++++++++++++++++++++++++++++
 makedumpfile.h |  1 +
 2 files changed, 35 insertions(+)

diff --git a/makedumpfile.c b/makedumpfile.c
index 1183330..73a466e 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -7972,6 +7972,40 @@ out:
 	return ret;
 }
 
+/*
+ * Get the amount of free memory from /proc/meminfo.
+ */
+unsigned long long
+get_free_memory_size(void) {
+	char buf[BUFSIZE_FGETS];
+	char unit[4];
+	unsigned long long free_size = 0;
+	char *name_meminfo = "/proc/meminfo";
+	FILE *file_meminfo;
+
+	if ((file_meminfo = fopen(name_meminfo, "r")) == NULL) {
+		ERRMSG("Can't open the %s. %s\n", name_meminfo, strerror(errno));
+		return FALSE;
+	}
+
+	while (fgets(buf, BUFSIZE_FGETS, file_meminfo) != NULL) {
+		if (sscanf(buf, "MemFree: %llu %s", &free_size, unit) == 2) {
+			if (strcmp(unit, "kB") == 0) {
+				free_size *= 1024;
+				goto out;
+			}
+		}
+	}
+
+	ERRMSG("Can't get free memory size.\n");
+	free_size = 0;
+out:
+	if (fclose(file_meminfo) < 0)
+		ERRMSG("Can't close the %s. %s\n", name_meminfo, strerror(errno));
+
+	return free_size;
+}
+
 static struct option longopts[] = {
 	{"split", no_argument, NULL, 's'}, 
 	{"reassemble", no_argument, NULL, 'r'},
diff --git a/makedumpfile.h b/makedumpfile.h
index 97aca2a..c91b4e9 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1611,5 +1611,6 @@ struct elf_prstatus {
 unsigned long long get_num_dumpable_cyclic(void);
 int get_loads_dumpfile_cyclic(void);
 int initial_xen(void);
+unsigned long long get_free_memory_size(void);
 
 #endif /* MAKEDUMPFILE_H */
-- 
1.7.11

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

[PATCH 2/3] Calculate the size of cyclic buffer automatically.

[Atsushi Kumagai]

The size of cyclic buffer is set to DEFAULT_BUFSIZE_CYCLIC(1MB) when
the size isn't specified. But 1MB is too small for large memory machine
and the number of cycle will be so large. As a result, a lot of time
will be spend for dump filtering.

To resolve the issue above, I add calculate_cyclic_buffer_size() to
calculate the buffer size as needed.
Concretely, the logic is to choose the lesser value of the two below
as the size of cyclic buffer:

  a. the size enough for storing the 1st/2nd bitmap for the whole of vmcore
  b. 80% of free memory (as safety limit)

calculate_cyclic_buffer_size() uses max_mapnr, so it needs to be done
after get_max_mapnr().

Signed-off-by: Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp>
---
 makedumpfile.c | 70 +++++++++++++++++++++++++++++++++++++++-------------------
 makedumpfile.h |  6 +----
 2 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index 73a466e..f784400 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -2573,29 +2573,6 @@ initial(void)
 	}
 #endif
 
-	if (info->flag_cyclic) {
-		/*
-		 * buffer size is specified as Kbyte
-		 */
-		if (info->bufsize_cyclic == 0)
-			info->bufsize_cyclic = DEFAULT_BUFSIZE_CYCLIC;
-		else
-			info->bufsize_cyclic <<= 10;
-	
-		/*
-		 * Max buffer size is 100 MB
-		 */
-		if (info->bufsize_cyclic > (100 << 20)) {
-			MSG("Specified buffer size is too large, ");
-			MSG("The buffer size for the cyclic mode will be truncated to 100 MB.\n");
-			info->bufsize_cyclic = (100 << 20);
-		}
-		info->pfn_cyclic = info->bufsize_cyclic * BITPERBYTE;
-
-		DEBUG_MSG("\n");
-		DEBUG_MSG("Buffer size for the cyclic mode: %ld\n", info->bufsize_cyclic);
-	}
-
 	if (info->flag_exclude_xen_dom) {
 		if(info->flag_cyclic) {
 			info->flag_cyclic = FALSE;
@@ -2737,6 +2714,30 @@ out:
 	if (!get_max_mapnr())
 		return FALSE;
 
+	if (info->flag_cyclic) {
+		/*
+		 * buffer size is specified as Kbyte
+		 */
+		if (info->bufsize_cyclic == 0) {
+			if (!calculate_cyclic_buffer_size())
+				return FALSE;
+		} else
+			info->bufsize_cyclic <<= 10;
+
+		/*
+		 * Max buffer size is 100 MB
+		 */
+		if (info->bufsize_cyclic > (100 << 20)) {
+			MSG("Specified buffer size is too large, ");
+			MSG("The buffer size for the cyclic mode will be truncated to 100 MB.\n");
+			info->bufsize_cyclic = (100 << 20);
+		}
+		info->pfn_cyclic = info->bufsize_cyclic * BITPERBYTE;
+
+		DEBUG_MSG("\n");
+		DEBUG_MSG("Buffer size for the cyclic mode: %ld\n", info->bufsize_cyclic);
+	}
+
 	if (debug_info) {
 		if (info->flag_sadump)
 			(void) sadump_virt_phys_base();
@@ -8006,6 +8007,29 @@ out:
 	return free_size;
 }
 
+
+/*
+ * Choose the lesser value of the two below as the size of cyclic buffer.
+ *  - the size enough for storing the 1st/2nd bitmap for the whole of vmcore
+ *  - 80% of free memory
+ */
+int
+calculate_cyclic_buffer_size(void) {
+	unsigned long long free_size, needed_size;
+
+	if (info->max_mapnr <= 0) {
+		ERRMSG("Invalid max_mapnr(%llu).\n", info->max_mapnr);
+		return FALSE;
+	}
+
+	free_size = get_free_memory_size() * 0.8;
+	needed_size = (info->max_mapnr * 2) / BITPERBYTE;
+
+	info->bufsize_cyclic = (free_size <= needed_size) ? free_size : needed_size;
+
+	return TRUE;
+}
+
 static struct option longopts[] = {
 	{"split", no_argument, NULL, 's'}, 
 	{"reassemble", no_argument, NULL, 'r'},
diff --git a/makedumpfile.h b/makedumpfile.h
index c91b4e9..8ffef76 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -199,11 +199,6 @@ isAnon(unsigned long mapping)
 #define FILENAME_STDOUT		"STDOUT"
 
 /*
- * For cyclic processing
- */
-#define DEFAULT_BUFSIZE_CYCLIC         (1024 * 1024)
-
-/*
  * Minimam vmcore has 2 ProgramHeaderTables(PT_NOTE and PT_LOAD).
  */
 #define MIN_ELF32_HEADER_SIZE \
@@ -1612,5 +1607,6 @@ unsigned long long get_num_dumpable_cyclic(void);
 int get_loads_dumpfile_cyclic(void);
 int initial_xen(void);
 unsigned long long get_free_memory_size(void);
+int calculate_cyclic_buffer_size(void);
 
 #endif /* MAKEDUMPFILE_H */
-- 
1.7.11

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

[PATCH 3/3] Remove --cyclic-buffer option.

[Atsushi Kumagai]

The size of cyclic buffer will be calculated automatically with
[PATCH 2/3], so users don't have to specify the size manually.

Signed-off-by: Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp>
---
 makedumpfile.8 | 16 ----------------
 makedumpfile.c | 31 ++++++-------------------------
 print_info.c   | 11 -----------
 3 files changed, 6 insertions(+), 52 deletions(-)

diff --git a/makedumpfile.8 b/makedumpfile.8
index 9f394f0..41d5822 100644
--- a/makedumpfile.8
+++ b/makedumpfile.8
@@ -348,22 +348,6 @@ on the following example.
 # makedumpfile \-\-reassemble dumpfile1 dumpfile2 dumpfile
 
 .TP
-\fB\-\-cyclic\-buffer\fR \fIbuffer_size\fR
-Specify the buffer size in kilo bytes for analysis in the cyclic mode.
-Actually, the double of \fIbuffer_size\fR kilo bytes will be allocated in memory.
-In the cyclic mode, the number of cycles is represented as:
-
-    num_of_cycles = system_memory / (\fIbuffer_size\fR * 1024 * bit_per_bytes * page_size )
-
-The lesser number of cycles, the faster working speed is expected.
-Default \fIbuffer_size\fR is 1024.
-
-.br
-.B Example:
-.br
-# makedumpfile \-\-cyclic\-buffer 1024 \-d 31 \-x vmlinux /proc/vmcore dumpfile
-
-.TP
 \fB\-\-non\-cyclic\fR
 Running in the non-cyclic mode, this mode uses the old filtering logic same as v1.4.4 or before.
 If you feel the cyclic mode is too slow, please try this mode.
diff --git a/makedumpfile.c b/makedumpfile.c
index f784400..ac05a87 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -2715,27 +2715,8 @@ out:
 		return FALSE;
 
 	if (info->flag_cyclic) {
-		/*
-		 * buffer size is specified as Kbyte
-		 */
-		if (info->bufsize_cyclic == 0) {
-			if (!calculate_cyclic_buffer_size())
-				return FALSE;
-		} else
-			info->bufsize_cyclic <<= 10;
-
-		/*
-		 * Max buffer size is 100 MB
-		 */
-		if (info->bufsize_cyclic > (100 << 20)) {
-			MSG("Specified buffer size is too large, ");
-			MSG("The buffer size for the cyclic mode will be truncated to 100 MB.\n");
-			info->bufsize_cyclic = (100 << 20);
-		}
-		info->pfn_cyclic = info->bufsize_cyclic * BITPERBYTE;
-
-		DEBUG_MSG("\n");
-		DEBUG_MSG("Buffer size for the cyclic mode: %ld\n", info->bufsize_cyclic);
+		if (!calculate_cyclic_buffer_size())
+			return FALSE;
 	}
 
 	if (debug_info) {
@@ -8026,6 +8007,10 @@ calculate_cyclic_buffer_size(void) {
 	needed_size = (info->max_mapnr * 2) / BITPERBYTE;
 
 	info->bufsize_cyclic = (free_size <= needed_size) ? free_size : needed_size;
+	info->pfn_cyclic = info->bufsize_cyclic * BITPERBYTE;
+
+	DEBUG_MSG("\n");
+	DEBUG_MSG("Buffer size for the cyclic mode: %ld\n", info->bufsize_cyclic);
 
 	return TRUE;
 }
@@ -8043,7 +8028,6 @@ static struct option longopts[] = {
 	{"help", no_argument, NULL, 'h'},
 	{"diskset", required_argument, NULL, 'k'},
 	{"non-cyclic", no_argument, NULL, 'Y'},
-	{"cyclic-buffer", required_argument, NULL, 'Z'},
 	{0, 0, 0, 0}
 };
 
@@ -8167,9 +8151,6 @@ main(int argc, char *argv[])
 			info->flag_read_vmcoreinfo = 1;
 			info->name_vmcoreinfo = optarg;
 			break;
-		case 'Z':
-			info->bufsize_cyclic = atoi(optarg);
-			break;
 		case '?':
 			MSG("Commandline parameter is invalid.\n");
 			MSG("Try `makedumpfile --help' for more information.\n");
diff --git a/print_info.c b/print_info.c
index f3ee43a..ac96448 100644
--- a/print_info.c
+++ b/print_info.c
@@ -168,17 +168,6 @@ print_usage(void)
 	MSG("      Reassemble multiple DUMPFILEs, which are created by --split option,\n");
 	MSG("      into one DUMPFILE. dumpfile1 and dumpfile2 are reassembled into dumpfile.\n");
 	MSG("\n");
-	MSG("  [--cyclic-buffer BUFFER_SIZE]:\n");
-	MSG("      Specify the buffer size in kilo bytes for analysis in the cyclic mode.\n");
-	MSG("      Actually, the double of BUFFER_SIZE kilo bytes will be allocated in memory.\n");
-	MSG("      In the cyclic mode, the number of cycles is represented as:\n");
-	MSG("\n");
-	MSG("          num_of_cycles = system_memory / \n");
-	MSG("                          (BUFFER_SIZE * 1024 * bit_per_bytes * page_size)\n");
-	MSG("\n");
-	MSG("      The lesser number of cycles, the faster working speed is expected.\n");
-	MSG("      Default BUFFER_SIZE is 1024.\n");
-	MSG("\n");
 	MSG("  [--non-cyclic]:\n");
 	MSG("      Running in the non-cyclic mode, this mode uses the old filtering logic\n");
 	MSG("      same as v1.4.4 or before.\n");
-- 
1.7.11

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

Re: [PATCH 0/3] makedumpfile: calculate the size of cyclic buffer automatically.

[Lisa Mitchell]

On Fri, 2012-11-09 at 14:17 +0000, Vivek Goyal wrote:
> On Fri, Nov 09, 2012 at 04:02:14PM +0900, Atsushi Kumagai wrote:
> > Hello,
> > 
> > I made the patch set based on v1.5.1-beta to calculate the size of cyclic 
> > buffer automatically.
> > 
> > In v1.5.0, users have to specify the buffer size depending on system memory
> > size with --cyclic-buffer option in order to get decent performance.
> > 
> > This patch set avoids the inconvenience above by choosing the lesser value
> > of the two below as the size of cyclic buffer automatically:
> > 
> >   a. the size enough for storing the 1st/2nd bitmap for the whole of vmcore
> >   b. 80% of free memory (as safety limit)
> > 
> > Additionally, this patch set remove --cyclic-buffer option because I think
> > it has been already unnecessary.
> 
> I was wondering how about retaining --cyclic-buffer <size> option. If 
> there are use cases where user's don't like default of 80% of free memory
> they can override this policy by specifying --cyclic-buffer.
> 
> Thanks
> Vivek

I second that, I'd like the --cyclic-buffer option to stay.  I see PATCH
3/3 to remove cyclic buffer option, and I also would like it left there,
for debug of issues that may come up later.  It's great that the code
will automatically calculate the best cyclic buffer option, which is
what it should do for the best customer out-of-the-box dump enablement,
but the ability to manipulate it for debug purposes, or for workarounds
if things go wrong, is valuable. 



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

Re: [PATCH 0/3] makedumpfile: calculate the size of cyclic buffer automatically.

[Vivek Goyal]

On Fri, Nov 09, 2012 at 04:02:14PM +0900, Atsushi Kumagai wrote:
> Hello,
> 
> I made the patch set based on v1.5.1-beta to calculate the size of cyclic 
> buffer automatically.
> 
> In v1.5.0, users have to specify the buffer size depending on system memory
> size with --cyclic-buffer option in order to get decent performance.
> 
> This patch set avoids the inconvenience above by choosing the lesser value
> of the two below as the size of cyclic buffer automatically:
> 
>   a. the size enough for storing the 1st/2nd bitmap for the whole of vmcore
>   b. 80% of free memory (as safety limit)
> 
> Additionally, this patch set remove --cyclic-buffer option because I think
> it has been already unnecessary.

I was wondering how about retaining --cyclic-buffer <size> option. If 
there are use cases where user's don't like default of 80% of free memory
they can override this policy by specifying --cyclic-buffer.

Thanks
Vivek

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

Re: [PATCH 0/3] makedumpfile: calculate the size of cyclic buffer automatically.

[Atsushi Kumagai]

Hello,

On Fri, 09 Nov 2012 03:46:21 -0700
Lisa Mitchell <lisa.mitchell@hp.com> wrote:

> On Fri, 2012-11-09 at 14:17 +0000, Vivek Goyal wrote:
> > On Fri, Nov 09, 2012 at 04:02:14PM +0900, Atsushi Kumagai wrote:
> > > Hello,
> > > 
> > > I made the patch set based on v1.5.1-beta to calculate the size of cyclic 
> > > buffer automatically.
> > > 
> > > In v1.5.0, users have to specify the buffer size depending on system memory
> > > size with --cyclic-buffer option in order to get decent performance.
> > > 
> > > This patch set avoids the inconvenience above by choosing the lesser value
> > > of the two below as the size of cyclic buffer automatically:
> > > 
> > >   a. the size enough for storing the 1st/2nd bitmap for the whole of vmcore
> > >   b. 80% of free memory (as safety limit)
> > > 
> > > Additionally, this patch set remove --cyclic-buffer option because I think
> > > it has been already unnecessary.
> > 
> > I was wondering how about retaining --cyclic-buffer <size> option. If 
> > there are use cases where user's don't like default of 80% of free memory
> > they can override this policy by specifying --cyclic-buffer.
> > 
> > Thanks
> > Vivek
> 
> I second that, I'd like the --cyclic-buffer option to stay.  I see PATCH
> 3/3 to remove cyclic buffer option, and I also would like it left there,
> for debug of issues that may come up later.  It's great that the code
> will automatically calculate the best cyclic buffer option, which is
> what it should do for the best customer out-of-the-box dump enablement,
> but the ability to manipulate it for debug purposes, or for workarounds
> if things go wrong, is valuable.

OK, I understand that --cyclic-buffer option is still needed, so I will 
retain it by removing the [PATCH 3/3].

Additionally, I will change [PATCH 2/3] as below because I think 100 MB
as safety limit is inconvenient for debugging.


Thanks
Atsushi Kumagai


diff --git a/makedumpfile.c b/makedumpfile.c
index 9c17280..8fa718f 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -2721,17 +2721,27 @@ out:
                if (info->bufsize_cyclic == 0) {
                        if (!calculate_cyclic_buffer_size())
                                return FALSE;
-               } else
+               } else {
+                       unsigned long long free_memory;
+
+                       /*
+                        * The buffer size is specified as Kbyte with
+                        * --cyclic-buffer <size> option.
+                        */
                        info->bufsize_cyclic <<= 10;

-               /*
-                * Max buffer size is 100 MB
-                */
-               if (info->bufsize_cyclic > (100 << 20)) {
-                       MSG("Specified buffer size is too large, ");
-                       MSG("The buffer size for the cyclic mode will be truncated to 100 MB.\n");
-                       info->bufsize_cyclic = (100 << 20);
+                       /*
+                        * Truncate the buffer size to free memory size.
+                        */
+                       free_memory = get_free_memory_size();
+                       if (info->bufsize_cyclic > free_memory) {
+                               MSG("Specified buffer size is larger than free memory.");
+                               MSG("The buffer size for the cyclic mode will ");
+                               MSG("be truncated to %lld byte.\n", free_memory);
+                               info->bufsize_cyclic = free_memory;
+                       }
                }
+
                info->pfn_cyclic = info->bufsize_cyclic * BITPERBYTE;

                DEBUG_MSG("\n");

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