[PATCH 0/4] [-mm patch] Cleanup add-vmcoreinfo.patch v2

[PATCH 0/4] [-mm patch] Cleanup add-vmcoreinfo.patch v2

[Ken'ichi Ohmichi]

Hi Andrew,

I updated the cleanup patchset for add-vmcoreinfo.patch.
* Cleanup add-vmcoreinfo.patch v1:
http://www.ussg.iu.edu/hypermail/linux/kernel/0709.1/0318.html

This patchset (v2) includes a new patch[4/4] for adding a prefix
"VMCOREINFO_" to the vmcoreinfo macros. Other patches ([1/4]-[3/4])
don't have any changes.

Changelog:
  - Add "Add a prefix VMCOREINFO_ to the vmcoreinfo macros" patch.

Patchset:
[1/4] Cleanup the coding style according to Andrew's comments:
  http://lists.infradead.org/pipermail/kexec/2007-August/000522.html
  - vmcoreinfo_append_str() should have suitable __attribute__s so that
    the compiler can check its use.
  - vmcoreinfo_max_size should have size_t.
  - Use get_seconds() instead of xtime.tv_sec.
  - Use init_uts_ns.name.release instead of UTS_RELEASE.

[2/4] Add nodemask_t's size and NR_FREE_PAGES's value to vmcoreinfo_data.
  The dump filetering command 'makedumpfile'(v1.1.6 or before) had assumed
  the above values, and it was not good from the reliability viewpoint.
  So makedumpfile v1.2.0 came to need these values and I created the patch
  to let the kernel output them.
  makedumpfile site:
  https://sourceforge.net/projects/makedumpfile/

[3/4] Use the existing ia64_tpa() instead of asm code.

[4/4] Add a prefix "VMCOREINFO_" to the vmcoreinfo macros.
  Old vmcoreinfo macros were defined as generic names SYMBOL/SIZE/OFFSET
  /LENGTH/CONFIG, and it is impossible to grep for them. So these names
  should be changed. This discussion is the following:
  http://www.ussg.iu.edu/hypermail/linux/kernel/0709.1/0415.html


Thanks
Ken'ichi Ohmichi

[PATCH 1/4] [-mm patch] Cleanup the coding style according to Andrew's comments

[Ken'ichi Ohmichi]

[1/4] Cleanup the coding style according to Andrew's comments:
http://lists.infradead.org/pipermail/kexec/2007-August/000522.html
- vmcoreinfo_append_str() should have suitable __attribute__s so that
   the compiler can check its use.
- vmcoreinfo_max_size should have size_t.
- Use get_seconds() instead of xtime.tv_sec.
- Use init_uts_ns.name.release instead of UTS_RELEASE.


Thanks
Ken'ichi Ohmichi

---
Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

---
diff -rpuN a/include/linux/kexec.h b/include/linux/kexec.h
--- a/include/linux/kexec.h	2007-09-10 19:37:59.000000000 +0900
+++ b/include/linux/kexec.h	2007-09-10 23:09:37.000000000 +0900
@@ -123,18 +123,20 @@ int kexec_should_crash(struct task_struc
  void crash_save_cpu(struct pt_regs *regs, int cpu);
  void crash_save_vmcoreinfo(void);
  void arch_crash_save_vmcoreinfo(void);
-void vmcoreinfo_append_str(const char *fmt, ...);
+void vmcoreinfo_append_str(const char *fmt, ...)
+	__attribute__ ((format (printf, 1, 2)));
  unsigned long paddr_vmcoreinfo_note(void);

  #define SYMBOL(name) \
  	vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)&name)
  #define SIZE(name) \
-	vmcoreinfo_append_str("SIZE(%s)=%d\n", #name, sizeof(struct name))
+	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
+			      (unsigned long)sizeof(struct name))
  #define OFFSET(name, field) \
-	vmcoreinfo_append_str("OFFSET(%s.%s)=%d\n", #name, #field, \
-			      &(((struct name *)0)->field))
+	vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
+			      (unsigned long)&(((struct name *)0)->field))
  #define LENGTH(name, value) \
