Discussion:
[PATCH 1/2] kexec: add a dummy note for each offline cpu
Pingfan Liu
2016-12-14 06:11:26 UTC
Permalink
kexec-tools always allocates program headers for each possible cpu. This
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.

Signed-off-by: Pingfan Liu <***@redhat.com>
---
This unnecessary warning buzz on all archs when there is offline cpu

include/uapi/linux/elf.h | 1 +
kernel/kexec_core.c | 9 +++++++++
2 files changed, 10 insertions(+)

diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index b59ee07..9744f1e 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -367,6 +367,7 @@ typedef struct elf64_shdr {
* using the corresponding note types via the PTRACE_GETREGSET and
* PTRACE_SETREGSET requests.
*/
+#define NT_DUMMY 0
#define NT_PRSTATUS 1
#define NT_PRFPREG 2
#define NT_PRPSINFO 3
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 5616755..aeac16e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -891,9 +891,12 @@ void __crash_kexec(struct pt_regs *regs)
if (mutex_trylock(&kexec_mutex)) {
if (kexec_crash_image) {
struct pt_regs fixed_regs;
+ unsigned int cpu;

crash_setup_regs(&fixed_regs, regs);
crash_save_vmcoreinfo();
+ for_each_cpu_not(cpu, cpu_online_mask)
+ crash_save_cpu(NULL, cpu);
machine_crash_shutdown(&fixed_regs);
machine_kexec(kexec_crash_image);
}
@@ -1040,6 +1043,12 @@ void crash_save_cpu(struct pt_regs *regs, int cpu)
buf = (u32 *)per_cpu_ptr(crash_notes, cpu);
if (!buf)
return;
+ if (regs == NULL) {
+ buf = append_elf_note(buf, KEXEC_CORE_NOTE_NAME, NT_DUMMY,
+ NULL, 0);
+ final_note(buf);
+ return;
+ }
memset(&prstatus, 0, sizeof(prstatus));
prstatus.pr_pid = current->pid;
elf_core_copy_kernel_regs(&prstatus.pr_reg, regs);
--
2.7.4
Pingfan Liu
2016-12-14 06:11:27 UTC
Permalink
kexec-tools always allocates program headers for possible cpus. But
when crashing, offline cpus have dummy headers. We do not copy these
dummy notes into ELF file, also have no need of warning on them.

Signed-off-by: Pingfan Liu <***@redhat.com>
---
fs/proc/vmcore.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 8ab782d..bbc9dad 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -526,9 +526,10 @@ static u64 __init get_vmcore_size(size_t elfsz, size_t elfnotesegsz,
*/
static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
{
- int i, rc=0;
+ int i, j, rc = 0;
Elf64_Phdr *phdr_ptr;
Elf64_Nhdr *nhdr_ptr;
+ bool warn;

phdr_ptr = (Elf64_Phdr *)(ehdr_ptr + 1);
for (i = 0; i < ehdr_ptr->e_phnum; i++, phdr_ptr++) {
@@ -536,6 +537,7 @@ static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
u64 offset, max_sz, sz, real_sz = 0;
if (phdr_ptr->p_type != PT_NOTE)
continue;
+ warn = true;
max_sz = phdr_ptr->p_memsz;
offset = phdr_ptr->p_offset;
notes_section = kmalloc(max_sz, GFP_KERNEL);
@@ -547,7 +549,7 @@ static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
return rc;
}
nhdr_ptr = notes_section;
- while (nhdr_ptr->n_namesz != 0) {
+ for (j = 0; nhdr_ptr->n_namesz != 0; j++) {
sz = sizeof(Elf64_Nhdr) +
(((u64)nhdr_ptr->n_namesz + 3) & ~3) +
(((u64)nhdr_ptr->n_descsz + 3) & ~3);
@@ -559,11 +561,22 @@ static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
real_sz += sz;
nhdr_ptr = (Elf64_Nhdr*)((char*)nhdr_ptr + sz);
}
+ if (real_sz != 0)
+ warn = false;
+ if (j == 1) {
+ nhdr_ptr = notes_section;
+ if ((nhdr_ptr->n_type == NT_DUMMY)
+ && !strncmp(KEXEC_CORE_NOTE_NAME,
+ (char *)nhdr_ptr + sizeof(Elf64_Nhdr),
+ strlen(KEXEC_CORE_NOTE_NAME))) {
+ /* do not copy this dummy note */
+ real_sz = 0;
+ }
+ }
kfree(notes_section);
phdr_ptr->p_memsz = real_sz;
- if (real_sz == 0) {
+ if (warn)
pr_warn("Warning: Zero PT_NOTE entries found\n");
- }
}

return 0;
--
2.7.4
Xunlei Pang
2016-12-14 23:56:48 UTC
Permalink
Post by Pingfan Liu
kexec-tools always allocates program headers for possible cpus. But
when crashing, offline cpus have dummy headers. We do not copy these
dummy notes into ELF file, also have no need of warning on them.
---
fs/proc/vmcore.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 8ab782d..bbc9dad 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -526,9 +526,10 @@ static u64 __init get_vmcore_size(size_t elfsz, size_t elfnotesegsz,
*/
static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
{
- int i, rc=0;
+ int i, j, rc = 0;
Elf64_Phdr *phdr_ptr;
Elf64_Nhdr *nhdr_ptr;
+ bool warn;
phdr_ptr = (Elf64_Phdr *)(ehdr_ptr + 1);
for (i = 0; i < ehdr_ptr->e_phnum; i++, phdr_ptr++) {
@@ -536,6 +537,7 @@ static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
u64 offset, max_sz, sz, real_sz = 0;
if (phdr_ptr->p_type != PT_NOTE)
continue;
+ warn = true;
max_sz = phdr_ptr->p_memsz;
offset = phdr_ptr->p_offset;
notes_section = kmalloc(max_sz, GFP_KERNEL);
@@ -547,7 +549,7 @@ static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
return rc;
}
nhdr_ptr = notes_section;
- while (nhdr_ptr->n_namesz != 0) {
+ for (j = 0; nhdr_ptr->n_namesz != 0; j++) {
Hi Pingfan,

I think we don't need to be this complex, how about simply check before while loop,
if it is the cpu dummy note(initialize it with some magic), then handle it differently,
e.g. set a "nowarn" flag to use afterwards and make sure it has zero p_memsz?

Also do the similar thing for update_note_header_size_elf32()?

Regards,
Xunlei
Post by Pingfan Liu
sz = sizeof(Elf64_Nhdr) +
(((u64)nhdr_ptr->n_namesz + 3) & ~3) +
(((u64)nhdr_ptr->n_descsz + 3) & ~3);
@@ -559,11 +561,22 @@ static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
real_sz += sz;
nhdr_ptr = (Elf64_Nhdr*)((char*)nhdr_ptr + sz);
}
+ if (real_sz != 0)
+ warn = false;
+ if (j == 1) {
+ nhdr_ptr = notes_section;
+ if ((nhdr_ptr->n_type == NT_DUMMY)
+ && !strncmp(KEXEC_CORE_NOTE_NAME,
+ (char *)nhdr_ptr + sizeof(Elf64_Nhdr),
+ strlen(KEXEC_CORE_NOTE_NAME))) {
+ /* do not copy this dummy note */
+ real_sz = 0;
+ }
+ }
kfree(notes_section);
phdr_ptr->p_memsz = real_sz;
- if (real_sz == 0) {
+ if (warn)
pr_warn("Warning: Zero PT_NOTE entries found\n");
- }
}
return 0;
Liu ping fan
2016-12-15 03:04:25 UTC
Permalink
Post by Xunlei Pang
Post by Pingfan Liu
kexec-tools always allocates program headers for possible cpus. But
when crashing, offline cpus have dummy headers. We do not copy these
dummy notes into ELF file, also have no need of warning on them.
---
fs/proc/vmcore.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 8ab782d..bbc9dad 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -526,9 +526,10 @@ static u64 __init get_vmcore_size(size_t elfsz, size_t elfnotesegsz,
*/
static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
{
- int i, rc=0;
+ int i, j, rc = 0;
Elf64_Phdr *phdr_ptr;
Elf64_Nhdr *nhdr_ptr;
+ bool warn;
phdr_ptr = (Elf64_Phdr *)(ehdr_ptr + 1);
for (i = 0; i < ehdr_ptr->e_phnum; i++, phdr_ptr++) {
@@ -536,6 +537,7 @@ static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
u64 offset, max_sz, sz, real_sz = 0;
if (phdr_ptr->p_type != PT_NOTE)
continue;
+ warn = true;
max_sz = phdr_ptr->p_memsz;
offset = phdr_ptr->p_offset;
notes_section = kmalloc(max_sz, GFP_KERNEL);
@@ -547,7 +549,7 @@ static int __init update_note_header_size_elf64(const Elf64_Ehdr *ehdr_ptr)
return rc;
}
nhdr_ptr = notes_section;
- while (nhdr_ptr->n_namesz != 0) {
+ for (j = 0; nhdr_ptr->n_namesz != 0; j++) {
Hi Pingfan,
I think we don't need to be this complex, how about simply check before while loop,
if it is the cpu dummy note(initialize it with some magic), then handle it differently,
e.g. set a "nowarn" flag to use afterwards and make sure it has zero p_memsz?
I had thought that how the percpu note section was filled. But you are
right, we can suppose that for all archs, cpus just overwrite the
note, not append the note.
Post by Xunlei Pang
Also do the similar thing for update_note_header_size_elf32()?
Yes, will fix it.

Thx,
Liu ping fan
2016-12-14 06:19:09 UTC
Permalink
Post by Pingfan Liu
kexec-tools always allocates program headers for each possible cpu. This
The code is in the file: kexec-tools/kexec/crashdump-elf.c
nr_cpus = sysconf(_SC_NPROCESSORS_CONF);
Post by Pingfan Liu
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.
---
This unnecessary warning buzz on all archs when there is offline cpu
include/uapi/linux/elf.h | 1 +
kernel/kexec_core.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index b59ee07..9744f1e 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -367,6 +367,7 @@ typedef struct elf64_shdr {
* using the corresponding note types via the PTRACE_GETREGSET and
* PTRACE_SETREGSET requests.
*/
+#define NT_DUMMY 0
#define NT_PRSTATUS 1
#define NT_PRFPREG 2
#define NT_PRPSINFO 3
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 5616755..aeac16e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -891,9 +891,12 @@ void __crash_kexec(struct pt_regs *regs)
if (mutex_trylock(&kexec_mutex)) {
if (kexec_crash_image) {
struct pt_regs fixed_regs;
+ unsigned int cpu;
crash_setup_regs(&fixed_regs, regs);
crash_save_vmcoreinfo();
+ for_each_cpu_not(cpu, cpu_online_mask)
+ crash_save_cpu(NULL, cpu);
machine_crash_shutdown(&fixed_regs);
machine_kexec(kexec_crash_image);
}
@@ -1040,6 +1043,12 @@ void crash_save_cpu(struct pt_regs *regs, int cpu)
buf = (u32 *)per_cpu_ptr(crash_notes, cpu);
if (!buf)
return;
+ if (regs == NULL) {
+ buf = append_elf_note(buf, KEXEC_CORE_NOTE_NAME, NT_DUMMY,
+ NULL, 0);
+ final_note(buf);
+ return;
+ }
memset(&prstatus, 0, sizeof(prstatus));
prstatus.pr_pid = current->pid;
elf_core_copy_kernel_regs(&prstatus.pr_reg, regs);
--
2.7.4
Baoquan He
2016-12-14 07:40:46 UTC
Permalink
Post by Pingfan Liu
kexec-tools always allocates program headers for each possible cpu. This
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.
When you execute dmesg on your testing machine and grep nr_cpu_ids,
what's the value of nr_cpu_ids?
Post by Pingfan Liu
---
This unnecessary warning buzz on all archs when there is offline cpu
include/uapi/linux/elf.h | 1 +
kernel/kexec_core.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index b59ee07..9744f1e 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -367,6 +367,7 @@ typedef struct elf64_shdr {
* using the corresponding note types via the PTRACE_GETREGSET and
* PTRACE_SETREGSET requests.
*/
+#define NT_DUMMY 0
#define NT_PRSTATUS 1
#define NT_PRFPREG 2
#define NT_PRPSINFO 3
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 5616755..aeac16e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -891,9 +891,12 @@ void __crash_kexec(struct pt_regs *regs)
if (mutex_trylock(&kexec_mutex)) {
if (kexec_crash_image) {
struct pt_regs fixed_regs;
+ unsigned int cpu;
crash_setup_regs(&fixed_regs, regs);
crash_save_vmcoreinfo();
+ for_each_cpu_not(cpu, cpu_online_mask)
+ crash_save_cpu(NULL, cpu);
machine_crash_shutdown(&fixed_regs);
machine_kexec(kexec_crash_image);
}
@@ -1040,6 +1043,12 @@ void crash_save_cpu(struct pt_regs *regs, int cpu)
buf = (u32 *)per_cpu_ptr(crash_notes, cpu);
if (!buf)
return;
+ if (regs == NULL) {
+ buf = append_elf_note(buf, KEXEC_CORE_NOTE_NAME, NT_DUMMY,
+ NULL, 0);
+ final_note(buf);
+ return;
+ }
memset(&prstatus, 0, sizeof(prstatus));
prstatus.pr_pid = current->pid;
elf_core_copy_kernel_regs(&prstatus.pr_reg, regs);
--
2.7.4
Liu ping fan
2016-12-14 08:15:29 UTC
Permalink
Post by Baoquan He
Post by Pingfan Liu
kexec-tools always allocates program headers for each possible cpu. This
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.
When you execute dmesg on your testing machine and grep nr_cpu_ids,
what's the value of nr_cpu_ids?
nr_cpu_ids=128
Baoquan He
2016-12-14 08:25:01 UTC
Permalink
Post by Liu ping fan
Post by Baoquan He
Post by Pingfan Liu
kexec-tools always allocates program headers for each possible cpu. This
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.
When you execute dmesg on your testing machine and grep nr_cpu_ids,
what's the value of nr_cpu_ids?
nr_cpu_ids=128
And what's the cpu number of in "lscpu" command?
Liu ping fan
2016-12-14 08:39:00 UTC
Permalink
Post by Baoquan He
Post by Liu ping fan
Post by Baoquan He
Post by Pingfan Liu
kexec-tools always allocates program headers for each possible cpu. This
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.
When you execute dmesg on your testing machine and grep nr_cpu_ids,
what's the value of nr_cpu_ids?
nr_cpu_ids=128
And what's the cpu number of in "lscpu" command?
NUMA node1 CPU(s): 0-7

The system booted up with 128 possible cpu and only 8 online.
Also I tested on x86 guest, after bootup with 8 cpus, then offline 4
of them, the zero PT_NOTE warning buzz too.
Baoquan He
2016-12-14 08:44:02 UTC
Permalink
Post by Liu ping fan
Post by Baoquan He
Post by Liu ping fan
Post by Baoquan He
Post by Pingfan Liu
kexec-tools always allocates program headers for each possible cpu. This
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.
When you execute dmesg on your testing machine and grep nr_cpu_ids,
what's the value of nr_cpu_ids?
nr_cpu_ids=128
And what's the cpu number of in "lscpu" command?
NUMA node1 CPU(s): 0-7
The system booted up with 128 possible cpu and only 8 online.
Also I tested on x86 guest, after bootup with 8 cpus, then offline 4
of them, the zero PT_NOTE warning buzz too.
Yes, this is what I think not quite appropriate using
for_each_cpu_not(cpu, cpu_online_mask). Maybe it need try to save on
those cpus which is present but not online. not online seems not good,
it's not reasonable to save those getting apic but no cpu plugged.

Thanks
Baoquan
Liu ping fan
2016-12-14 09:06:26 UTC
Permalink
[...]
Post by Baoquan He
Post by Liu ping fan
Post by Baoquan He
Post by Liu ping fan
Post by Baoquan He
When you execute dmesg on your testing machine and grep nr_cpu_ids,
what's the value of nr_cpu_ids?
nr_cpu_ids=128
And what's the cpu number of in "lscpu" command?
NUMA node1 CPU(s): 0-7
The system booted up with 128 possible cpu and only 8 online.
Also I tested on x86 guest, after bootup with 8 cpus, then offline 4
of them, the zero PT_NOTE warning buzz too.
Yes, this is what I think not quite appropriate using
for_each_cpu_not(cpu, cpu_online_mask). Maybe it need try to save on
those cpus which is present but not online. not online seems not good,
it's not reasonable to save those getting apic but no cpu plugged.
In the file: kexec-tools/kexec/crashdump-elf.c
nr_cpus = sysconf(_SC_NPROCESSORS_CONF);
And this is why this patch needs to make a mark on these offline cpu.
This is no something like "_SC_NPROCESSORS_PRESENT" option, so just
work around it in kernel side.
Anyway for crash kernel, we only write "core" in percpu notes without
no more info, and it costs nothing when capture kernel gather the
PT_NOTE.

Thx,
Pingfan
Xunlei Pang
2016-12-14 08:48:25 UTC
Permalink
Post by Pingfan Liu
kexec-tools always allocates program headers for each possible cpu. This
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.
Hmm, we can initialize the cpu crash note buf in crash_notes_memory_init(), needless
to do it at the crash moment, right?

BTW, does this cause any issue, for example the crash utility can't parse the vmcore
properly? or just reproduce lots of warnings after offline multiple cpus?

Regards,
Xunlei
Post by Pingfan Liu
---
This unnecessary warning buzz on all archs when there is offline cpu
include/uapi/linux/elf.h | 1 +
kernel/kexec_core.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index b59ee07..9744f1e 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -367,6 +367,7 @@ typedef struct elf64_shdr {
* using the corresponding note types via the PTRACE_GETREGSET and
* PTRACE_SETREGSET requests.
*/
+#define NT_DUMMY 0
#define NT_PRSTATUS 1
#define NT_PRFPREG 2
#define NT_PRPSINFO 3
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 5616755..aeac16e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -891,9 +891,12 @@ void __crash_kexec(struct pt_regs *regs)
if (mutex_trylock(&kexec_mutex)) {
if (kexec_crash_image) {
struct pt_regs fixed_regs;
+ unsigned int cpu;
crash_setup_regs(&fixed_regs, regs);
crash_save_vmcoreinfo();
+ for_each_cpu_not(cpu, cpu_online_mask)
+ crash_save_cpu(NULL, cpu);
machine_crash_shutdown(&fixed_regs);
machine_kexec(kexec_crash_image);
}
@@ -1040,6 +1043,12 @@ void crash_save_cpu(struct pt_regs *regs, int cpu)
buf = (u32 *)per_cpu_ptr(crash_notes, cpu);
if (!buf)
return;
+ if (regs == NULL) {
+ buf = append_elf_note(buf, KEXEC_CORE_NOTE_NAME, NT_DUMMY,
+ NULL, 0);
+ final_note(buf);
+ return;
+ }
memset(&prstatus, 0, sizeof(prstatus));
prstatus.pr_pid = current->pid;
elf_core_copy_kernel_regs(&prstatus.pr_reg, regs);
Liu ping fan
2016-12-14 08:56:14 UTC
Permalink
Post by Xunlei Pang
Post by Pingfan Liu
kexec-tools always allocates program headers for each possible cpu. This
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.
Hmm, we can initialize the cpu crash note buf in crash_notes_memory_init(), needless
to do it at the crash moment, right?
The cpus can be on-off-on.., We can not know the user's action.
Post by Xunlei Pang
BTW, does this cause any issue, for example the crash utility can't parse the vmcore
properly? or just reproduce lots of warnings after offline multiple cpus?
No. This patch just place a mark on these offline cpu. The next patch
for capture kernel will recognize this case, and ignore this kind of
pt_note by the code:
real_sz = 0; // although the size of this kind of PT_NOTE is not zero,
but it contains nothing useful, so just ignore it
phdr_ptr->p_memsz = real_sz

Thx,
Pingfan
Xunlei Pang
2016-12-14 09:10:31 UTC
Permalink
Post by Liu ping fan
Post by Xunlei Pang
Post by Pingfan Liu
kexec-tools always allocates program headers for each possible cpu. This
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.
Hmm, we can initialize the cpu crash note buf in crash_notes_memory_init(), needless
to do it at the crash moment, right?
The cpus can be on-off-on.., We can not know the user's action.
I meant we can add the fake note into the cpu note buf, then the crash happens, the online ones
will be overwritten with the real note data, while others(!online) will still be the fake note.
Post by Liu ping fan
Post by Xunlei Pang
BTW, does this cause any issue, for example the crash utility can't parse the vmcore
properly? or just reproduce lots of warnings after offline multiple cpus?
No. This patch just place a mark on these offline cpu. The next patch
for capture kernel will recognize this case, and ignore this kind of
real_sz = 0; // although the size of this kind of PT_NOTE is not zero,
but it contains nothing useful, so just ignore it
phdr_ptr->p_memsz = real_sz
If there is any other vmcore functional issue besides throwing "Warning: Zero PT_NOTE entries found"?

Regards,
Xunlei
Liu ping fan
2016-12-14 09:13:06 UTC
Permalink
[...]
Post by Xunlei Pang
Post by Liu ping fan
No. This patch just place a mark on these offline cpu. The next patch
for capture kernel will recognize this case, and ignore this kind of
real_sz = 0; // although the size of this kind of PT_NOTE is not zero,
but it contains nothing useful, so just ignore it
phdr_ptr->p_memsz = real_sz
If there is any other vmcore functional issue besides throwing "Warning: Zero PT_NOTE entries found"?
Not at present when I debugged.
I just think we can not suppose the behaviour of different archs, so
just mark out the dummy pt_note. If some archs want to use these notes
memory,
they will just overwrite the dummy.

Thx,
Pingfan
Xunlei Pang
2016-12-14 23:49:15 UTC
Permalink
Post by Liu ping fan
[...]
Post by Xunlei Pang
Post by Liu ping fan
No. This patch just place a mark on these offline cpu. The next patch
for capture kernel will recognize this case, and ignore this kind of
real_sz = 0; // although the size of this kind of PT_NOTE is not zero,
but it contains nothing useful, so just ignore it
phdr_ptr->p_memsz = real_sz
If there is any other vmcore functional issue besides throwing "Warning: Zero PT_NOTE entries found"?
Not at present when I debugged.
Well, agree that we should fix it given that it produces many unnecessary warnings on some machines.
Post by Liu ping fan
I just think we can not suppose the behaviour of different archs, so
just mark out the dummy pt_note. If some archs want to use these notes
memory,
they will just overwrite the dummy.
For cpu crash_notes, it should be arch-independent, and related to elf format.
Post by Liu ping fan
Thx,
Pingfan
_______________________________________________
kexec mailing list
http://lists.infradead.org/mailman/listinfo/kexec
Dave Young
2016-12-15 07:16:46 UTC
Permalink
Hi, Pingfan
Post by Pingfan Liu
kexec-tools always allocates program headers for each possible cpu. This
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.
I thought you saw the warnings on ppc64 and it might be a ppc64 issue.
But if this is instead a general issue, can we think about if this is
really necessary?

Does it have any side effect other than the warning messages? If there
is nothing bad other than the warnings maybe leave it as is will be
a better way.
Post by Pingfan Liu
---
This unnecessary warning buzz on all archs when there is offline cpu
include/uapi/linux/elf.h | 1 +
kernel/kexec_core.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index b59ee07..9744f1e 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -367,6 +367,7 @@ typedef struct elf64_shdr {
* using the corresponding note types via the PTRACE_GETREGSET and
* PTRACE_SETREGSET requests.
*/
+#define NT_DUMMY 0
#define NT_PRSTATUS 1
#define NT_PRFPREG 2
#define NT_PRPSINFO 3
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 5616755..aeac16e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -891,9 +891,12 @@ void __crash_kexec(struct pt_regs *regs)
if (mutex_trylock(&kexec_mutex)) {
if (kexec_crash_image) {
struct pt_regs fixed_regs;
+ unsigned int cpu;
crash_setup_regs(&fixed_regs, regs);
crash_save_vmcoreinfo();
+ for_each_cpu_not(cpu, cpu_online_mask)
+ crash_save_cpu(NULL, cpu);
machine_crash_shutdown(&fixed_regs);
machine_kexec(kexec_crash_image);
}
@@ -1040,6 +1043,12 @@ void crash_save_cpu(struct pt_regs *regs, int cpu)
buf = (u32 *)per_cpu_ptr(crash_notes, cpu);
if (!buf)
return;
+ if (regs == NULL) {
+ buf = append_elf_note(buf, KEXEC_CORE_NOTE_NAME, NT_DUMMY,
+ NULL, 0);
+ final_note(buf);
+ return;
+ }
memset(&prstatus, 0, sizeof(prstatus));
prstatus.pr_pid = current->pid;
elf_core_copy_kernel_regs(&prstatus.pr_reg, regs);
--
2.7.4
Thanks
Dave
Liu ping fan
2016-12-15 08:43:44 UTC
Permalink
Post by Dave Young
Hi, Pingfan
Post by Pingfan Liu
kexec-tools always allocates program headers for each possible cpu. This
incurs zero PT_NOTE for offline cpu. We mark this case so that later,
the capture kernel can distinguish it from the mistake of allocated
program header.
The counterpart of the capture kernel comes in next patch.
I thought you saw the warnings on ppc64 and it might be a ppc64 issue.
It affects all archs, since kexec-tools creates each program header
for each _present_ cpu. So if the cpu is offline, then the warning
buzzes.
Post by Dave Young
But if this is instead a general issue, can we think about if this is
really necessary?
Does it have any side effect other than the warning messages? If there
is nothing bad other than the warnings maybe leave it as is will be
a better way.
I think kernel warning always made people upset who do not know the
back ground of the related field.
While on the other hand, the program behaves correctly against our
expectation, so just suppress the warning to make everyone easy.

Regards,
Pingfan
Post by Dave Young
Post by Pingfan Liu
---
This unnecessary warning buzz on all archs when there is offline cpu
include/uapi/linux/elf.h | 1 +
kernel/kexec_core.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index b59ee07..9744f1e 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -367,6 +367,7 @@ typedef struct elf64_shdr {
* using the corresponding note types via the PTRACE_GETREGSET and
* PTRACE_SETREGSET requests.
*/
+#define NT_DUMMY 0
#define NT_PRSTATUS 1
#define NT_PRFPREG 2
#define NT_PRPSINFO 3
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 5616755..aeac16e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -891,9 +891,12 @@ void __crash_kexec(struct pt_regs *regs)
if (mutex_trylock(&kexec_mutex)) {
if (kexec_crash_image) {
struct pt_regs fixed_regs;
+ unsigned int cpu;
crash_setup_regs(&fixed_regs, regs);
crash_save_vmcoreinfo();
+ for_each_cpu_not(cpu, cpu_online_mask)
+ crash_save_cpu(NULL, cpu);
machine_crash_shutdown(&fixed_regs);
machine_kexec(kexec_crash_image);
}
@@ -1040,6 +1043,12 @@ void crash_save_cpu(struct pt_regs *regs, int cpu)
buf = (u32 *)per_cpu_ptr(crash_notes, cpu);
if (!buf)
return;
+ if (regs == NULL) {
+ buf = append_elf_note(buf, KEXEC_CORE_NOTE_NAME, NT_DUMMY,
+ NULL, 0);
+ final_note(buf);
+ return;
+ }
memset(&prstatus, 0, sizeof(prstatus));
prstatus.pr_pid = current->pid;
elf_core_copy_kernel_regs(&prstatus.pr_reg, regs);
--
2.7.4
Thanks
Dave
Loading...