public inbox for kexec@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] --dump-dmesg fix for post 3.5 kernels
@ 2013-01-31 11:48 Louis Bouchard
  2013-01-31 11:48 ` Louis Bouchard
  0 siblings, 1 reply; 5+ messages in thread
From: Louis Bouchard @ 2013-01-31 11:48 UTC (permalink / raw)
  To: kexec

Hello,

This patch fixes the --dump-dmesg options that no longer works with
the variable-length record format of the kernel log buffer.

Kind regards,

...Louis

--
Louis Bouchard
Backline Support Analyst
Canonical Ltd
Ubuntu support: http://landscape.canonical.com 

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH] --dump-dmesg fix for post 3.5 kernels
  2013-01-31 11:48 [PATCH] --dump-dmesg fix for post 3.5 kernels Louis Bouchard
@ 2013-01-31 11:48 ` Louis Bouchard
  2013-02-04  8:27   ` Atsushi Kumagai
  0 siblings, 1 reply; 5+ messages in thread
From: Louis Bouchard @ 2013-01-31 11:48 UTC (permalink / raw)
  To: kexec

       This patch enable the --dump-dmesg functionality that
       was broken with post 3.5 kernel which use the variable-length
       record format for the kernel log buffer.

Signed-off-by: Louis Bouchard <louis.bouchard@canonical.com>
---
 makedumpfile.c |  242 +++++++++++++++++++++++++++++++++++++++++++++-----------
 makedumpfile.h |   16 ++++
 2 files changed, 213 insertions(+), 45 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index 715ca6e..a5180f6 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -20,6 +20,7 @@
 #include "erase_info.h"
 #include "sadump_info.h"
 #include <stddef.h>
+#include <ctype.h>
 #include <sys/time.h>
 
 struct symbol_table	symbol_table;
@@ -848,6 +849,8 @@ get_symbol_info(void)
 	SYMBOL_INIT(log_buf, "log_buf");
 	SYMBOL_INIT(log_buf_len, "log_buf_len");
 	SYMBOL_INIT(log_end, "log_end");
+	SYMBOL_INIT(log_first_idx, "log_first_idx");
+	SYMBOL_INIT(log_next_idx, "log_next_idx");
 	SYMBOL_INIT(max_pfn, "max_pfn");
 	SYMBOL_INIT(modules, "modules");
 	SYMBOL_INIT(high_memory, "high_memory");
@@ -1176,6 +1179,13 @@ get_structure_info(void)
 	OFFSET_INIT(elf64_phdr.p_paddr, "elf64_phdr", "p_paddr");
 	OFFSET_INIT(elf64_phdr.p_memsz, "elf64_phdr", "p_memsz");
 
+	SIZE_INIT(log, "log");
+	OFFSET_INIT(log.ts_nsec, "log", "ts_nsec");
+	OFFSET_INIT(log.len, "log", "len");
+	OFFSET_INIT(log.text_len, "log", "text_len");
+	OFFSET_INIT(log.dict_len, "log", "dict_len");
+	OFFSET_INIT(log.dict_len, "log", "level");
+
 	return TRUE;
 }
 
@@ -1354,6 +1364,8 @@ write_vmcoreinfo_data(void)
 	WRITE_SYMBOL("log_buf", log_buf);
 	WRITE_SYMBOL("log_buf_len", log_buf_len);
 	WRITE_SYMBOL("log_end", log_end);
+	WRITE_SYMBOL("log_first_idx", log_first_idx);
+	WRITE_SYMBOL("log_next_idx", log_next_idx);
 	WRITE_SYMBOL("max_pfn", max_pfn);
 	WRITE_SYMBOL("high_memory", high_memory);
 	WRITE_SYMBOL("node_remap_start_vaddr", node_remap_start_vaddr);
@@ -1404,6 +1416,10 @@ write_vmcoreinfo_data(void)
 	WRITE_MEMBER_OFFSET("node_memblk_s.size", node_memblk_s.size);
 	WRITE_MEMBER_OFFSET("node_memblk_s.nid", node_memblk_s.nid);
 	WRITE_MEMBER_OFFSET("vm_struct.addr", vm_struct.addr);
+	WRITE_MEMBER_OFFSET("log.ts_nsec",log.ts_nsec);
+	WRITE_MEMBER_OFFSET("log.len",log.len);
+	WRITE_MEMBER_OFFSET("log.text_len",log.text_len);
+	WRITE_MEMBER_OFFSET("log.dict_len",log.dict_len);
 
 	if (SYMBOL(node_data) != NOT_FOUND_SYMBOL)
 		WRITE_ARRAY_LENGTH("node_data", node_data);
@@ -1664,6 +1680,8 @@ read_vmcoreinfo(void)
 	READ_SYMBOL("log_buf", log_buf);
 	READ_SYMBOL("log_buf_len", log_buf_len);
 	READ_SYMBOL("log_end", log_end);
+	READ_SYMBOL("log_first_idx",log_first_idx);
+	READ_SYMBOL("log_next_idx",log_next_idx);
 	READ_SYMBOL("max_pfn", max_pfn);
 	READ_SYMBOL("high_memory", high_memory);
 	READ_SYMBOL("node_remap_start_vaddr", node_remap_start_vaddr);
@@ -1679,6 +1697,7 @@ read_vmcoreinfo(void)
 	READ_STRUCTURE_SIZE("node_memblk_s", node_memblk_s);
 	READ_STRUCTURE_SIZE("nodemask_t", nodemask_t);
 	READ_STRUCTURE_SIZE("pageflags", pageflags);
+	READ_STRUCTURE_SIZE("log", log);
 
 	READ_MEMBER_OFFSET("page.flags", page.flags);
 	READ_MEMBER_OFFSET("page._count", page._count);
@@ -1707,6 +1726,10 @@ read_vmcoreinfo(void)
 	READ_MEMBER_OFFSET("node_memblk_s.size", node_memblk_s.size);
 	READ_MEMBER_OFFSET("node_memblk_s.nid", node_memblk_s.nid);
 	READ_MEMBER_OFFSET("vm_struct.addr", vm_struct.addr);
+	READ_MEMBER_OFFSET("log.ts_nsec",log.ts_nsec);
+	READ_MEMBER_OFFSET("log.len",log.len);
+	READ_MEMBER_OFFSET("log.text_len",log.text_len);
+	READ_MEMBER_OFFSET("log.dict_len",log.dict_len);
 
 	READ_ARRAY_LENGTH("node_data", node_data);
 	READ_ARRAY_LENGTH("pgdat_list", pgdat_list);
@@ -3448,14 +3471,99 @@ reset_bitmap_of_free_pages(unsigned long node_zones)
 	return TRUE;
 }
 
+static int
+dump_log_entry(char *logptr, int fp)
+{
+	char *msg, *p;
+	unsigned int i, text_len;
+	unsigned long long ts_nsec;
+	char buf[BUFSIZE];
+	ulonglong nanos; 
+	ulong rem;
+
+	text_len = USHORT(logptr + OFFSET(log.text_len));
+	ts_nsec = ULONGLONG(logptr + OFFSET(log.ts_nsec));
+
+	nanos = (ulonglong)ts_nsec / (ulonglong)1000000000;
+	rem = (ulonglong)ts_nsec % (ulonglong)1000000000;
+
+	msg = logptr + SIZE(log);
+
+	sprintf(buf,"[%5lld.%06ld] ",nanos,rem/1000);
+
+        for (i = 0, p = msg; i < text_len; i++, p++) {
+                if (*p == '\n')
+                       sprintf(buf,"%s.",buf);
+                else if (isprint(*p) || isspace(*p))
+                       sprintf(buf,"%s%c",buf,*p);
+                else
+                       sprintf(buf,"%s.",buf);
+        }
+
+        sprintf(buf,"%s\n",buf);
+
+	if (write(info->fd_dumpfile, buf, strlen(buf)) < 0)
+		return FALSE;
+	else
+		return TRUE;
+}
+
+/* 
+ * get log record by index; idx must point to valid message.
+ */
+static char *
+log_from_idx(unsigned int idx, char *logbuf)
+{
+	char *logptr;
+	unsigned int msglen;
+
+	logptr = logbuf + idx;
+
+	/*
+	 * A length == 0 record is the end of buffer marker. 
+	 * Wrap around and return the message at the start of 
+	 * the buffer.
+	 */
+
+	msglen = USHORT(logptr + OFFSET(log.len));
+	if (!msglen)
+		logptr = logbuf;
+
+	return logptr;
+}
+
+static long 
+log_next(unsigned int idx, char *logbuf)
+{
+	char *logptr;
+	unsigned int msglen;
+
+	logptr = logbuf + idx;
+
+	/*
+	 * A length == 0 record is the end of buffer marker. Wrap around and
+	 * read the message at the start of the buffer as *this* one, and
+	 * return the one after that.
+	 */
+
+	msglen = USHORT(logptr + OFFSET(log.len));
+	if (!msglen) {
+		msglen = USHORT(logbuf + OFFSET(log.len));
+		return msglen;
+	}
+
+        return idx + msglen;
+}
+
 int
 dump_dmesg()
 {
 	int log_buf_len, length_log, length_oldlog, ret = FALSE;
-	unsigned long log_buf, log_end, index;
+	unsigned long index, log_buf, log_end;
+	unsigned int idx, log_first_idx, log_next_idx;
 	unsigned long log_end_2_6_24;
 	unsigned      log_end_2_6_25;
-	char *log_buffer = NULL;
+	char *log_buffer = NULL, *log_ptr = NULL;
 
 	/*
 	 * log_end has been changed to "unsigned" since linux-2.6.25.
@@ -3473,29 +3581,51 @@ dump_dmesg()
 		return FALSE;
 
 	if ((SYMBOL(log_buf) == NOT_FOUND_SYMBOL)
-	    || (SYMBOL(log_buf_len) == NOT_FOUND_SYMBOL)
-	    || (SYMBOL(log_end) == NOT_FOUND_SYMBOL)) {
+	    || (SYMBOL(log_buf_len) == NOT_FOUND_SYMBOL)) {
 		ERRMSG("Can't find some symbols for log_buf.\n");
 		return FALSE;
 	}
+	/*
+	* kernel 3.5 variable-length record buffer structure
+	*/
+	if (SYMBOL(log_end) == NOT_FOUND_SYMBOL) {
+		if ((SYMBOL(log_first_idx) == NOT_FOUND_SYMBOL)
+		   || (SYMBOL(log_next_idx) == NOT_FOUND_SYMBOL)){
+		   ERRMSG("Can't find variable-length record symbols");
+		   return FALSE;
+		} else {
+			if (!readmem(VADDR, SYMBOL(log_first_idx), &log_first_idx,
+	    		sizeof(log_first_idx))) {
+				ERRMSG("Can't get log_first_idx.\n");
+				return FALSE;
+				}
+			if (!readmem(VADDR, SYMBOL(log_next_idx), &log_next_idx,
+	    		sizeof(log_next_idx))) {
+				ERRMSG("Can't get log_next_idx.\n");
+				return FALSE;
+				}
+			}
+		}
 	if (!readmem(VADDR, SYMBOL(log_buf), &log_buf, sizeof(log_buf))) {
 		ERRMSG("Can't get log_buf.\n");
 		return FALSE;
 	}
-	if (info->kernel_version >= KERNEL_VERSION(2, 6, 25)) {
-		if (!readmem(VADDR, SYMBOL(log_end), &log_end_2_6_25,
-		    sizeof(log_end_2_6_25))) {
-			ERRMSG("Can't to get log_end.\n");
-			return FALSE;
-		}
-		log_end = log_end_2_6_25;
-	} else {
-		if (!readmem(VADDR, SYMBOL(log_end), &log_end_2_6_24,
-		    sizeof(log_end_2_6_24))) {
-			ERRMSG("Can't to get log_end.\n");
-			return FALSE;
+	if (info->kernel_version < KERNEL_VERSION(3, 5, 0)) {
+		if (info->kernel_version >= KERNEL_VERSION(2, 6, 25)) {
+			if (!readmem(VADDR, SYMBOL(log_end), &log_end_2_6_25,
+			    sizeof(log_end_2_6_25))) {
+				ERRMSG("Can't to get log_end.\n");
+				return FALSE;
+			}
+			log_end = log_end_2_6_25;
+		} else {
+			if (!readmem(VADDR, SYMBOL(log_end), &log_end_2_6_24,
+			    sizeof(log_end_2_6_24))) {
+				ERRMSG("Can't to get log_end.\n");
+				return FALSE;
+			}
+			log_end = log_end_2_6_24;
 		}
-		log_end = log_end_2_6_24;
 	}
 	if (!readmem(VADDR, SYMBOL(log_buf_len), &log_buf_len,
 	    sizeof(log_buf_len))) {
@@ -3503,47 +3633,69 @@ dump_dmesg()
 		return FALSE;
 	}
 	DEBUG_MSG("\n");
-	DEBUG_MSG("log_buf      : %lx\n", log_buf);
-	DEBUG_MSG("log_end      : %lx\n", log_end);
-	DEBUG_MSG("log_buf_len  : %d\n", log_buf_len);
+	DEBUG_MSG("log_buf       : %lx\n", log_buf);
+	DEBUG_MSG("log_end       : %lx\n", log_end);
+	DEBUG_MSG("log_buf_len   : %d\n", log_buf_len);
+	DEBUG_MSG("log_first_idx : %u\n", log_first_idx);
+	DEBUG_MSG("log_next_idx  : %u\n", log_next_idx);
 
 	if ((log_buffer = malloc(log_buf_len)) == NULL) {
 		ERRMSG("Can't allocate memory for log_buf. %s\n",
-		    strerror(errno));
+			   strerror(errno));
 		return FALSE;
 	}
-
-	if (log_end < log_buf_len) {
-		length_log = log_end;
-		if(!readmem(VADDR, log_buf, log_buffer, length_log)) {
-			ERRMSG("Can't read dmesg log.\n");
+	
+	if (info->kernel_version < KERNEL_VERSION(3, 5,0)) {
+		if (log_end < log_buf_len) {
+			length_log = log_end;
+			if(!readmem(VADDR, log_buf, log_buffer, length_log)) {
+				ERRMSG("Can't read dmesg log.\n");
+				goto out;
+			}
+		} else {
+			index = log_end & (log_buf_len - 1);
+			DEBUG_MSG("index        : %lx\n", index);
+			length_log = log_buf_len;
+			length_oldlog = log_buf_len - index;
+			if(!readmem(VADDR, log_buf + index, log_buffer, length_oldlog)) {
+				ERRMSG("Can't read old dmesg log.\n");
+				goto out;
+			}
+			if(!readmem(VADDR, log_buf, log_buffer + length_oldlog, index)) {
+				ERRMSG("Can't read new dmesg log.\n");
+				goto out;
+			}
+		}
+		DEBUG_MSG("length_log   : %d\n", length_log);
+	
+		if (!open_dump_file()) {
+			ERRMSG("Can't open output file.\n");
 			goto out;
 		}
+		if (write(info->fd_dumpfile, log_buffer, length_log) < 0)
+			goto out;
+	
+		if (!close_files_for_creating_dumpfile())
+			goto out;
 	} else {
-		index = log_end & (log_buf_len - 1);
-		DEBUG_MSG("index        : %lx\n", index);
-		length_log = log_buf_len;
-		length_oldlog = log_buf_len - index;
-		if(!readmem(VADDR, log_buf + index, log_buffer, length_oldlog)) {
-			ERRMSG("Can't read old dmesg log.\n");
+		if(!readmem(VADDR, log_buf, log_buffer, log_buf_len)) {
+			ERRMSG("Can't read indexed dmesg log.\n");
 			goto out;
 		}
-		if(!readmem(VADDR, log_buf, log_buffer + length_oldlog, index)) {
-			ERRMSG("Can't read new dmesg log.\n");
+		if (!open_dump_file()) {
+			ERRMSG("Can't open output file.\n");
 			goto out;
 		}
+		idx = log_first_idx;
+		while (idx != log_next_idx ) {
+			log_ptr = log_from_idx(idx, log_buffer);
+			if (!dump_log_entry(log_ptr,info->fd_dumpfile))
+				goto out;
+			idx = log_next(idx, log_buffer);
+		}
+		if (!close_files_for_creating_dumpfile())
+			goto out;
 	}
-	DEBUG_MSG("length_log   : %d\n", length_log);
-
-	if (!open_dump_file()) {
-		ERRMSG("Can't open output file.\n");
-		goto out;
-	}
-	if (write(info->fd_dumpfile, log_buffer, length_log) < 0)
-		goto out;
-
-	if (!close_files_for_creating_dumpfile())
-		goto out;
 
 	ret = TRUE;
 out:
diff --git a/makedumpfile.h b/makedumpfile.h
index 4c4d201..b36c529 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -206,6 +206,7 @@ isAnon(unsigned long mapping)
 #define FILENAME_BITMAP		"kdump_bitmapXXXXXX"
 #define FILENAME_STDOUT		"STDOUT"
 
+
 /*
  * Minimam vmcore has 2 ProgramHeaderTables(PT_NOTE and PT_LOAD).
  */
@@ -223,6 +224,8 @@ static inline int string_exists(char *s) { return (s ? TRUE : FALSE); }
 #define USHORT(ADDR)	*((unsigned short *)(ADDR))
 #define UINT(ADDR)	*((unsigned int *)(ADDR))
 #define ULONG(ADDR)	*((unsigned long *)(ADDR))
+#define ULONGLONG(ADDR)	*((unsigned long long *)(ADDR))
+
 
 /*
  * for symbol
@@ -819,6 +822,8 @@ struct cache_data {
 	size_t	cache_size;
 	off_t	offset;
 };
+typedef unsigned long int ulong;
+typedef unsigned long long int ulonglong;
 
 /*
  * makedumpfile header
@@ -1094,6 +1099,8 @@ struct symbol_table {
 	unsigned long long	log_buf;
 	unsigned long long	log_buf_len;
 	unsigned long long	log_end;
+	unsigned long long	log_first_idx;
+	unsigned long long	log_next_idx;
 	unsigned long long	max_pfn;
 	unsigned long long	node_remap_start_vaddr;
 	unsigned long long	node_remap_end_vaddr;
@@ -1173,6 +1180,7 @@ struct size_table {
 	long	cpumask_t;
 	long	kexec_segment;
 	long	elf64_hdr;
+	long	log;
 
 	long	pageflags;
 };
@@ -1306,6 +1314,14 @@ struct offset_table {
 		long	p_paddr;
 		long	p_memsz;
 	} elf64_phdr;
+
+	struct log_s {
+        	long ts_nsec;
+        	long len;
+        	long text_len;
+        	long dict_len;
+	} log;
+
 };
 
 /*
-- 
1.7.10.4


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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] --dump-dmesg fix for post 3.5 kernels
  2013-01-31 11:48 ` Louis Bouchard
