2007-09-14 02:49:38

by Ken'ichi Ohmichi

[permalink] [raw]
Subject: [PATCH 0/4] [-mm patch] Cleanup add-vmcoreinfo.patch v2


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


2007-09-14 02:54:18

by Ken'ichi Ohmichi

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


[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 <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>

---
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);

_

2007-09-14 02:56:37

by Ken'ichi Ohmichi

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


[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 <[email protected]>

---
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();

_

2007-09-14 02:58:18

by Ken'ichi Ohmichi

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


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


Thanks
Ken'ichi Ohmichi

---
Signed-off-by: Ken'ichi Ohmichi <[email protected]>

---
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);
}

_

2007-09-14 03:00:34

by Ken'ichi Ohmichi

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


[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 <[email protected]>

---
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();

_

2007-09-14 03:37:25

by David Rientjes

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

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.

2007-09-15 02:04:25

by Andrew Morton

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

On Fri, 14 Sep 2007 12:00:18 +0900 "Ken'ichi Ohmichi" <[email protected]> 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.

2007-09-20 05:17:15

by Ken'ichi Ohmichi

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


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

2007-09-20 05:29:17

by Ken'ichi Ohmichi

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


Hi Andrew,

Andrew Morton wrote:
> > On Fri, 14 Sep 2007 12:00:18 +0900 "Ken'ichi Ohmichi" <[email protected]> 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

2007-09-20 05:31:33

by Ken'ichi Ohmichi

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


[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 <[email protected]>
Signed-off-by: Ken'ichi Ohmichi <[email protected]>

---
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);
_

2007-09-20 05:33:11

by Ken'ichi Ohmichi

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


[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 <[email protected]>
Signed-off-by: Ken'ichi Ohmichi <[email protected]>
---
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) \
_

2007-09-20 12:19:16

by Satyam Sharma

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

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 <[email protected]>

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 <[email protected]>

> --- 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

2007-09-20 12:26:18

by Satyam Sharma

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



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 <[email protected]>

Same deal here ...


> Signed-off-by: Ken'ichi Ohmichi <[email protected]>
> ---
> 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

2007-09-20 18:41:58

by David Rientjes

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

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 <[email protected]>
>
> 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 <[email protected]>

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

2007-09-25 08:50:25

by Ken'ichi Ohmichi

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


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 <[email protected]>
> >
> > 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 <[email protected]>
> >
>> >> --- 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 <[email protected]>
Acked-by: David Rientjes <[email protected]>

---
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);
_

2007-09-25 08:51:38

by Ken'ichi Ohmichi

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


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 <[email protected]>

---
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);
_

2007-09-25 08:59:44

by Ken'ichi Ohmichi

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

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 <[email protected]>
Signed-off-by: Ken'ichi Ohmichi <[email protected]>
Signed-off-by: Bernhard Walle <[email protected]>
Signed-off-by: Daisuke Nishimura <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>

---
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
};
_