-	vmcoreinfo_append_str("LENGTH(%s)=%d\n", #name, value)
+	vmcoreinfo_append_str("LENGTH(%s)=%lu\n", #name, (unsigned long)value)
  #define CONFIG(name) \
  	vmcoreinfo_append_str("CONFIG_%s=y\n", #name)

@@ -177,8 +179,8 @@ extern struct resource crashk_res;
  typedef u32 note_buf_t[KEXEC_NOTE_BYTES/4];
  extern note_buf_t *crash_notes;
  extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
-extern unsigned int vmcoreinfo_size;
-extern unsigned int vmcoreinfo_max_size;
+extern size_t vmcoreinfo_size;
+extern size_t vmcoreinfo_max_size;


  #else /* !CONFIG_KEXEC */
diff -rpuN a/kernel/kexec.c b/kernel/kexec.c
--- a/kernel/kexec.c	2007-09-10 19:38:00.000000000 +0900
+++ b/kernel/kexec.c	2007-09-10 23:09:58.000000000 +0900
@@ -38,8 +38,8 @@ note_buf_t* crash_notes;
  /* vmcoreinfo stuff */
  unsigned char vmcoreinfo_data[VMCOREINFO_BYTES];
  u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
-unsigned int vmcoreinfo_size = 0;
-unsigned int vmcoreinfo_max_size = sizeof(vmcoreinfo_data);
+size_t vmcoreinfo_size;
+size_t vmcoreinfo_max_size = sizeof(vmcoreinfo_data);

  /* Location of the reserved area for the crash kernel */
  struct resource crashk_res = {
@@ -1153,7 +1153,7 @@ void crash_save_vmcoreinfo(void)
  	if (!vmcoreinfo_size)
  		return;

-	vmcoreinfo_append_str("CRASHTIME=%d", xtime.tv_sec);
+	vmcoreinfo_append_str("CRASHTIME=%ld", get_seconds());

  	buf = (u32 *)vmcoreinfo_note;

@@ -1195,8 +1195,8 @@ unsigned long __attribute__ ((weak)) pad

  static int __init crash_save_vmcoreinfo_init(void)
  {
-	vmcoreinfo_append_str("OSRELEASE=%s\n", UTS_RELEASE);
-	vmcoreinfo_append_str("PAGESIZE=%d\n", PAGE_SIZE);
+	vmcoreinfo_append_str("OSRELEASE=%s\n", init_uts_ns.name.release);
+	vmcoreinfo_append_str("PAGESIZE=%ld\n", PAGE_SIZE);

  	SYMBOL(init_uts_ns);
  	SYMBOL(node_online_map);
diff -rpuN a/kernel/ksysfs.c b/kernel/ksysfs.c
--- a/kernel/ksysfs.c	2007-09-10 19:38:00.000000000 +0900
+++ b/kernel/ksysfs.c	2007-09-10 23:09:07.000000000 +0900
@@ -65,7 +65,7 @@ static ssize_t vmcoreinfo_show(struct ks
  {
  	return sprintf(page, "%lx %x\n",
  		       paddr_vmcoreinfo_note(),
-		       vmcoreinfo_max_size);
+		       (unsigned int)vmcoreinfo_max_size);
  }
  KERNEL_ATTR_RO(vmcoreinfo);

_

[PATCH 2/4] [-mm patch] Add nodemask_t's size and NR_FREE_PAGES's value to vmcoreinfo_data.

[Ken'ichi Ohmichi]

[2/4] Add nodemask_t's size and NR_FREE_PAGES's value to vmcoreinfo_data.
   The dump filetering command 'makedumpfile'(v1.1.6 or before) had assumed
   the above values, and it was not good from the reliability viewpoint.
   So makedumpfile v1.2.0 came to need these values and I created the patch
   to let the kernel output them.
   makedumpfile site:
   https://sourceforge.net/projects/makedumpfile/


Thanks
Ken'ichi Ohmichi

---
Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>

---
diff -rpuN a/include/linux/kexec.h b/include/linux/kexec.h
--- a/include/linux/kexec.h	2007-09-10 23:28:42.000000000 +0900
+++ b/include/linux/kexec.h	2007-09-10 23:29:52.000000000 +0900
@@ -132,11 +132,16 @@ unsigned long paddr_vmcoreinfo_note(void
  #define SIZE(name) \
  	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
  			      (unsigned long)sizeof(struct name))
+#define TYPEDEF_SIZE(name) \
+	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
+			      (unsigned long)sizeof(name))
  #define OFFSET(name, field) \
  	vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
  			      (unsigned long)&(((struct name *)0)->field))
  #define LENGTH(name, value) \
  	vmcoreinfo_append_str("LENGTH(%s)=%lu\n", #name, (unsigned long)value)
+#define NUMBER(name) \
+	vmcoreinfo_append_str("NUMBER(%s)=%ld\n", #name, (long)name)
  #define CONFIG(name) \
  	vmcoreinfo_append_str("CONFIG_%s=y\n", #name)

diff -rpuN a/kernel/kexec.c b/kernel/kexec.c
--- a/kernel/kexec.c	2007-09-10 23:28:42.000000000 +0900
+++ b/kernel/kexec.c	2007-09-10 23:29:02.000000000 +0900
@@ -1218,6 +1218,7 @@ static int __init crash_save_vmcoreinfo_
  	SIZE(zone);
  	SIZE(free_area);
  	SIZE(list_head);
+	TYPEDEF_SIZE(nodemask_t);
  	OFFSET(page, flags);
  	OFFSET(page, _count);
  	OFFSET(page, mapping);
@@ -1237,6 +1238,7 @@ static int __init crash_save_vmcoreinfo_
  	OFFSET(list_head, next);
  	OFFSET(list_head, prev);
  	LENGTH(zone.free_area, MAX_ORDER);
+	NUMBER(NR_FREE_PAGES);

  	arch_crash_save_vmcoreinfo();

_

Re: [PATCH 2/4] [-mm patch] Add nodemask_t's size and NR_FREE_PAGES's value to vmcoreinfo_data.

[David Rientjes]

On Fri, 14 Sep 2007, Ken'ichi Ohmichi wrote:

> diff -rpuN a/include/linux/kexec.h b/include/linux/kexec.h
> --- a/include/linux/kexec.h	2007-09-10 23:28:42.000000000 +0900
> +++ b/include/linux/kexec.h	2007-09-10 23:29:52.000000000 +0900
> @@ -132,11 +132,16 @@ unsigned long paddr_vmcoreinfo_note(void
>  #define SIZE(name) \
>  	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
>  			      (unsigned long)sizeof(struct name))
> +#define TYPEDEF_SIZE(name) \
> +	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
> +			      (unsigned long)sizeof(name))
>   #define OFFSET(name, field) \
>  	vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
>  			      (unsigned long)&(((struct name *)0)->field))
>  #define LENGTH(name, value) \
>  	vmcoreinfo_append_str("LENGTH(%s)=%lu\n", #name, (unsigned long)value)
> +#define NUMBER(name) \
> +	vmcoreinfo_append_str("NUMBER(%s)=%ld\n", #name, (long)name)
>  #define CONFIG(name) \
>  	vmcoreinfo_append_str("CONFIG_%s=y\n", #name)
> 

The #define SIZE() should be renamed STRUCT_SIZE() since it's always 
returning the size of the struct with a given name.  This would allow 
TYPEDEF_SIZE() to simply become SIZE() since it need not be used 
exclusively for typedefs.

Re: [PATCH 2/4] [-mm patch] Add nodemask_t's size and NR_FREE_PAGES's value to vmcoreinfo_data.

[Ken'ichi Ohmichi]

Hi David,

David Rientjes wrote:
> On Fri, 14 Sep 2007, Ken'ichi Ohmichi wrote:
>
>> diff -rpuN a/include/linux/kexec.h b/include/linux/kexec.h
>> --- a/include/linux/kexec.h    2007-09-10 23:28:42.000000000 +0900
>> +++ b/include/linux/kexec.h    2007-09-10 23:29:52.000000000 +0900
>> @@ -132,11 +132,16 @@ unsigned long paddr_vmcoreinfo_note(void
>>  #define SIZE(name) \
>>      vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
>>                    (unsigned long)sizeof(struct name))
>> +#define TYPEDEF_SIZE(name) \
>> +    vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
>> +                  (unsigned long)sizeof(name))
>>   #define OFFSET(name, field) \
>>      vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
>>                    (unsigned long)&(((struct name *)0)->field))
>>  #define LENGTH(name, value) \
>>      vmcoreinfo_append_str("LENGTH(%s)=%lu\n", #name, (unsigned long)value)
>> +#define NUMBER(name) \
>> +    vmcoreinfo_append_str("NUMBER(%s)=%ld\n", #name, (long)name)
>>  #define CONFIG(name) \
>>      vmcoreinfo_append_str("CONFIG_%s=y\n", #name)
>>
>
> The #define SIZE() should be renamed STRUCT_SIZE() since it's always returning the size of the struct with a given name.  This would allow TYPEDEF_SIZE() to simply become SIZE() since it need not be used exclusively for typedefs.

Thank you for the comment.
That sounds good, I'll post a new patch including it.


Thanks
Ken'ichi Ohmichi

[PATCH 3/4] [-mm patch] Use the existing ia64_tpa() instead of asm code.

[Ken'ichi Ohmichi]

[3/4] Use the existing ia64_tpa() instead of asm code.


Thanks
Ken'ichi Ohmichi

---
Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>

---
diff -rpuN a/arch/ia64/kernel/machine_kexec.c b/arch/ia64/kernel/machine_kexec.c
--- a/arch/ia64/kernel/machine_kexec.c	2007-09-10 23:30:33.000000000 +0900
+++ b/arch/ia64/kernel/machine_kexec.c	2007-09-10 23:31:37.000000000 +0900
@@ -21,6 +21,7 @@
  #include <asm/setup.h>
  #include <asm/delay.h>
  #include <asm/meminit.h>
+#include <asm/processor.h>

  typedef NORET_TYPE void (*relocate_new_kernel_t)(
  					unsigned long indirection_page,
@@ -149,9 +150,6 @@ void arch_crash_save_vmcoreinfo(void)

  unsigned long paddr_vmcoreinfo_note(void)
  {
-	unsigned long vaddr, paddr;
-	vaddr = (unsigned long)(char *)&vmcoreinfo_note;
-	asm volatile ("tpa %0 = %1" : "=r"(paddr) : "r"(vaddr) : "memory");
-	return paddr;
+	return ia64_tpa((unsigned long)(char *)&vmcoreinfo_note);
  }

_

[PATCH 4/4] [-mm patch] Add a prefix "VMCOREINFO_" to the vmcoreinfo macros.

[Ken'ichi Ohmichi]

[4/4] Add a prefix "VMCOREINFO_" to the vmcoreinfo macros.
   Old vmcoreinfo macros were defined as generic names SYMBOL/SIZE/OFFSET
   /LENGTH/CONFIG, and it is impossible to grep for them. So these names
   should be changed. This discussion is the following:
   http://www.ussg.iu.edu/hypermail/linux/kernel/0709.1/0415.html


Thanks
Ken'ichi Ohmichi

---
Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>

---
diff -rpuN a/arch/i386/kernel/machine_kexec.c b/arch/i386/kernel/machine_kexec.c
--- a/arch/i386/kernel/machine_kexec.c	2007-09-14 11:12:35.000000000 +0900
+++ b/arch/i386/kernel/machine_kexec.c	2007-09-14 11:12:43.000000000 +0900
@@ -174,11 +174,11 @@ early_param("crashkernel", parse_crashke
  void arch_crash_save_vmcoreinfo(void)
  {
  #ifdef CONFIG_ARCH_DISCONTIGMEM_ENABLE
-	SYMBOL(node_data);
-	LENGTH(node_data, MAX_NUMNODES);
+	VMCOREINFO_SYMBOL(node_data);
+	VMCOREINFO_LENGTH(node_data, MAX_NUMNODES);
  #endif
  #ifdef CONFIG_X86_PAE
-	CONFIG(X86_PAE);
+	VMCOREINFO_CONFIG(X86_PAE);
  #endif
  }

diff -rpuN a/arch/ia64/kernel/machine_kexec.c b/arch/ia64/kernel/machine_kexec.c
--- a/arch/ia64/kernel/machine_kexec.c	2007-09-14 11:12:35.000000000 +0900
+++ b/arch/ia64/kernel/machine_kexec.c	2007-09-14 11:12:43.000000000 +0900
@@ -132,19 +132,19 @@ void machine_kexec(struct kimage *image)
  void arch_crash_save_vmcoreinfo(void)
  {
  #ifdef CONFIG_ARCH_DISCONTIGMEM_ENABLE
-	SYMBOL(pgdat_list);
-	LENGTH(pgdat_list, MAX_NUMNODES);
+	VMCOREINFO_SYMBOL(pgdat_list);
+	VMCOREINFO_LENGTH(pgdat_list, MAX_NUMNODES);

-	SYMBOL(node_memblk);
-	LENGTH(node_memblk, NR_NODE_MEMBLKS);
-	SIZE(node_memblk_s);
-	OFFSET(node_memblk_s, start_paddr);
-	OFFSET(node_memblk_s, size);
+	VMCOREINFO_SYMBOL(node_memblk);
+	VMCOREINFO_LENGTH(node_memblk, NR_NODE_MEMBLKS);
+	VMCOREINFO_SIZE(node_memblk_s);
+	VMCOREINFO_OFFSET(node_memblk_s, start_paddr);
+	VMCOREINFO_OFFSET(node_memblk_s, size);
  #endif
  #ifdef CONFIG_PGTABLE_3
-	CONFIG(PGTABLE_3);
+	VMCOREINFO_CONFIG(PGTABLE_3);
  #elif  CONFIG_PGTABLE_4
-	CONFIG(PGTABLE_4);
+	VMCOREINFO_CONFIG(PGTABLE_4);
  #endif
  }

diff -rpuN a/arch/x86_64/kernel/machine_kexec.c b/arch/x86_64/kernel/machine_kexec.c
--- a/arch/x86_64/kernel/machine_kexec.c	2007-09-14 11:12:35.000000000 +0900
+++ b/arch/x86_64/kernel/machine_kexec.c	2007-09-14 11:12:43.000000000 +0900
@@ -261,8 +261,8 @@ early_param("crashkernel", setup_crashke
  void arch_crash_save_vmcoreinfo(void)
  {
  #ifdef CONFIG_ARCH_DISCONTIGMEM_ENABLE
-	SYMBOL(node_data);
-	LENGTH(node_data, MAX_NUMNODES);
+	VMCOREINFO_SYMBOL(node_data);
+	VMCOREINFO_LENGTH(node_data, MAX_NUMNODES);
  #endif
  }

diff -rpuN a/include/linux/kexec.h b/include/linux/kexec.h
--- a/include/linux/kexec.h	2007-09-14 11:12:34.000000000 +0900
+++ b/include/linux/kexec.h	2007-09-14 11:13:38.000000000 +0900
@@ -127,22 +127,22 @@ void vmcoreinfo_append_str(const char *f
  	__attribute__ ((format (printf, 1, 2)));
  unsigned long paddr_vmcoreinfo_note(void);

-#define SYMBOL(name) \
+#define VMCOREINFO_SYMBOL(name) \
  	vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)&name)
-#define SIZE(name) \
+#define VMCOREINFO_SIZE(name) \
  	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
  			      (unsigned long)sizeof(struct name))
-#define TYPEDEF_SIZE(name) \
+#define VMCOREINFO_TYPEDEF_SIZE(name) \
  	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
  			      (unsigned long)sizeof(name))
-#define OFFSET(name, field) \
+#define VMCOREINFO_OFFSET(name, field) \
  	vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
  			      (unsigned long)&(((struct name *)0)->field))
-#define LENGTH(name, value) \
+#define VMCOREINFO_LENGTH(name, value) \
  	vmcoreinfo_append_str("LENGTH(%s)=%lu\n", #name, (unsigned long)value)
-#define NUMBER(name) \
+#define VMCOREINFO_NUMBER(name) \
  	vmcoreinfo_append_str("NUMBER(%s)=%ld\n", #name, (long)name)
-#define CONFIG(name) \
+#define VMCOREINFO_CONFIG(name) \
  	vmcoreinfo_append_str("CONFIG_%s=y\n", #name)

  extern struct kimage *kexec_image;
diff -rpuN a/kernel/kexec.c b/kernel/kexec.c
--- a/kernel/kexec.c	2007-09-14 11:12:34.000000000 +0900
+++ b/kernel/kexec.c	2007-09-14 11:16:09.000000000 +0900
@@ -1198,47 +1198,47 @@ static int __init crash_save_vmcoreinfo_
  	vmcoreinfo_append_str("OSRELEASE=%s\n", init_uts_ns.name.release);
  	vmcoreinfo_append_str("PAGESIZE=%ld\n", PAGE_SIZE);

-	SYMBOL(init_uts_ns);
-	SYMBOL(node_online_map);
-	SYMBOL(swapper_pg_dir);
-	SYMBOL(_stext);
+	VMCOREINFO_SYMBOL(init_uts_ns);
+	VMCOREINFO_SYMBOL(node_online_map);
+	VMCOREINFO_SYMBOL(swapper_pg_dir);
+	VMCOREINFO_SYMBOL(_stext);

  #ifndef CONFIG_NEED_MULTIPLE_NODES
-	SYMBOL(mem_map);
-	SYMBOL(contig_page_data);
+	VMCOREINFO_SYMBOL(mem_map);
+	VMCOREINFO_SYMBOL(contig_page_data);
  #endif
  #ifdef CONFIG_SPARSEMEM
-	SYMBOL(mem_section);
-	LENGTH(mem_section, NR_SECTION_ROOTS);
-	SIZE(mem_section);
-	OFFSET(mem_section, section_mem_map);
+	VMCOREINFO_SYMBOL(mem_section);
+	VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
+	VMCOREINFO_SIZE(mem_section);
+	VMCOREINFO_OFFSET(mem_section, section_mem_map);
  #endif
-	SIZE(page);
-	SIZE(pglist_data);
-	SIZE(zone);
-	SIZE(free_area);
-	SIZE(list_head);
-	TYPEDEF_SIZE(nodemask_t);
-	OFFSET(page, flags);
-	OFFSET(page, _count);
-	OFFSET(page, mapping);
-	OFFSET(page, lru);
-	OFFSET(pglist_data, node_zones);
-	OFFSET(pglist_data, nr_zones);
+	VMCOREINFO_SIZE(page);
+	VMCOREINFO_SIZE(pglist_data);
+	VMCOREINFO_SIZE(zone);
+	VMCOREINFO_SIZE(free_area);
+	VMCOREINFO_SIZE(list_head);
+	VMCOREINFO_TYPEDEF_SIZE(nodemask_t);
+	VMCOREINFO_OFFSET(page, flags);
+	VMCOREINFO_OFFSET(page, _count);
+	VMCOREINFO_OFFSET(page, mapping);
+	VMCOREINFO_OFFSET(page, lru);
+	VMCOREINFO_OFFSET(pglist_data, node_zones);
+	VMCOREINFO_OFFSET(pglist_data, nr_zones);
  #ifdef CONFIG_FLAT_NODE_MEM_MAP
-	OFFSET(pglist_data, node_mem_map);
+	VMCOREINFO_OFFSET(pglist_data, node_mem_map);
  #endif
-	OFFSET(pglist_data, node_start_pfn);
-	OFFSET(pglist_data, node_spanned_pages);
-	OFFSET(pglist_data, node_id);
-	OFFSET(zone, free_area);
-	OFFSET(zone, vm_stat);
-	OFFSET(zone, spanned_pages);
-	OFFSET(free_area, free_list);
-	OFFSET(list_head, next);
-	OFFSET(list_head, prev);
-	LENGTH(zone.free_area, MAX_ORDER);
-	NUMBER(NR_FREE_PAGES);
+	VMCOREINFO_OFFSET(pglist_data, node_start_pfn);
+	VMCOREINFO_OFFSET(pglist_data, node_spanned_pages);
+	VMCOREINFO_OFFSET(pglist_data, node_id);
+	VMCOREINFO_OFFSET(zone, free_area);
+	VMCOREINFO_OFFSET(zone, vm_stat);
+	VMCOREINFO_OFFSET(zone, spanned_pages);
+	VMCOREINFO_OFFSET(free_area, free_list);
+	VMCOREINFO_OFFSET(list_head, next);
+	VMCOREINFO_OFFSET(list_head, prev);
+	VMCOREINFO_LENGTH(zone.free_area, MAX_ORDER);
+	VMCOREINFO_NUMBER(NR_FREE_PAGES);

  	arch_crash_save_vmcoreinfo();

_

Re: [PATCH 4/4] [-mm patch] Add a prefix "VMCOREINFO_" to the vmcoreinfo macros.

[Andrew Morton]

On Fri, 14 Sep 2007 12:00:18 +0900 "Ken'ichi Ohmichi" <oomichi@mxs.nes.nec.co.jp> wrote:

> [4/4] Add a prefix "VMCOREINFO_" to the vmcoreinfo macros.
>    Old vmcoreinfo macros were defined as generic names SYMBOL/SIZE/OFFSET
>    /LENGTH/CONFIG, and it is impossible to grep for them. So these names
>    should be changed. This discussion is the following:
>    http://www.ussg.iu.edu/hypermail/linux/kernel/0709.1/0415.html

I already had these patches, except for #4.

Your email client is space-stuffing the patches, so I have to do s/^ / /g
to apply them.  Please see
http://mbligh.org/linuxdocs/Email/Clients/Thunderbird and/or the
soon-to-be-merged Documentation/email-clients.txt

I plan on folding all of

add-vmcoreinfo.patch
add-vmcore-cleanup-the-coding-style-according-to-andrews-comments.patch
add-vmcore-add-nodemask_ts-size-and-nr_free_pagess-value-to-vmcoreinfo_data.patch
add-vmcore-use-the-existing-ia64_tpa-instead-of-asm-code.patch
add-vmcore-add-a-prefix-vmcoreinfo_-to-the-vmcoreinfo-macros.patch

into a single patch for upstream.

Re: [PATCH 4/4] [-mm patch] Add a prefix "VMCOREINFO_" to the vmcoreinfo macros.

[Ken'ichi Ohmichi]

Hi Andrew,

Andrew Morton wrote:
> > On Fri, 14 Sep 2007 12:00:18 +0900 "Ken'ichi Ohmichi" <oomichi@mxs.nes.nec.co.jp> wrote:
> > 
>> >> [4/4] Add a prefix "VMCOREINFO_" to the vmcoreinfo macros.
>> >>    Old vmcoreinfo macros were defined as generic names SYMBOL/SIZE/OFFSET
>> >>    /LENGTH/CONFIG, and it is impossible to grep for them. So these names
>> >>    should be changed. This discussion is the following:
>> >>    http://www.ussg.iu.edu/hypermail/linux/kernel/0709.1/0415.html
> > 
> > I already had these patches, except for #4.
> > 
> > Your email client is space-stuffing the patches, so I have to do s/^ / /g
> > to apply them.  Please see
> > http://mbligh.org/linuxdocs/Email/Clients/Thunderbird and/or the
> > soon-to-be-merged Documentation/email-clients.txt

Sorry for wasting your time.
I changed my thunderbird's setting according to the above site.


> > I plan on folding all of
> > 
> > add-vmcoreinfo.patch
> > add-vmcore-cleanup-the-coding-style-according-to-andrews-comments.patch
> > add-vmcore-add-nodemask_ts-size-and-nr_free_pagess-value-to-vmcoreinfo_data.patch
> > add-vmcore-use-the-existing-ia64_tpa-instead-of-asm-code.patch
> > add-vmcore-add-a-prefix-vmcoreinfo_-to-the-vmcoreinfo-macros.patch
> > 
> > into a single patch for upstream.

Thank you for merging them into linux-2.6.23-rc6-mm1.
Additionally, could you merge the following patches to clean up
add-vmcoreinfo.patch ?

[PATCH 5/4] [-mm patch] Rename macros returning the size.
 The #define SIZE() should be renamed STRUCT_SIZE() since it's always 
 returning the size of the struct with a given name.  This would allow 
 TYPEDEF_SIZE() to simply become SIZE() since it need not be used 
 exclusively for typedefs. This idea is David Rientjes's.
 http://www.ussg.iu.edu/hypermail/linux/kernel/0709.1/1964.html
 
[PATCH 6/4] [-mm patch] use the existing offsetof().
 It is better that offsetof() is used for VMCOREINFO_OFFSET().
 This idea is Joe Perches's.


Thanks
Ken'ichi Ohmichi 

[PATCH 5/4] [-mm patch] Rename macros returning the size.

[Ken'ichi Ohmichi]

[PATCH 5/4] [-mm patch] Rename macros returning the size.
 The #define SIZE() should be renamed STRUCT_SIZE() since it's always 
 returning the size of the struct with a given name.  This would allow 
 TYPEDEF_SIZE() to simply become SIZE() since it need not be used 
 exclusively for typedefs. This idea is David Rientjes's.
 http://www.ussg.iu.edu/hypermail/linux/kernel/0709.1/1964.html

Thanks
Ken'ichi Ohmichi

---
Signed-off-by: David Rientjes <rientjes@google.com>
Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>

---
diff -rpuN a/arch/ia64/kernel/machine_kexec.c b/arch/ia64/kernel/machine_kexec.c
--- a/arch/ia64/kernel/machine_kexec.c	2007-09-18 15:22:19.000000000 +0900
+++ b/arch/ia64/kernel/machine_kexec.c	2007-09-18 15:22:41.000000000 +0900
@@ -137,7 +137,7 @@ void arch_crash_save_vmcoreinfo(void)
 
 	VMCOREINFO_SYMBOL(node_memblk);
 	VMCOREINFO_LENGTH(node_memblk, NR_NODE_MEMBLKS);
-	VMCOREINFO_SIZE(node_memblk_s);
+	VMCOREINFO_STRUCT_SIZE(node_memblk_s);
 	VMCOREINFO_OFFSET(node_memblk_s, start_paddr);
 	VMCOREINFO_OFFSET(node_memblk_s, size);
 #endif
diff -rpuN a/include/linux/kexec.h b/include/linux/kexec.h
--- a/include/linux/kexec.h	2007-09-18 15:22:19.000000000 +0900
+++ b/include/linux/kexec.h	2007-09-18 15:23:22.000000000 +0900
@@ -131,10 +131,10 @@ unsigned long paddr_vmcoreinfo_note(void
 	vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)&name)
 #define VMCOREINFO_SIZE(name) \
 	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
-			      (unsigned long)sizeof(struct name))
-#define VMCOREINFO_TYPEDEF_SIZE(name) \
-	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
 			      (unsigned long)sizeof(name))
+#define VMCOREINFO_STRUCT_SIZE(name) \
+	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
+			      (unsigned long)sizeof(struct name))
 #define VMCOREINFO_OFFSET(name, field) \
 	vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
 			      (unsigned long)&(((struct name *)0)->field))
diff -rpuN a/kernel/kexec.c b/kernel/kexec.c
--- a/kernel/kexec.c	2007-09-18 15:22:19.000000000 +0900
+++ b/kernel/kexec.c	2007-09-18 15:22:41.000000000 +0900
@@ -1210,15 +1210,15 @@ static int __init crash_save_vmcoreinfo_
 #ifdef CONFIG_SPARSEMEM
 	VMCOREINFO_SYMBOL(mem_section);
 	VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
-	VMCOREINFO_SIZE(mem_section);
+	VMCOREINFO_STRUCT_SIZE(mem_section);
 	VMCOREINFO_OFFSET(mem_section, section_mem_map);
 #endif
-	VMCOREINFO_SIZE(page);
-	VMCOREINFO_SIZE(pglist_data);
-	VMCOREINFO_SIZE(zone);
-	VMCOREINFO_SIZE(free_area);
-	VMCOREINFO_SIZE(list_head);
-	VMCOREINFO_TYPEDEF_SIZE(nodemask_t);
+	VMCOREINFO_STRUCT_SIZE(page);
+	VMCOREINFO_STRUCT_SIZE(pglist_data);
+	VMCOREINFO_STRUCT_SIZE(zone);
+	VMCOREINFO_STRUCT_SIZE(free_area);
+	VMCOREINFO_STRUCT_SIZE(list_head);
+	VMCOREINFO_SIZE(nodemask_t);
 	VMCOREINFO_OFFSET(page, flags);
 	VMCOREINFO_OFFSET(page, _count);
 	VMCOREINFO_OFFSET(page, mapping);
_

Re: [PATCH 5/4] [-mm patch] Rename macros returning the size.

[Satyam Sharma]

Hi,


On Thu, 20 Sep 2007, Ken'ichi Ohmichi wrote:
> 
> [PATCH 5/4] [-mm patch] Rename macros returning the size.
>  The #define SIZE() should be renamed STRUCT_SIZE() since it's always 
>  returning the size of the struct with a given name.  This would allow 
>  TYPEDEF_SIZE() to simply become SIZE() since it need not be used 
>  exclusively for typedefs. This idea is David Rientjes's.
>  http://www.ussg.iu.edu/hypermail/linux/kernel/0709.1/1964.html
> 
> Thanks
> Ken'ichi Ohmichi
> 
> ---
> Signed-off-by: David Rientjes <rientjes@google.com>

Hmm, I think adding a s-o-b line for David here isn't quite correct.
When someone reviews a patch and gives a suggestion, you only need to
copy him on the next iteration (and he may ack it or whatever, if he
wants) -- but adding a s-o-b line like that ends up (incorrectly)
denoting that he came between the author-to-git-commit chain ...


> Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>

> --- a/include/linux/kexec.h	2007-09-18 15:22:19.000000000 +0900
> +++ b/include/linux/kexec.h	2007-09-18 15:23:22.000000000 +0900
> @@ -131,10 +131,10 @@ unsigned long paddr_vmcoreinfo_note(void
>  	vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)&name)
>  #define VMCOREINFO_SIZE(name) \
>  	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
> -			      (unsigned long)sizeof(struct name))
> -#define VMCOREINFO_TYPEDEF_SIZE(name) \
> -	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
>  			      (unsigned long)sizeof(name))
> +#define VMCOREINFO_STRUCT_SIZE(name) \
> +	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
> +			      (unsigned long)sizeof(struct name))
>  #define VMCOREINFO_OFFSET(name, field) \
>  	vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
>  			      (unsigned long)&(((struct name *)0)->field))

Please use %zu and lose all the ugly (unsigned long) casts here.

Otherwise it looks good,

Satyam

Re: [PATCH 5/4] [-mm patch] Rename macros returning the size.

[David Rientjes]

On Thu, 20 Sep 2007, Satyam Sharma wrote:

> > [PATCH 5/4] [-mm patch] Rename macros returning the size.
> >  The #define SIZE() should be renamed STRUCT_SIZE() since it's always 
> >  returning the size of the struct with a given name.  This would allow 
> >  TYPEDEF_SIZE() to simply become SIZE() since it need not be used 
> >  exclusively for typedefs. This idea is David Rientjes's.
> >  http://www.ussg.iu.edu/hypermail/linux/kernel/0709.1/1964.html
> > 
> > Thanks
> > Ken'ichi Ohmichi
> > 
> > ---
> > Signed-off-by: David Rientjes <rientjes@google.com>
> 
> Hmm, I think adding a s-o-b line for David here isn't quite correct.
> When someone reviews a patch and gives a suggestion, you only need to
> copy him on the next iteration (and he may ack it or whatever, if he
> wants) -- but adding a s-o-b line like that ends up (incorrectly)
> denoting that he came between the author-to-git-commit chain ...
> 

Feel free to add

	Acked-by: David Rientjes <rientjes@google.com>

since I've reviewed the patch and agree with it.

Re: [PATCH 5/4] [-mm patch] Rename macros returning the size.

[Ken'ichi Ohmichi]

Hi Satyam,

Satyam Sharma wrote:
> > On Thu, 20 Sep 2007, Ken'ichi Ohmichi wrote:
>> >> [PATCH 5/4] [-mm patch] Rename macros returning the size.
>> >>  The #define SIZE() should be renamed STRUCT_SIZE() since it's always 
>> >>  returning the size of the struct with a given name.  This would allow 
>> >>  TYPEDEF_SIZE() to simply become SIZE() since it need not be used 
>> >>  exclusively for typedefs. This idea is David Rientjes's.
>> >>  http://www.ussg.iu.edu/hypermail/linux/kernel/0709.1/1964.html
>> >>
>> >> Thanks
>> >> Ken'ichi Ohmichi
>> >>
>> >> ---
>> >> Signed-off-by: David Rientjes <rientjes@google.com>
> > 
> > Hmm, I think adding a s-o-b line for David here isn't quite correct.
> > When someone reviews a patch and gives a suggestion, you only need to
> > copy him on the next iteration (and he may ack it or whatever, if he
> > wants) -- but adding a s-o-b line like that ends up (incorrectly)
> > denoting that he came between the author-to-git-commit chain ...

Thank you for kind explanation.
I can understand s-o-b clearly.


>> >> Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>
> > 
>> >> --- a/include/linux/kexec.h	2007-09-18 15:22:19.000000000 +0900
>> >> +++ b/include/linux/kexec.h	2007-09-18 15:23:22.000000000 +0900
>> >> @@ -131,10 +131,10 @@ unsigned long paddr_vmcoreinfo_note(void
>> >>  	vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)&name)
>> >>  #define VMCOREINFO_SIZE(name) \
>> >>  	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
>> >> -			      (unsigned long)sizeof(struct name))
>> >> -#define VMCOREINFO_TYPEDEF_SIZE(name) \
>> >> -	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
>> >>  			      (unsigned long)sizeof(name))
>> >> +#define VMCOREINFO_STRUCT_SIZE(name) \
>> >> +	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
>> >> +			      (unsigned long)sizeof(struct name))
>> >>  #define VMCOREINFO_OFFSET(name, field) \
>> >>  	vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
>> >>  			      (unsigned long)&(((struct name *)0)->field))
> > 
> > Please use %zu and lose all the ugly (unsigned long) casts here.

That sounds good.
I updated the patch according to your comment.


Thanks
Ken'ichi Ohmichi 

---
Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>
Acked-by: David Rientjes <rientjes@google.com>

---
diff -rpuN a/arch/ia64/kernel/machine_kexec.c b/arch/ia64/kernel/machine_kexec.c
--- a/arch/ia64/kernel/machine_kexec.c	2007-09-21 09:25:35.000000000 +0900
+++ b/arch/ia64/kernel/machine_kexec.c	2007-09-21 09:26:13.000000000 +0900
@@ -137,7 +137,7 @@ void arch_crash_save_vmcoreinfo(void)
 
 	VMCOREINFO_SYMBOL(node_memblk);
 	VMCOREINFO_LENGTH(node_memblk, NR_NODE_MEMBLKS);
-	VMCOREINFO_SIZE(node_memblk_s);
+	VMCOREINFO_STRUCT_SIZE(node_memblk_s);
 	VMCOREINFO_OFFSET(node_memblk_s, start_paddr);
 	VMCOREINFO_OFFSET(node_memblk_s, size);
 #endif
diff -rpuN a/include/linux/kexec.h b/include/linux/kexec.h
--- a/include/linux/kexec.h	2007-09-21 09:25:32.000000000 +0900
+++ b/include/linux/kexec.h	2007-09-21 09:27:23.000000000 +0900
@@ -130,11 +130,9 @@ unsigned long paddr_vmcoreinfo_note(void
 #define VMCOREINFO_SYMBOL(name) \
 	vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)&name)
 #define VMCOREINFO_SIZE(name) \
-	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
-			      (unsigned long)sizeof(struct name))
-#define VMCOREINFO_TYPEDEF_SIZE(name) \
-	vmcoreinfo_append_str("SIZE(%s)=%lu\n", #name, \
-			      (unsigned long)sizeof(name))
+	vmcoreinfo_append_str("SIZE(%s)=%zu\n", #name, sizeof(name))
+#define VMCOREINFO_STRUCT_SIZE(name) \
+	vmcoreinfo_append_str("SIZE(%s)=%zu\n", #name, sizeof(struct name))
 #define VMCOREINFO_OFFSET(name, field) \
 	vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
 			      (unsigned long)&(((struct name *)0)->field))
diff -rpuN a/kernel/kexec.c b/kernel/kexec.c
--- a/kernel/kexec.c	2007-09-21 09:25:27.000000000 +0900
+++ b/kernel/kexec.c	2007-09-21 09:26:13.000000000 +0900
@@ -1210,15 +1210,15 @@ static int __init crash_save_vmcoreinfo_
 #ifdef CONFIG_SPARSEMEM
 	VMCOREINFO_SYMBOL(mem_section);
 	VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
-	VMCOREINFO_SIZE(mem_section);
+	VMCOREINFO_STRUCT_SIZE(mem_section);
 	VMCOREINFO_OFFSET(mem_section, section_mem_map);
 #endif
-	VMCOREINFO_SIZE(page);
-	VMCOREINFO_SIZE(pglist_data);
-	VMCOREINFO_SIZE(zone);
-	VMCOREINFO_SIZE(free_area);
-	VMCOREINFO_SIZE(list_head);
-	VMCOREINFO_TYPEDEF_SIZE(nodemask_t);
+	VMCOREINFO_STRUCT_SIZE(page);
+	VMCOREINFO_STRUCT_SIZE(pglist_data);
+	VMCOREINFO_STRUCT_SIZE(zone);
+	VMCOREINFO_STRUCT_SIZE(free_area);
+	VMCOREINFO_STRUCT_SIZE(list_head);
+	VMCOREINFO_SIZE(nodemask_t);
 	VMCOREINFO_OFFSET(page, flags);
 	VMCOREINFO_OFFSET(page, _count);
 	VMCOREINFO_OFFSET(page, mapping);
_

[PATCH 6/4] [-mm patch] use the existing offsetof().

[Ken'ichi Ohmichi]

[PATCH 6/4] [-mm patch] use the existing offsetof().
 It is better that offsetof() is used for VMCOREINFO_OFFSET().
 This idea is Joe Perches's.



Thanks
Ken'ichi Ohmichi 

---
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>
---
diff -rpuN a/include/linux/kexec.h b/include/linux/kexec.h
--- a/include/linux/kexec.h	2007-09-18 15:23:22.000000000 +0900
+++ b/include/linux/kexec.h	2007-09-18 15:27:29.000000000 +0900
@@ -137,7 +137,7 @@ unsigned long paddr_vmcoreinfo_note(void
 			      (unsigned long)sizeof(struct name))
 #define VMCOREINFO_OFFSET(name, field) \
 	vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
-			      (unsigned long)&(((struct name *)0)->field))
+			      (unsigned long)offsetof(struct name, field))
 #define VMCOREINFO_LENGTH(name, value) \
 	vmcoreinfo_append_str("LENGTH(%s)=%lu\n", #name, (unsigned long)value)
 #define VMCOREINFO_NUMBER(name) \
_

Re: [PATCH 6/4] [-mm patch] use the existing offsetof().

[Satyam Sharma]

On Thu, 20 Sep 2007, Ken'ichi Ohmichi wrote:
> 
> [PATCH 6/4] [-mm patch] use the existing offsetof().
>  It is better that offsetof() is used for VMCOREINFO_OFFSET().
>  This idea is Joe Perches's.
> 
> 
> 
> Thanks
> Ken'ichi Ohmichi 
> 
> ---
> Signed-off-by: Joe Perches <joe@perches.com>

Same deal here ...


> Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>
> ---
> diff -rpuN a/include/linux/kexec.h b/include/linux/kexec.h
> --- a/include/linux/kexec.h	2007-09-18 15:23:22.000000000 +0900
> +++ b/include/linux/kexec.h	2007-09-18 15:27:29.000000000 +0900
> @@ -137,7 +137,7 @@ unsigned long paddr_vmcoreinfo_note(void
>  			      (unsigned long)sizeof(struct name))
>  #define VMCOREINFO_OFFSET(name, field) \
>  	vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
> -			      (unsigned long)&(((struct name *)0)->field))
> +			      (unsigned long)offsetof(struct name, field))
>  #define VMCOREINFO_LENGTH(name, value) \
>  	vmcoreinfo_append_str("LENGTH(%s)=%lu\n", #name, (unsigned long)value)

... and here. Use %zu and lose the casts.

BTW I don't think these macros are such a big win in readability or
code clarity anyway. And I noticed that there are still some open
calls to vmcoreinfo_append_str() in kexec.c (such as for the OS_RELEASE
case) which you haven't macro-ized ... we end up having code that looks
like:

	vmcoreinfo_append_str(...);
	...

	...
	VMCOREINFO_OFFSET(...);
	...

and it's not even obvious (unless you follow on to the definition of the
later macro) that the above two are *exactly* the same. So you could also
consider just losing the macros altogether.