@ 2013-02-04  8:27   ` Atsushi Kumagai
  2013-02-04  8:46     ` Bouchard Louis
  0 siblings, 1 reply; 5+ messages in thread
From: Atsushi Kumagai @ 2013-02-04  8:27 UTC (permalink / raw)
  To: louis.bouchard; +Cc: kexec

Hello Louis,

On Thu, 31 Jan 2013 12:48:08 +0100
Louis Bouchard <louis.bouchard@canonical.com> wrote:

>        This patch enable the --dump-dmesg functionality that
>        was broken with post 3.5 kernel which use the variable-length
>        record format for the kernel log buffer.

I appreciate your hard work.
Your patch seems to work correctly, but I have some comments.
(You don't need to resend the patch.)

> 
> Signed-off-by: Louis Bouchard <louis.bouchard@canonical.com>
> ---
>  makedumpfile.c |  242 +++++++++++++++++++++++++++++++++++++++++++++-----------
>  makedumpfile.h |   16 ++++
>  2 files changed, 213 insertions(+), 45 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 715ca6e..a5180f6 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -20,6 +20,7 @@
>  #include "erase_info.h"
>  #include "sadump_info.h"
>  #include <stddef.h>
> +#include <ctype.h>
>  #include <sys/time.h>
>  
>  struct symbol_table	symbol_table;
> @@ -848,6 +849,8 @@ get_symbol_info(void)
>  	SYMBOL_INIT(log_buf, "log_buf");
>  	SYMBOL_INIT(log_buf_len, "log_buf_len");
>  	SYMBOL_INIT(log_end, "log_end");
> +	SYMBOL_INIT(log_first_idx, "log_first_idx");
> +	SYMBOL_INIT(log_next_idx, "log_next_idx");
>  	SYMBOL_INIT(max_pfn, "max_pfn");
>  	SYMBOL_INIT(modules, "modules");
>  	SYMBOL_INIT(high_memory, "high_memory");
> @@ -1176,6 +1179,13 @@ get_structure_info(void)
>  	OFFSET_INIT(elf64_phdr.p_paddr, "elf64_phdr", "p_paddr");
>  	OFFSET_INIT(elf64_phdr.p_memsz, "elf64_phdr", "p_memsz");
>  
> +	SIZE_INIT(log, "log");
> +	OFFSET_INIT(log.ts_nsec, "log", "ts_nsec");
> +	OFFSET_INIT(log.len, "log", "len");
> +	OFFSET_INIT(log.text_len, "log", "text_len");
> +	OFFSET_INIT(log.dict_len, "log", "dict_len");
> +	OFFSET_INIT(log.dict_len, "log", "level");
> +

Why you overwrite log.dict_len with log.level ?
And is log.dict_len necessary for this feature ?

>  	return TRUE;
>  }
>  
> @@ -1354,6 +1364,8 @@ write_vmcoreinfo_data(void)
>  	WRITE_SYMBOL("log_buf", log_buf);
>  	WRITE_SYMBOL("log_buf_len", log_buf_len);
>  	WRITE_SYMBOL("log_end", log_end);
> +	WRITE_SYMBOL("log_first_idx", log_first_idx);
> +	WRITE_SYMBOL("log_next_idx", log_next_idx);
>  	WRITE_SYMBOL("max_pfn", max_pfn);
>  	WRITE_SYMBOL("high_memory", high_memory);
>  	WRITE_SYMBOL("node_remap_start_vaddr", node_remap_start_vaddr);
> @@ -1404,6 +1416,10 @@ write_vmcoreinfo_data(void)
>  	WRITE_MEMBER_OFFSET("node_memblk_s.size", node_memblk_s.size);
>  	WRITE_MEMBER_OFFSET("node_memblk_s.nid", node_memblk_s.nid);
>  	WRITE_MEMBER_OFFSET("vm_struct.addr", vm_struct.addr);
> +	WRITE_MEMBER_OFFSET("log.ts_nsec",log.ts_nsec);
> +	WRITE_MEMBER_OFFSET("log.len",log.len);
> +	WRITE_MEMBER_OFFSET("log.text_len",log.text_len);
> +	WRITE_MEMBER_OFFSET("log.dict_len",log.dict_len);
>  
>  	if (SYMBOL(node_data) != NOT_FOUND_SYMBOL)
>  		WRITE_ARRAY_LENGTH("node_data", node_data);
> @@ -1664,6 +1680,8 @@ read_vmcoreinfo(void)
>  	READ_SYMBOL("log_buf", log_buf);
>  	READ_SYMBOL("log_buf_len", log_buf_len);
>  	READ_SYMBOL("log_end", log_end);
> +	READ_SYMBOL("log_first_idx",log_first_idx);
> +	READ_SYMBOL("log_next_idx",log_next_idx);
>  	READ_SYMBOL("max_pfn", max_pfn);
>  	READ_SYMBOL("high_memory", high_memory);
>  	READ_SYMBOL("node_remap_start_vaddr", node_remap_start_vaddr);
> @@ -1679,6 +1697,7 @@ read_vmcoreinfo(void)
>  	READ_STRUCTURE_SIZE("node_memblk_s", node_memblk_s);
>  	READ_STRUCTURE_SIZE("nodemask_t", nodemask_t);
>  	READ_STRUCTURE_SIZE("pageflags", pageflags);
> +	READ_STRUCTURE_SIZE("log", log);

