[PATCH 1/2] kexec: add a dummy note for each offline cpu

[PATCH 1/2] kexec: add a dummy note for each offline cpu

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

Signed-off-by: Pingfan Liu <piliu@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


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

[PATCH 2/2] [fs] proc/vmcore: check the dummy place holder for offline cpu to avoid warning

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

Signed-off-by: Pingfan Liu <piliu@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


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

Re: [PATCH 2/2] [fs] proc/vmcore: check the dummy place holder for offline cpu to avoid warning

[Xunlei Pang]

On 12/14/2016 at 02:11 PM, Pingfan Liu wrote:
> 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 <piliu@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++) {

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

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


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

Re: [PATCH 2/2] [fs] proc/vmcore: check the dummy place holder for offline cpu to avoid warning

[Liu ping fan]

On Thu, Dec 15, 2016 at 7:56 AM, Xunlei Pang <xpang@redhat.com> wrote:
> On 12/14/2016 at 02:11 PM, Pingfan Liu wrote:
>> 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 <piliu@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++) {
>
> 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.

> Also do the similar thing for update_note_header_size_elf32()?
>
Yes, will fix it.

Thx,

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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Liu ping fan]

On Wed, Dec 14, 2016 at 2:11 PM, Pingfan Liu <piliu@redhat.com> wrote:
> 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);

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

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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Baoquan He]

On 12/14/16 at 02:11pm, Pingfan Liu wrote:
> 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?

> 
> Signed-off-by: Pingfan Liu <piliu@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
> 

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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Liu ping fan]

On Wed, Dec 14, 2016 at 3:40 PM, Baoquan He <bhe@redhat.com> wrote:
> On 12/14/16 at 02:11pm, Pingfan Liu wrote:
>> 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

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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Baoquan He]

On 12/14/16 at 04:15pm, Liu ping fan wrote:
> On Wed, Dec 14, 2016 at 3:40 PM, Baoquan He <bhe@redhat.com> wrote:
> > On 12/14/16 at 02:11pm, Pingfan Liu wrote:
> >> 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?

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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Liu ping fan]

On Wed, Dec 14, 2016 at 4:25 PM, Baoquan He <bhe@redhat.com> wrote:
> On 12/14/16 at 04:15pm, Liu ping fan wrote:
>> On Wed, Dec 14, 2016 at 3:40 PM, Baoquan He <bhe@redhat.com> wrote:
>> > On 12/14/16 at 02:11pm, Pingfan Liu wrote:
>> >> 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.

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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Baoquan He]

On 12/14/16 at 04:39pm, Liu ping fan wrote:
> On Wed, Dec 14, 2016 at 4:25 PM, Baoquan He <bhe@redhat.com> wrote:
> > On 12/14/16 at 04:15pm, Liu ping fan wrote:
> >> On Wed, Dec 14, 2016 at 3:40 PM, Baoquan He <bhe@redhat.com> wrote:
> >> > On 12/14/16 at 02:11pm, Pingfan Liu wrote:
> >> >> 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


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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Liu ping fan]

[...]
>> >> >
>> >> > 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

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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Xunlei Pang]

On 12/14/2016 at 02:11 PM, Pingfan Liu wrote:
> 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

>
> Signed-off-by: Pingfan Liu <piliu@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);


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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Liu ping fan]

On Wed, Dec 14, 2016 at 4:48 PM, Xunlei Pang <xpang@redhat.com> wrote:
> On 12/14/2016 at 02:11 PM, Pingfan Liu wrote:
>> 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.

> 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

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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Xunlei Pang]

On 12/14/2016 at 04:56 PM, Liu ping fan wrote:
> On Wed, Dec 14, 2016 at 4:48 PM, Xunlei Pang <xpang@redhat.com> wrote:
>> On 12/14/2016 at 02:11 PM, Pingfan Liu wrote:
>>> 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.

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

If there is any other vmcore functional issue besides throwing "Warning: Zero PT_NOTE entries found"?

Regards,
Xunlei

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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

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

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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Xunlei Pang]

On 12/14/2016 at 05:13 PM, Liu ping fan wrote:
> [...]
>>> 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
>> 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.

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

>
> Thx,
> Pingfan
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec


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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Dave Young]

Hi, Pingfan
On 12/14/16 at 02:11pm, Pingfan Liu wrote:
> 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.

> 
> Signed-off-by: Pingfan Liu <piliu@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
> 

Thanks
Dave

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

Re: [PATCH 1/2] kexec: add a dummy note for each offline cpu

[Liu ping fan]

On Thu, Dec 15, 2016 at 3:16 PM, Dave Young <dyoung@redhat.com> wrote:
> Hi, Pingfan
> On 12/14/16 at 02:11pm, Pingfan Liu wrote:
>> 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.

> 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
>>
>> Signed-off-by: Pingfan Liu <piliu@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
>>
>
> Thanks
> Dave

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