Satyam

Re: [PATCH 6/4] [-mm patch] use the existing offsetof().

[Ken'ichi Ohmichi]

Hi Satyam,

Satyam Sharma wrote:
> > BTW I don't think these macros are such a big win in readability or
> > code clarity anyway. And I noticed that there are still some open
> > calls to vmcoreinfo_append_str() in kexec.c (such as for the OS_RELEASE
> > case) which you haven't macro-ized ... we end up having code that looks
> > like:
> > 
> > 	vmcoreinfo_append_str(...);
> > 	...
> > 
> > 	...
> > 	VMCOREINFO_OFFSET(...);
> > 	...
> > 
> > and it's not even obvious (unless you follow on to the definition of the
> > later macro) that the above two are *exactly* the same. So you could also
> > consider just losing the macros altogether.

I want to use these macros for calls to vmcoreinfo_append_str(), because
the strings included in each macro ("SIZE(XXX)=", "OFFSET(XXX)=", etc.)
are very important. makedumpfile command reads these strings and gets the
debugging information (structure sizes, field offsets, etc.). If losing
these macros and there is a typo like "OFESET(page.mapping)=", the command
cannot run. To prevent this mistake, these macros are very useful.

For more readability, I think it is good that all the calls are changed
to macros having a prefix "VMCOREINFO_" like the attached patch.


Thanks
Ken'ichi Ohmichi 

---
Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>

---
diff -rpuN a/include/linux/kexec.h b/include/linux/kexec.h
--- a/include/linux/kexec.h	2007-09-21 09:28:23.000000000 +0900
+++ b/include/linux/kexec.h	2007-09-21 10:26:24.000000000 +0900
@@ -127,6 +127,10 @@ void vmcoreinfo_append_str(const char *f
 	__attribute__ ((format (printf, 1, 2)));
 unsigned long paddr_vmcoreinfo_note(void);
 
+#define VMCOREINFO_OSRELEASE(name) \
+	vmcoreinfo_append_str("OSRELEASE=%s\n", #name)
+#define VMCOREINFO_PAGESIZE(value) \
+	vmcoreinfo_append_str("PAGESIZE=%ld\n", value)
 #define VMCOREINFO_SYMBOL(name) \
 	vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)&name)
 #define VMCOREINFO_SIZE(name) \
@@ -134,8 +138,8 @@ unsigned long paddr_vmcoreinfo_note(void
 #define VMCOREINFO_STRUCT_SIZE(name) \
 	vmcoreinfo_append_str("SIZE(%s)=%zu\n", #name, sizeof(struct name))
 #define VMCOREINFO_OFFSET(name, field) \
-	vmcoreinfo_append_str("OFFSET(%s.%s)=%lu\n", #name, #field, \
-			      (unsigned long)&(((struct name *)0)->field))
+	vmcoreinfo_append_str("OFFSET(%s.%s)=%zu\n", #name, #field, \
+			      offsetof(struct name, field))
 #define VMCOREINFO_LENGTH(name, value) \
 	vmcoreinfo_append_str("LENGTH(%s)=%lu\n", #name, (unsigned long)value)
 #define VMCOREINFO_NUMBER(name) \
diff -rpuN a/kernel/kexec.c b/kernel/kexec.c
--- a/kernel/kexec.c	2007-09-21 09:28:22.000000000 +0900
+++ b/kernel/kexec.c	2007-09-21 10:17:56.000000000 +0900
@@ -1195,8 +1195,8 @@ unsigned long __attribute__ ((weak)) pad
 
 static int __init crash_save_vmcoreinfo_init(void)
 {
-	vmcoreinfo_append_str("OSRELEASE=%s\n", init_uts_ns.name.release);
-	vmcoreinfo_append_str("PAGESIZE=%ld\n", PAGE_SIZE);
+	VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
+	VMCOREINFO_PAGESIZE(PAGE_SIZE);
 
 	VMCOREINFO_SYMBOL(init_uts_ns);
 	VMCOREINFO_SYMBOL(node_online_map);
_

Re: [PATCH 4/4] [-mm patch] Add a prefix "VMCOREINFO_" to the vmcoreinfo macros.

[Ken'ichi Ohmichi]

Hi Andrew,

Andrew Morton wrote:
> > I plan on folding all of
> > 
> > add-vmcoreinfo.patch
> > add-vmcore-cleanup-the-coding-style-according-to-andrews-comments.patch
> > add-vmcore-add-nodemask_ts-size-and-nr_free_pagess-value-to-vmcoreinfo_data.patch
> > add-vmcore-use-the-existing-ia64_tpa-instead-of-asm-code.patch
> > add-vmcore-add-a-prefix-vmcoreinfo_-to-the-vmcoreinfo-macros.patch
> > 
> > into a single patch for upstream.

I created a new single add-vmcoreinfo.patch with the following cleanup patches.

* Cleanup patches:
  1. add-vmcore-cleanup-the-coding-style-according-to-andrews-comments.patch
  2. add-vmcore-add-nodemask_ts-size-and-nr_free_pagess-value-to-vmcoreinfo_data.patch
  3. add-vmcore-use-the-existing-ia64_tpa-instead-of-asm-code.patch
  4. add-vmcore-add-a-prefix-vmcoreinfo_-to-the-vmcoreinfo-macros.patch
  5. Re: [PATCH 5/4] [-mm patch] Rename macros returning the size.
     from Ken'ichi Ohmichi, 2007/09/25
     http://www.ussg.iu.edu/hypermail/linux/kernel/0709.3/0582.html
  6. Re: [PATCH 6/4] [-mm patch] use the existing offsetof().
     from Ken'ichi Ohmichi, 2007/09/25
     http://www.ussg.iu.edu/hypermail/linux/kernel/0709.3/0584.html

Thanks
Ken'ichi Ohmichi 

---
Signed-off-by: Dan Aloni <da-x@monatomic.org>
Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>
Signed-off-by: Bernhard Walle <bwalle@suse.de>
Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

---
diff -rpuN a/arch/i386/kernel/machine_kexec.c b/arch/i386/kernel/machine_kexec.c
--- a/arch/i386/kernel/machine_kexec.c	2007-09-21 13:56:02.000000000 +0900
+++ b/arch/i386/kernel/machine_kexec.c	2007-09-21 13:56:13.000000000 +0900
@@ -10,6 +10,7 @@
 #include <linux/kexec.h>
 #include <linux/delay.h>
 #include <linux/init.h>
+#include <linux/numa.h>
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
 #include <asm/tlbflush.h>
@@ -169,3 +170,15 @@ static int __init parse_crashkernel(char
 	return 0;
 }
 early_param("crashkernel", parse_crashkernel);
+
+void arch_crash_save_vmcoreinfo(void)
+{
+#ifdef CONFIG_ARCH_DISCONTIGMEM_ENABLE
+	VMCOREINFO_SYMBOL(node_data);
+	VMCOREINFO_LENGTH(node_data, MAX_NUMNODES);
+#endif
+#ifdef CONFIG_X86_PAE
+	VMCOREINFO_CONFIG(X86_PAE);
+#endif
+}
+
diff -rpuN a/arch/ia64/kernel/machine_kexec.c b/arch/ia64/kernel/machine_kexec.c
--- a/arch/ia64/kernel/machine_kexec.c	2007-09-21 13:56:02.000000000 +0900
+++ b/arch/ia64/kernel/machine_kexec.c	2007-09-21 14:00:15.000000000 +0900
@@ -15,10 +15,13 @@
 #include <linux/cpu.h>
 #include <linux/irq.h>
 #include <linux/efi.h>
+#include <linux/numa.h>
+#include <linux/mmzone.h>
 #include <asm/mmu_context.h>
 #include <asm/setup.h>
 #include <asm/delay.h>
 #include <asm/meminit.h>
+#include <asm/processor.h>
 
 typedef NORET_TYPE void (*relocate_new_kernel_t)(
 					unsigned long indirection_page,
@@ -125,3 +128,28 @@ void machine_kexec(struct kimage *image)
 	unw_init_running(ia64_machine_kexec, image);
 	for(;;);
 }
+
+void arch_crash_save_vmcoreinfo(void)
+{
+#ifdef CONFIG_ARCH_DISCONTIGMEM_ENABLE
+	VMCOREINFO_SYMBOL(pgdat_list);
+	VMCOREINFO_LENGTH(pgdat_list, MAX_NUMNODES);
+
+	VMCOREINFO_SYMBOL(node_memblk);
+	VMCOREINFO_LENGTH(node_memblk, NR_NODE_MEMBLKS);
+	VMCOREINFO_STRUCT_SIZE(node_memblk_s);
+	VMCOREINFO_OFFSET(node_memblk_s, start_paddr);
+	VMCOREINFO_OFFSET(node_memblk_s, size);
+#endif
+#ifdef CONFIG_PGTABLE_3
+	VMCOREINFO_CONFIG(PGTABLE_3);
+#elif CONFIG_PGTABLE_4
+	VMCOREINFO_CONFIG(PGTABLE_4);
+#endif
+}
+
+unsigned long paddr_vmcoreinfo_note(void)
+{
+	return ia64_tpa((unsigned long)(char *)&vmcoreinfo_note);
+}
+
diff -rpuN a/arch/ia64/mm/discontig.c b/arch/ia64/mm/discontig.c
--- a/arch/ia64/mm/discontig.c	2007-09-21 13:56:02.000000000 +0900
+++ b/arch/ia64/mm/discontig.c	2007-09-21 13:56:14.000000000 +0900
@@ -48,7 +48,7 @@ struct early_node_data {
 static struct early_node_data mem_data[MAX_NUMNODES] __initdata;
 static nodemask_t memory_less_mask __initdata;
 
-static pg_data_t *pgdat_list[MAX_NUMNODES];
+pg_data_t *pgdat_list[MAX_NUMNODES];
 
 /*
  * To prevent cache aliasing effects, align per-node structures so that they
diff -rpuN a/arch/x86_64/kernel/machine_kexec.c b/arch/x86_64/kernel/machine_kexec.c
--- a/arch/x86_64/kernel/machine_kexec.c	2007-09-21 13:56:02.000000000 +0900
+++ b/arch/x86_64/kernel/machine_kexec.c	2007-09-21 13:56:14.000000000 +0900
@@ -10,6 +10,7 @@
 #include <linux/kexec.h>
 #include <linux/string.h>
 #include <linux/reboot.h>
+#include <linux/numa.h>
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
 #include <asm/mmu_context.h>
@@ -257,3 +258,11 @@ static int __init setup_crashkernel(char
 }
 early_param("crashkernel", setup_crashkernel);
 
+void arch_crash_save_vmcoreinfo(void)
+{
+#ifdef CONFIG_ARCH_DISCONTIGMEM_ENABLE
+	VMCOREINFO_SYMBOL(node_data);
+	VMCOREINFO_LENGTH(node_data, MAX_NUMNODES);
+#endif
+}
+
diff -rpuN a/include/asm-ia64/numa.h b/include/asm-ia64/numa.h
--- a/include/asm-ia64/numa.h	2007-09-21 13:56:02.000000000 +0900
+++ b/include/asm-ia64/numa.h	2007-09-21 13:56:12.000000000 +0900
@@ -24,6 +24,7 @@
 
 extern u16 cpu_to_node_map[NR_CPUS] __cacheline_aligned;
 extern cpumask_t node_to_cpu_mask[MAX_NUMNODES] __cacheline_aligned;
+extern pg_data_t *pgdat_list[MAX_NUMNODES];
 
 /* Stuff below this line could be architecture independent */
 
diff -rpuN a/include/linux/kexec.h b/include/linux/kexec.h
--- a/include/linux/kexec.h	2007-09-21 13:56:02.000000000 +0900
+++ b/include/linux/kexec.h	2007-09-21 13:56:30.000000000 +0900
@@ -121,6 +121,32 @@ extern struct page *kimage_alloc_control
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
 void crash_save_cpu(struct pt_regs *regs, int cpu);
+void crash_save_vmcoreinfo(void);
+void arch_crash_save_vmcoreinfo(void);
+void vmcoreinfo_append_str(const char *fmt, ...)
+	__attribute__ ((format (printf, 1, 2)));
+unsigned long paddr_vmcoreinfo_note(void);
+
+#define VMCOREINFO_OSRELEASE(name) \
+	vmcoreinfo_append_str("OSRELEASE=%s\n", #name)
+#define VMCOREINFO_PAGESIZE(value) \
+	vmcoreinfo_append_str("PAGESIZE=%ld\n", value)
+#define VMCOREINFO_SYMBOL(name) \
+	vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)&name)
+#define VMCOREINFO_SIZE(name) \
+	vmcoreinfo_append_str("SIZE(%s)=%zu\n", #name, sizeof(name))
+#define VMCOREINFO_STRUCT_SIZE(name) \
+	vmcoreinfo_append_str("SIZE(%s)=%zu\n", #name, sizeof(struct name))
+#define VMCOREINFO_OFFSET(name, field) \
+	vmcoreinfo_append_str("OFFSET(%s.%s)=%zu\n", #name, #field, \
+			      offsetof(struct name, field))
+#define VMCOREINFO_LENGTH(name, value) \
+	vmcoreinfo_append_str("LENGTH(%s)=%lu\n", #name, (unsigned long)value)
+#define VMCOREINFO_NUMBER(name) \
+	vmcoreinfo_append_str("NUMBER(%s)=%ld\n", #name, (long)name)
+#define VMCOREINFO_CONFIG(name) \
+	vmcoreinfo_append_str("CONFIG_%s=y\n", #name)
+
 extern struct kimage *kexec_image;
 extern struct kimage *kexec_crash_image;
 
@@ -148,11 +174,20 @@ extern struct kimage *kexec_crash_image;
 
 #define KEXEC_FLAGS    (KEXEC_ON_CRASH)  /* List of defined/legal kexec flags */
 
+#define VMCOREINFO_BYTES           (4096)
+#define VMCOREINFO_NOTE_NAME       "VMCOREINFO"
+#define VMCOREINFO_NOTE_NAME_BYTES ALIGN(sizeof(VMCOREINFO_NOTE_NAME), 4)
+#define VMCOREINFO_NOTE_SIZE       (KEXEC_NOTE_HEAD_BYTES*2 + VMCOREINFO_BYTES \
+				    + VMCOREINFO_NOTE_NAME_BYTES)
+
 /* Location of a reserved region to hold the crash kernel.
  */
 extern struct resource crashk_res;
 typedef u32 note_buf_t[KEXEC_NOTE_BYTES/4];
 extern note_buf_t *crash_notes;
+extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
+extern size_t vmcoreinfo_size;
+extern size_t vmcoreinfo_max_size;
 
 
 #else /* !CONFIG_KEXEC */
diff -rpuN a/kernel/kexec.c b/kernel/kexec.c
--- a/kernel/kexec.c	2007-09-21 13:56:02.000000000 +0900
+++ b/kernel/kexec.c	2007-09-21 13:56:30.000000000 +0900
@@ -21,16 +21,26 @@
 #include <linux/hardirq.h>
 #include <linux/elf.h>
 #include <linux/elfcore.h>
+#include <linux/utsrelease.h>
+#include <linux/utsname.h>
+#include <linux/numa.h>
 
 #include <asm/page.h>
 #include <asm/uaccess.h>
 #include <asm/io.h>
 #include <asm/system.h>
 #include <asm/semaphore.h>
+#include <asm/sections.h>
 
 /* Per cpu memory for storing cpu states in case of system crash. */
 note_buf_t* crash_notes;
 
+/* vmcoreinfo stuff */
+unsigned char vmcoreinfo_data[VMCOREINFO_BYTES];
+u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
+size_t vmcoreinfo_size;
+size_t vmcoreinfo_max_size = sizeof(vmcoreinfo_data);
+
 /* Location of the reserved area for the crash kernel */
 struct resource crashk_res = {
 	.name  = "Crash kernel",
@@ -1060,6 +1070,7 @@ void crash_kexec(struct pt_regs *regs)
 		if (kexec_crash_image) {
 			struct pt_regs fixed_regs;
 			crash_setup_regs(&fixed_regs, regs);
+			crash_save_vmcoreinfo();
 			machine_crash_shutdown(&fixed_regs);
 			machine_kexec(kexec_crash_image);
 		}
@@ -1134,3 +1145,104 @@ static int __init crash_notes_memory_ini
 	return 0;
 }
 module_init(crash_notes_memory_init)
+
+void crash_save_vmcoreinfo(void)
+{
+	u32 *buf;
+
+	if (!vmcoreinfo_size)
+		return;
+
+	vmcoreinfo_append_str("CRASHTIME=%ld", get_seconds());
+
+	buf = (u32 *)vmcoreinfo_note;
+
+	buf = append_elf_note(buf, VMCOREINFO_NOTE_NAME, 0, vmcoreinfo_data,
+			      vmcoreinfo_size);
+
+	final_note(buf);
+}
+
+void vmcoreinfo_append_str(const char *fmt, ...)
+{
+	va_list args;
+	char buf[0x50];
+	int r;
+
+	va_start(args, fmt);
+	r = vsnprintf(buf, sizeof(buf), fmt, args);
+	va_end(args);
+
+	if (r + vmcoreinfo_size > vmcoreinfo_max_size)
+		r = vmcoreinfo_max_size - vmcoreinfo_size;
+
+	memcpy(&vmcoreinfo_data[vmcoreinfo_size], buf, r);
+
+	vmcoreinfo_size += r;
+}
+
+/*
+ * provide an empty default implementation here -- architecture
+ * code may override this
+ */
+void __attribute__ ((weak)) arch_crash_save_vmcoreinfo(void)
+{}
+
+unsigned long __attribute__ ((weak)) paddr_vmcoreinfo_note(void)
+{
+	return __pa((unsigned long)(char *)&vmcoreinfo_note);
+}
+
+static int __init crash_save_vmcoreinfo_init(void)
+{
+	VMCOREINFO_OSRELEASE(init_uts_ns.name.release);
+	VMCOREINFO_PAGESIZE(PAGE_SIZE);
+
+	VMCOREINFO_SYMBOL(init_uts_ns);
+	VMCOREINFO_SYMBOL(node_online_map);
+	VMCOREINFO_SYMBOL(swapper_pg_dir);
+	VMCOREINFO_SYMBOL(_stext);
+
+#ifndef CONFIG_NEED_MULTIPLE_NODES
+	VMCOREINFO_SYMBOL(mem_map);
+	VMCOREINFO_SYMBOL(contig_page_data);
+#endif
+#ifdef CONFIG_SPARSEMEM
+	VMCOREINFO_SYMBOL(mem_section);
+	VMCOREINFO_LENGTH(mem_section, NR_SECTION_ROOTS);
+	VMCOREINFO_STRUCT_SIZE(mem_section);
+	VMCOREINFO_OFFSET(mem_section, section_mem_map);
+#endif
+	VMCOREINFO_STRUCT_SIZE(page);
+	VMCOREINFO_STRUCT_SIZE(pglist_data);
+	VMCOREINFO_STRUCT_SIZE(zone);
+	VMCOREINFO_STRUCT_SIZE(free_area);
+	VMCOREINFO_STRUCT_SIZE(list_head);
+	VMCOREINFO_SIZE(nodemask_t);
+	VMCOREINFO_OFFSET(page, flags);
+	VMCOREINFO_OFFSET(page, _count);
+	VMCOREINFO_OFFSET(page, mapping);
+	VMCOREINFO_OFFSET(page, lru);
+	VMCOREINFO_OFFSET(pglist_data, node_zones);
+	VMCOREINFO_OFFSET(pglist_data, nr_zones);
+#ifdef CONFIG_FLAT_NODE_MEM_MAP
+	VMCOREINFO_OFFSET(pglist_data, node_mem_map);
+#endif
+	VMCOREINFO_OFFSET(pglist_data, node_start_pfn);
+	VMCOREINFO_OFFSET(pglist_data, node_spanned_pages);
+	VMCOREINFO_OFFSET(pglist_data, node_id);
+	VMCOREINFO_OFFSET(zone, free_area);
+	VMCOREINFO_OFFSET(zone, vm_stat);
+	VMCOREINFO_OFFSET(zone, spanned_pages);
+	VMCOREINFO_OFFSET(free_area, free_list);
+	VMCOREINFO_OFFSET(list_head, next);
+	VMCOREINFO_OFFSET(list_head, prev);
+	VMCOREINFO_LENGTH(zone.free_area, MAX_ORDER);
+	VMCOREINFO_NUMBER(NR_FREE_PAGES);
+
+	arch_crash_save_vmcoreinfo();
+
+	return 0;
+}
+
+module_init(crash_save_vmcoreinfo_init)
diff -rpuN a/kernel/ksysfs.c b/kernel/ksysfs.c
--- a/kernel/ksysfs.c	2007-09-21 13:56:02.000000000 +0900
+++ b/kernel/ksysfs.c	2007-09-21 15:38:38.000000000 +0900
@@ -60,6 +60,14 @@ static ssize_t kexec_crash_loaded_show(s
 	return sprintf(page, "%d\n", !!kexec_crash_image);
 }
 KERNEL_ATTR_RO(kexec_crash_loaded);
+
+static ssize_t vmcoreinfo_show(struct kset *kset, char *page)
+{
+	return sprintf(page, "%lx %zx\n",
+		       paddr_vmcoreinfo_note(), vmcoreinfo_max_size);
+}
+KERNEL_ATTR_RO(vmcoreinfo);
+
 #endif /* CONFIG_KEXEC */
 
 /*
@@ -95,6 +103,7 @@ static struct attribute * kernel_attrs[]
 #ifdef CONFIG_KEXEC
 	&kexec_loaded_attr.attr,
 	&kexec_crash_loaded_attr.attr,
+	&vmcoreinfo_attr.attr,
 #endif
 	NULL
 };
_