You forgot to add "WRITE_STRUCTURE_SIZE("log", log)", I'll add it.

Lastly, your patch causes the following warning:

  makedumpfile.c: In function 'dump_dmesg':
  makedumpfile.c:3562: warning: 'log_end' may be used uninitialized in this function

So, I'll fix this patch with the change below:


diff --git a/makedumpfile.c b/makedumpfile.c
index 2a2fcbd..ebedd07 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -1384,6 +1384,7 @@ write_vmcoreinfo_data(void)
	WRITE_STRUCTURE_SIZE("node_memblk_s", node_memblk_s);
	WRITE_STRUCTURE_SIZE("nodemask_t", nodemask_t);
	WRITE_STRUCTURE_SIZE("pageflags", pageflags);
+	WRITE_STRUCTURE_SIZE("log", log);

	/*
	 * write the member offset of 1st kernel
@@ -3626,7 +3627,9 @@ dump_dmesg()
			}
			log_end = log_end_2_6_24;
		}
-	}
+	} else
+		log_end = 0;
+
	if (!readmem(VADDR, SYMBOL(log_buf_len), &log_buf_len,
	    sizeof(log_buf_len))) {
		ERRMSG("Can't get log_buf_len.\n");


Thanks
Atsushi Kumagai

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] --dump-dmesg fix for post 3.5 kernels
  2013-02-04  8:27   ` Atsushi Kumagai
@ 2013-02-04  8:46     ` Bouchard Louis
  2013-02-05  1:24       ` Atsushi Kumagai
  0 siblings, 1 reply; 5+ messages in thread
From: Bouchard Louis @ 2013-02-04  8:46 UTC (permalink / raw)
  To: Atsushi Kumagai; +Cc: kexec

Hello,

Le 04/02/2013 09:27, Atsushi Kumagai a écrit :
> Hello Louis,
> 
> On Thu, 31 Jan 2013 12:48:08 +0100
> Louis Bouchard <louis.bouchard@canonical.com> wrote:
> 
>>        This patch enable the --dump-dmesg functionality that
>>        was broken with post 3.5 kernel which use the variable-length
>>        record format for the kernel log buffer.
> 
> I appreciate your hard work.
> Your patch seems to work correctly, but I have some comments.
> (You don't need to resend the patch.)
> 

Thanks. It was a very useful learning experience, especially for my
debian packaging tasks.

>>
>> Signed-off-by: Louis Bouchard <louis.bouchard@canonical.com>
>> ---
>>  makedumpfile.c |  242 +++++++++++++++++++++++++++++++++++++++++++++-----------
>>  makedumpfile.h |   16 ++++
>>  2 files changed, 213 insertions(+), 45 deletions(-)
>>
>> diff --git a/makedumpfile.c b/makedumpfile.c
>> index 715ca6e..a5180f6 100644
>> --- a/makedumpfile.c
>> +++ b/makedumpfile.c
>> @@ -20,6 +20,7 @@
>>  #include "erase_info.h"
>>  #include "sadump_info.h"
>>  #include <stddef.h>
>> +#include <ctype.h>
>>  #include <sys/time.h>
>>  
>>  struct symbol_table	symbol_table;
>> @@ -848,6 +849,8 @@ get_symbol_info(void)
>>  	SYMBOL_INIT(log_buf, "log_buf");
>>  	SYMBOL_INIT(log_buf_len, "log_buf_len");
>>  	SYMBOL_INIT(log_end, "log_end");
>> +	SYMBOL_INIT(log_first_idx, "log_first_idx");
>> +	SYMBOL_INIT(log_next_idx, "log_next_idx");
>>  	SYMBOL_INIT(max_pfn, "max_pfn");
>>  	SYMBOL_INIT(modules, "modules");
>>  	SYMBOL_INIT(high_memory, "high_memory");
>> @@ -1176,6 +1179,13 @@ get_structure_info(void)
>>  	OFFSET_INIT(elf64_phdr.p_paddr, "elf64_phdr", "p_paddr");
>>  	OFFSET_INIT(elf64_phdr.p_memsz, "elf64_phdr", "p_memsz");
>>  
>> +	SIZE_INIT(log, "log");
>> +	OFFSET_INIT(log.ts_nsec, "log", "ts_nsec");
>> +	OFFSET_INIT(log.len, "log", "len");
>> +	OFFSET_INIT(log.text_len, "log", "text_len");
>> +	OFFSET_INIT(log.dict_len, "log", "dict_len");
>> +	OFFSET_INIT(log.dict_len, "log", "level");
>> +
> 
> Why you overwrite log.dict_len with log.level ?
> And is log.dict_len necessary for this feature ?
> 

Sorry, this is a leftover from my log-level hack. The rest of the code
has been removed, so this should be removed as well.

>>  	return TRUE;
>>  }
>>  
>> @@ -1354,6 +1364,8 @@ write_vmcoreinfo_data(void)
>>  	WRITE_SYMBOL("log_buf", log_buf);
>>  	WRITE_SYMBOL("log_buf_len", log_buf_len);
>>  	WRITE_SYMBOL("log_end", log_end);
>> +	WRITE_SYMBOL("log_first_idx", log_first_idx);
>> +	WRITE_SYMBOL("log_next_idx", log_next_idx);
>>  	WRITE_SYMBOL("max_pfn", max_pfn);
>>  	WRITE_SYMBOL("high_memory", high_memory);
>>  	WRITE_SYMBOL("node_remap_start_vaddr", node_remap_start_vaddr);
>> @@ -1404,6 +1416,10 @@ write_vmcoreinfo_data(void)
>>  	WRITE_MEMBER_OFFSET("node_memblk_s.size", node_memblk_s.size);
>>  	WRITE_MEMBER_OFFSET("node_memblk_s.nid", node_memblk_s.nid);
>>  	WRITE_MEMBER_OFFSET("vm_struct.addr", vm_struct.addr);
>> +	WRITE_MEMBER_OFFSET("log.ts_nsec",log.ts_nsec);
>> +	WRITE_MEMBER_OFFSET("log.len",log.len);
>> +	WRITE_MEMBER_OFFSET("log.text_len",log.text_len);
>> +	WRITE_MEMBER_OFFSET("log.dict_len",log.dict_len);
>>  
>>  	if (SYMBOL(node_data) != NOT_FOUND_SYMBOL)
>>  		WRITE_ARRAY_LENGTH("node_data", node_data);
>> @@ -1664,6 +1680,8 @@ read_vmcoreinfo(void)
>>  	READ_SYMBOL("log_buf", log_buf);
>>  	READ_SYMBOL("log_buf_len", log_buf_len);
>>  	READ_SYMBOL("log_end", log_end);
>> +	READ_SYMBOL("log_first_idx",log_first_idx);
>> +	READ_SYMBOL("log_next_idx",log_next_idx);
>>  	READ_SYMBOL("max_pfn", max_pfn);
>>  	READ_SYMBOL("high_memory", high_memory);
>>  	READ_SYMBOL("node_remap_start_vaddr", node_remap_start_vaddr);
>> @@ -1679,6 +1697,7 @@ read_vmcoreinfo(void)
>>  	READ_STRUCTURE_SIZE("node_memblk_s", node_memblk_s);
>>  	READ_STRUCTURE_SIZE("nodemask_t", nodemask_t);
>>  	READ_STRUCTURE_SIZE("pageflags", pageflags);
>> +	READ_STRUCTURE_SIZE("log", log);
> 
> You forgot to add "WRITE_STRUCTURE_SIZE("log", log)", I'll add it.
> 
> Lastly, your patch causes the following warning:
> 
>   makedumpfile.c: In function 'dump_dmesg':
>   makedumpfile.c:3562: warning: 'log_end' may be used uninitialized in this function
> 
> So, I'll fix this patch with the change below:
> 
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 2a2fcbd..ebedd07 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -1384,6 +1384,7 @@ write_vmcoreinfo_data(void)
> 	WRITE_STRUCTURE_SIZE("node_memblk_s", node_memblk_s);
> 	WRITE_STRUCTURE_SIZE("nodemask_t", nodemask_t);
> 	WRITE_STRUCTURE_SIZE("pageflags", pageflags);
> +	WRITE_STRUCTURE_SIZE("log", log);
> 
> 	/*
> 	 * write the member offset of 1st kernel
> @@ -3626,7 +3627,9 @@ dump_dmesg()
> 			}
> 			log_end = log_end_2_6_24;
> 		}
> -	}
> +	} else
> +		log_end = 0;
> +

Oups, I missed this one, sorry.

> 	if (!readmem(VADDR, SYMBOL(log_buf_len), &log_buf_len,
> 	    sizeof(log_buf_len))) {
> 		ERRMSG("Can't get log_buf_len.\n");
> 
> 
> Thanks
> Atsushi Kumagai
> 

This all looks fine. I am planning to add this patch over your 1.5.1
version for Debian/Sid so we get this working (I need it myself for the
upcoming Ubuntu/Raring) if you don't see any problem. The debian patch
will be removed in the next version when it makes it into your upstream
release.

I'll go pick up your patch in your repo as soon as it makes it in. Just
let me know when you commit if you have a chance.

Kind regards,

...Louis
-- 
Louis Bouchard
Backline Support Analyst
Canonical Ltd
Ubuntu support: http://landscape.canonical.com

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] --dump-dmesg fix for post 3.5 kernels
  2013-02-04  8:46     ` Bouchard Louis
@ 2013-02-05  1:24       ` Atsushi Kumagai
  0 siblings, 0 replies; 5+ messages in thread
From: Atsushi Kumagai @ 2013-02-05  1:24 UTC (permalink / raw)
  To: louis.bouchard; +Cc: kexec

Hello Louis,

On Mon, 04 Feb 2013 09:46:39 +0100
Bouchard Louis <louis.bouchard@canonical.com> wrote:

[...]
> >>  
> >> +	SIZE_INIT(log, "log");
> >> +	OFFSET_INIT(log.ts_nsec, "log", "ts_nsec");
> >> +	OFFSET_INIT(log.len, "log", "len");
> >> +	OFFSET_INIT(log.text_len, "log", "text_len");
> >> +	OFFSET_INIT(log.dict_len, "log", "dict_len");
> >> +	OFFSET_INIT(log.dict_len, "log", "level");
> >> +
> > 
> > Why you overwrite log.dict_len with log.level ?
> > And is log.dict_len necessary for this feature ?
> > 
> 
> Sorry, this is a leftover from my log-level hack. The rest of the code
> has been removed, so this should be removed as well.

OK, I removed them.

> 
> >>  	return TRUE;
> >>  }
> >>  
> >> @@ -1354,6 +1364,8 @@ write_vmcoreinfo_data(void)
> >>  	WRITE_SYMBOL("log_buf", log_buf);
> >>  	WRITE_SYMBOL("log_buf_len", log_buf_len);
> >>  	WRITE_SYMBOL("log_end", log_end);
> >> +	WRITE_SYMBOL("log_first_idx", log_first_idx);
> >> +	WRITE_SYMBOL("log_next_idx", log_next_idx);
> >>  	WRITE_SYMBOL("max_pfn", max_pfn);
> >>  	WRITE_SYMBOL("high_memory", high_memory);
> >>  	WRITE_SYMBOL("node_remap_start_vaddr", node_remap_start_vaddr);
> >> @@ -1404,6 +1416,10 @@ write_vmcoreinfo_data(void)
> >>  	WRITE_MEMBER_OFFSET("node_memblk_s.size", node_memblk_s.size);
> >>  	WRITE_MEMBER_OFFSET("node_memblk_s.nid", node_memblk_s.nid);
> >>  	WRITE_MEMBER_OFFSET("vm_struct.addr", vm_struct.addr);
> >> +	WRITE_MEMBER_OFFSET("log.ts_nsec",log.ts_nsec);
> >> +	WRITE_MEMBER_OFFSET("log.len",log.len);
> >> +	WRITE_MEMBER_OFFSET("log.text_len",log.text_len);
> >> +	WRITE_MEMBER_OFFSET("log.dict_len",log.dict_len);
> >>  
> >>  	if (SYMBOL(node_data) != NOT_FOUND_SYMBOL)
> >>  		WRITE_ARRAY_LENGTH("node_data", node_data);
> >> @@ -1664,6 +1680,8 @@ read_vmcoreinfo(void)
> >>  	READ_SYMBOL("log_buf", log_buf);
> >>  	READ_SYMBOL("log_buf_len", log_buf_len);
> >>  	READ_SYMBOL("log_end", log_end);
> >> +	READ_SYMBOL("log_first_idx",log_first_idx);
> >> +	READ_SYMBOL("log_next_idx",log_next_idx);
> >>  	READ_SYMBOL("max_pfn", max_pfn);
> >>  	READ_SYMBOL("high_memory", high_memory);
> >>  	READ_SYMBOL("node_remap_start_vaddr", node_remap_start_vaddr);
> >> @@ -1679,6 +1697,7 @@ read_vmcoreinfo(void)
> >>  	READ_STRUCTURE_SIZE("node_memblk_s", node_memblk_s);
> >>  	READ_STRUCTURE_SIZE("nodemask_t", nodemask_t);
> >>  	READ_STRUCTURE_SIZE("pageflags", pageflags);
> >> +	READ_STRUCTURE_SIZE("log", log);
> > 
> > You forgot to add "WRITE_STRUCTURE_SIZE("log", log)", I'll add it.
> > 
> > Lastly, your patch causes the following warning:
> > 
> >   makedumpfile.c: In function 'dump_dmesg':
> >   makedumpfile.c:3562: warning: 'log_end' may be used uninitialized in this function
> > 
> > So, I'll fix this patch with the change below:
> > 
> > 
> > diff --git a/makedumpfile.c b/makedumpfile.c
> > index 2a2fcbd..ebedd07 100644
> > --- a/makedumpfile.c
> > +++ b/makedumpfile.c
> > @@ -1384,6 +1384,7 @@ write_vmcoreinfo_data(void)
> > 	WRITE_STRUCTURE_SIZE("node_memblk_s", node_memblk_s);
> > 	WRITE_STRUCTURE_SIZE("nodemask_t", nodemask_t);
> > 	WRITE_STRUCTURE_SIZE("pageflags", pageflags);
> > +	WRITE_STRUCTURE_SIZE("log", log);
> > 
> > 	/*
> > 	 * write the member offset of 1st kernel
> > @@ -3626,7 +3627,9 @@ dump_dmesg()
> > 			}
> > 			log_end = log_end_2_6_24;
> > 		}
> > -	}
> > +	} else
> > +		log_end = 0;
> > +
> 
> Oups, I missed this one, sorry.
> 
> > 	if (!readmem(VADDR, SYMBOL(log_buf_len), &log_buf_len,
> > 	    sizeof(log_buf_len))) {
> > 		ERRMSG("Can't get log_buf_len.\n");
> > 
> > 
> > Thanks
> > Atsushi Kumagai
> > 
> 
> This all looks fine. I am planning to add this patch over your 1.5.1
> version for Debian/Sid so we get this working (I need it myself for the
> upcoming Ubuntu/Raring) if you don't see any problem. The debian patch
> will be removed in the next version when it makes it into your upstream
> release.
> 
> I'll go pick up your patch in your repo as soon as it makes it in. Just
> let me know when you commit if you have a chance.

I fixed the patch, please pick up it from the devel branch:

http://makedumpfile.git.sourceforge.net/git/gitweb.cgi?p=makedumpfile/makedumpfile;a=shortlog;h=refs/heads/devel

I haven't introduced it until now, this is a branch for development:

 - There are the patches reviewed by me, so I hope that this branch
   hasn't many bugs. 

 - This branch will be released as a next official version if
   there is no problem.

 - But, this branch could be broken without a announcement,
   don't trust too much.

At least, the dmesg patch will be merged into the next version without
more changes, I'm sure that this branch meet your purpose.


Thanks
Atsushi Kumagai

> Kind regards,
> 
> ...Louis
> -- 
> Louis Bouchard
> Backline Support Analyst
> Canonical Ltd
> Ubuntu support: http://landscape.canonical.com

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-02-05  1:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-31 11:48 [PATCH] --dump-dmesg fix for post 3.5 kernels Louis Bouchard
2013-01-31 11:48 ` Louis Bouchard
2013-02-04  8:27   ` Atsushi Kumagai
2013-02-04  8:46     ` Bouchard Louis
2013-02-05  1:24       ` Atsushi Kumagai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox