linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition
@ 2017-06-09  2:17 Xunlei Pang
  2017-06-09  2:29 ` Dave Young
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Xunlei Pang @ 2017-06-09  2:17 UTC (permalink / raw)
  To: linux-kernel, kexec
  Cc: akpm, Eric Biederman, Dave Young, Baoquan He, Michael Holzheu,
	linux-s390, Xunlei Pang, Dave Anderson, Hari Bathini,
	Gustavo Luiz Duarte

S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
is now defined as follows:
    typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
It was changed by the CONFIG_CRASH_CORE feature.

This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.

Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
Cc: Dave Young <dyoung@redhat.com>
Cc: Dave Anderson <anderson@redhat.com>
Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
Cc: Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>
Signed-off-by: Xunlei Pang <xlpang@redhat.com>
---
 arch/s390/include/asm/kexec.h |  2 +-
 include/linux/crash_core.h    |  7 +++++++
 include/linux/kexec.h         | 11 +----------
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index 2f924bc..352deb8 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -47,7 +47,7 @@
  * Seven notes plus zero note at the end: prstatus, fpregset, timer,
  * tod_cmp, tod_reg, control regs, and prefix
  */
-#define KEXEC_NOTE_BYTES \
+#define CRASH_CORE_NOTE_BYTES \
 	(ALIGN(sizeof(struct elf_note), 4) * 8 + \
 	 ALIGN(sizeof("CORE"), 4) * 7 + \
 	 ALIGN(sizeof(struct elf_prstatus), 4) + \
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index e9de6b4..dbc6e5c 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -10,9 +10,16 @@
 #define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
 #define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
 
+/*
+ * The per-cpu notes area is a list of notes terminated by a "NULL"
+ * note header.  For kdump, the code in vmcore.c runs in the context
+ * of the second kernel to combine them into one note.
+ */
+#ifndef CRASH_CORE_NOTE_BYTES
 #define CRASH_CORE_NOTE_BYTES	   ((CRASH_CORE_NOTE_HEAD_BYTES * 2) +	\
 				     CRASH_CORE_NOTE_NAME_BYTES +	\
 				     CRASH_CORE_NOTE_DESC_BYTES)
+#endif
 
 #define VMCOREINFO_BYTES	   PAGE_SIZE
 #define VMCOREINFO_NOTE_NAME	   "VMCOREINFO"
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 3ea8275..133df03 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -14,7 +14,6 @@
 
 #if !defined(__ASSEMBLY__)
 
-#include <linux/crash_core.h>
 #include <asm/io.h>
 
 #include <uapi/linux/kexec.h>
@@ -25,6 +24,7 @@
 #include <linux/ioport.h>
 #include <linux/module.h>
 #include <asm/kexec.h>
+#include <linux/crash_core.h>
 
 /* Verify architecture specific macros are defined */
 
@@ -63,15 +63,6 @@
 #define KEXEC_CORE_NOTE_NAME	CRASH_CORE_NOTE_NAME
 
 /*
- * The per-cpu notes area is a list of notes terminated by a "NULL"
- * note header.  For kdump, the code in vmcore.c runs in the context
- * of the second kernel to combine them into one note.
- */
-#ifndef KEXEC_NOTE_BYTES
-#define KEXEC_NOTE_BYTES	CRASH_CORE_NOTE_BYTES
-#endif
-
-/*
  * This structure is used to hold the arguments that are used when loading
  * kernel binaries.
  */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition
  2017-06-09  2:17 [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition Xunlei Pang
@ 2017-06-09  2:29 ` Dave Young
  2017-06-09  7:45   ` Dave Young
  2017-06-09  4:12 ` Hari Bathini
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Dave Young @ 2017-06-09  2:29 UTC (permalink / raw)
  To: Xunlei Pang
  Cc: linux-kernel, kexec, akpm, Eric Biederman, Baoquan He,
	Michael Holzheu, linux-s390, Dave Anderson, Hari Bathini,
	Gustavo Luiz Duarte

On 06/09/17 at 10:17am, Xunlei Pang wrote:
> S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
> is now defined as follows:
>     typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
> It was changed by the CONFIG_CRASH_CORE feature.
> 
> This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
> renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
> 
> Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
> Cc: Dave Young <dyoung@redhat.com>
> Cc: Dave Anderson <anderson@redhat.com>
> Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
> Cc: Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>
> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> ---
>  arch/s390/include/asm/kexec.h |  2 +-
>  include/linux/crash_core.h    |  7 +++++++
>  include/linux/kexec.h         | 11 +----------
>  3 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
> index 2f924bc..352deb8 100644
> --- a/arch/s390/include/asm/kexec.h
> +++ b/arch/s390/include/asm/kexec.h
> @@ -47,7 +47,7 @@
>   * Seven notes plus zero note at the end: prstatus, fpregset, timer,
>   * tod_cmp, tod_reg, control regs, and prefix
>   */
> -#define KEXEC_NOTE_BYTES \
> +#define CRASH_CORE_NOTE_BYTES \
>  	(ALIGN(sizeof(struct elf_note), 4) * 8 + \
>  	 ALIGN(sizeof("CORE"), 4) * 7 + \
>  	 ALIGN(sizeof(struct elf_prstatus), 4) + \
> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index e9de6b4..dbc6e5c 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -10,9 +10,16 @@
>  #define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
>  #define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
>  
> +/*
> + * The per-cpu notes area is a list of notes terminated by a "NULL"
> + * note header.  For kdump, the code in vmcore.c runs in the context
> + * of the second kernel to combine them into one note.
> + */
> +#ifndef CRASH_CORE_NOTE_BYTES
>  #define CRASH_CORE_NOTE_BYTES	   ((CRASH_CORE_NOTE_HEAD_BYTES * 2) +	\
>  				     CRASH_CORE_NOTE_NAME_BYTES +	\
>  				     CRASH_CORE_NOTE_DESC_BYTES)
> +#endif
>  
>  #define VMCOREINFO_BYTES	   PAGE_SIZE
>  #define VMCOREINFO_NOTE_NAME	   "VMCOREINFO"
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 3ea8275..133df03 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -14,7 +14,6 @@
>  
>  #if !defined(__ASSEMBLY__)
>  
> -#include <linux/crash_core.h>
>  #include <asm/io.h>
>  
>  #include <uapi/linux/kexec.h>
> @@ -25,6 +24,7 @@
>  #include <linux/ioport.h>
>  #include <linux/module.h>
>  #include <asm/kexec.h>
> +#include <linux/crash_core.h>
>  
>  /* Verify architecture specific macros are defined */
>  
> @@ -63,15 +63,6 @@
>  #define KEXEC_CORE_NOTE_NAME	CRASH_CORE_NOTE_NAME
>  
>  /*
> - * The per-cpu notes area is a list of notes terminated by a "NULL"
> - * note header.  For kdump, the code in vmcore.c runs in the context
> - * of the second kernel to combine them into one note.
> - */
> -#ifndef KEXEC_NOTE_BYTES
> -#define KEXEC_NOTE_BYTES	CRASH_CORE_NOTE_BYTES
> -#endif

It is still not clear how does s390 use the crash_notes except this macro.
But from code point of view we do need to update this as well after the
crash_core splitting.

Acked-by: Dave Young <dyoung@redhat.com>

Thanks
Dave

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition
  2017-06-09  2:17 [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition Xunlei Pang
  2017-06-09  2:29 ` Dave Young
@ 2017-06-09  4:12 ` Hari Bathini
  2017-06-11 19:46 ` kbuild test robot
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Hari Bathini @ 2017-06-09  4:12 UTC (permalink / raw)
  To: Xunlei Pang, linux-kernel, kexec
  Cc: linux-s390, Baoquan He, Gustavo Luiz Duarte, Dave Anderson,
	Eric Biederman, akpm, Michael Holzheu, Dave Young



On Friday 09 June 2017 07:47 AM, Xunlei Pang wrote:
> S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
> is now defined as follows:
>      typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
> It was changed by the CONFIG_CRASH_CORE feature.
>
> This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
> renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
>
> Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
> Cc: Dave Young <dyoung@redhat.com>
> Cc: Dave Anderson <anderson@redhat.com>
> Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
> Cc: Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>
> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> ---
>   arch/s390/include/asm/kexec.h |  2 +-
>   include/linux/crash_core.h    |  7 +++++++
>   include/linux/kexec.h         | 11 +----------
>   3 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
> index 2f924bc..352deb8 100644
> --- a/arch/s390/include/asm/kexec.h
> +++ b/arch/s390/include/asm/kexec.h
> @@ -47,7 +47,7 @@
>    * Seven notes plus zero note at the end: prstatus, fpregset, timer,
>    * tod_cmp, tod_reg, control regs, and prefix
>    */
> -#define KEXEC_NOTE_BYTES \
> +#define CRASH_CORE_NOTE_BYTES \
>   	(ALIGN(sizeof(struct elf_note), 4) * 8 + \
>   	 ALIGN(sizeof("CORE"), 4) * 7 + \
>   	 ALIGN(sizeof(struct elf_prstatus), 4) + \
> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index e9de6b4..dbc6e5c 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -10,9 +10,16 @@
>   #define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
>   #define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
>
> +/*
> + * The per-cpu notes area is a list of notes terminated by a "NULL"
> + * note header.  For kdump, the code in vmcore.c runs in the context
> + * of the second kernel to combine them into one note.
> + */
> +#ifndef CRASH_CORE_NOTE_BYTES
>   #define CRASH_CORE_NOTE_BYTES	   ((CRASH_CORE_NOTE_HEAD_BYTES * 2) +	\
>   				     CRASH_CORE_NOTE_NAME_BYTES +	\
>   				     CRASH_CORE_NOTE_DESC_BYTES)
> +#endif
>
>   #define VMCOREINFO_BYTES	   PAGE_SIZE
>   #define VMCOREINFO_NOTE_NAME	   "VMCOREINFO"
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 3ea8275..133df03 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -14,7 +14,6 @@
>
>   #if !defined(__ASSEMBLY__)
>
> -#include <linux/crash_core.h>
>   #include <asm/io.h>
>
>   #include <uapi/linux/kexec.h>
> @@ -25,6 +24,7 @@
>   #include <linux/ioport.h>
>   #include <linux/module.h>
>   #include <asm/kexec.h>
> +#include <linux/crash_core.h>
>
>   /* Verify architecture specific macros are defined */
>
> @@ -63,15 +63,6 @@
>   #define KEXEC_CORE_NOTE_NAME	CRASH_CORE_NOTE_NAME
>
>   /*
> - * The per-cpu notes area is a list of notes terminated by a "NULL"
> - * note header.  For kdump, the code in vmcore.c runs in the context
> - * of the second kernel to combine them into one note.
> - */
> -#ifndef KEXEC_NOTE_BYTES
> -#define KEXEC_NOTE_BYTES	CRASH_CORE_NOTE_BYTES
> -#endif
> -
> -/*
>    * This structure is used to hold the arguments that are used when loading
>    * kernel binaries.
>    */

Looks good to me. Thanks for the fix..

- Hari

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition
  2017-06-09  2:29 ` Dave Young
@ 2017-06-09  7:45   ` Dave Young
  2017-06-11  8:43     ` Xunlei Pang
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Young @ 2017-06-09  7:45 UTC (permalink / raw)
  To: Xunlei Pang
  Cc: linux-kernel, kexec, akpm, Eric Biederman, Baoquan He,
	Michael Holzheu, linux-s390, Dave Anderson, Hari Bathini,
	Gustavo Luiz Duarte

On 06/09/17 at 10:29am, Dave Young wrote:
> On 06/09/17 at 10:17am, Xunlei Pang wrote:
> > S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
> > is now defined as follows:
> >     typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
> > It was changed by the CONFIG_CRASH_CORE feature.
> > 
> > This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
> > renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
> > 
> > Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
> > Cc: Dave Young <dyoung@redhat.com>
> > Cc: Dave Anderson <anderson@redhat.com>
> > Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
> > Cc: Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>
> > Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> > ---
> >  arch/s390/include/asm/kexec.h |  2 +-
> >  include/linux/crash_core.h    |  7 +++++++
> >  include/linux/kexec.h         | 11 +----------
> >  3 files changed, 9 insertions(+), 11 deletions(-)
> > 
> > diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
> > index 2f924bc..352deb8 100644
> > --- a/arch/s390/include/asm/kexec.h
> > +++ b/arch/s390/include/asm/kexec.h
> > @@ -47,7 +47,7 @@
> >   * Seven notes plus zero note at the end: prstatus, fpregset, timer,
> >   * tod_cmp, tod_reg, control regs, and prefix
> >   */
> > -#define KEXEC_NOTE_BYTES \
> > +#define CRASH_CORE_NOTE_BYTES \
> >  	(ALIGN(sizeof(struct elf_note), 4) * 8 + \
> >  	 ALIGN(sizeof("CORE"), 4) * 7 + \
> >  	 ALIGN(sizeof(struct elf_prstatus), 4) + \

I found that in mainline since below commit, above define should be
useless, but if distribution with older kernel does need your fix, so in
mainline the right fix should be dropping the s390 part about these
macros usage.

Anyway this need a comment from Michael.

commit 8a07dd02d7615d91d65d6235f7232e3f9b5d347f
Author: Martin Schwidefsky <schwidefsky@de.ibm.com>
Date:   Wed Oct 14 15:53:06 2015 +0200

    s390/kdump: remove code to create ELF notes in the crashed system
    
    The s390 architecture can store the CPU registers of the crashed
system
    after the kdump kernel has been started and this is the preferred
way.
    Remove the remaining code fragments that deal with storing CPU
registers
    while the crashed system is still active.
    
    Acked-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
    Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>


> > diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> > index e9de6b4..dbc6e5c 100644
> > --- a/include/linux/crash_core.h
> > +++ b/include/linux/crash_core.h
> > @@ -10,9 +10,16 @@
> >  #define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
> >  #define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
> >  
> > +/*
> > + * The per-cpu notes area is a list of notes terminated by a "NULL"
> > + * note header.  For kdump, the code in vmcore.c runs in the context
> > + * of the second kernel to combine them into one note.
> > + */
> > +#ifndef CRASH_CORE_NOTE_BYTES
> >  #define CRASH_CORE_NOTE_BYTES	   ((CRASH_CORE_NOTE_HEAD_BYTES * 2) +	\
> >  				     CRASH_CORE_NOTE_NAME_BYTES +	\
> >  				     CRASH_CORE_NOTE_DESC_BYTES)
> > +#endif
> >  
> >  #define VMCOREINFO_BYTES	   PAGE_SIZE
> >  #define VMCOREINFO_NOTE_NAME	   "VMCOREINFO"
> > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > index 3ea8275..133df03 100644
> > --- a/include/linux/kexec.h
> > +++ b/include/linux/kexec.h
> > @@ -14,7 +14,6 @@
> >  
> >  #if !defined(__ASSEMBLY__)
> >  
> > -#include <linux/crash_core.h>
> >  #include <asm/io.h>
> >  
> >  #include <uapi/linux/kexec.h>
> > @@ -25,6 +24,7 @@
> >  #include <linux/ioport.h>
> >  #include <linux/module.h>
> >  #include <asm/kexec.h>
> > +#include <linux/crash_core.h>
> >  
> >  /* Verify architecture specific macros are defined */
> >  
> > @@ -63,15 +63,6 @@
> >  #define KEXEC_CORE_NOTE_NAME	CRASH_CORE_NOTE_NAME
> >  
> >  /*
> > - * The per-cpu notes area is a list of notes terminated by a "NULL"
> > - * note header.  For kdump, the code in vmcore.c runs in the context
> > - * of the second kernel to combine them into one note.
> > - */
> > -#ifndef KEXEC_NOTE_BYTES
> > -#define KEXEC_NOTE_BYTES	CRASH_CORE_NOTE_BYTES
> > -#endif
> 
> It is still not clear how does s390 use the crash_notes except this macro.
> But from code point of view we do need to update this as well after the
> crash_core splitting.
> 
> Acked-by: Dave Young <dyoung@redhat.com>

Hold on the ack because of the new findings, wait for Michael's
feedback.

Thanks
Dave

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition
  2017-06-09  7:45   ` Dave Young
@ 2017-06-11  8:43     ` Xunlei Pang
  0 siblings, 0 replies; 9+ messages in thread
From: Xunlei Pang @ 2017-06-11  8:43 UTC (permalink / raw)
  To: Dave Young, Xunlei Pang
  Cc: linux-kernel, kexec, akpm, Eric Biederman, Baoquan He,
	Michael Holzheu, linux-s390, Dave Anderson, Hari Bathini,
	Gustavo Luiz Duarte

On 06/09/2017 at 03:45 PM, Dave Young wrote:
> On 06/09/17 at 10:29am, Dave Young wrote:
>> On 06/09/17 at 10:17am, Xunlei Pang wrote:
>>> S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
>>> is now defined as follows:
>>>     typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
>>> It was changed by the CONFIG_CRASH_CORE feature.
>>>
>>> This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
>>> renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
>>>
>>> Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
>>> Cc: Dave Young <dyoung@redhat.com>
>>> Cc: Dave Anderson <anderson@redhat.com>
>>> Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
>>> Cc: Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>
>>> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
>>> ---
>>>  arch/s390/include/asm/kexec.h |  2 +-
>>>  include/linux/crash_core.h    |  7 +++++++
>>>  include/linux/kexec.h         | 11 +----------
>>>  3 files changed, 9 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
>>> index 2f924bc..352deb8 100644
>>> --- a/arch/s390/include/asm/kexec.h
>>> +++ b/arch/s390/include/asm/kexec.h
>>> @@ -47,7 +47,7 @@
>>>   * Seven notes plus zero note at the end: prstatus, fpregset, timer,
>>>   * tod_cmp, tod_reg, control regs, and prefix
>>>   */
>>> -#define KEXEC_NOTE_BYTES \
>>> +#define CRASH_CORE_NOTE_BYTES \
>>>  	(ALIGN(sizeof(struct elf_note), 4) * 8 + \
>>>  	 ALIGN(sizeof("CORE"), 4) * 7 + \
>>>  	 ALIGN(sizeof(struct elf_prstatus), 4) + \
> I found that in mainline since below commit, above define should be
> useless, but if distribution with older kernel does need your fix, so in
> mainline the right fix should be dropping the s390 part about these
> macros usage.

Indeed, then I think we can remove this special definition of S390 to avoid confusion.

Regards,
Xunlei

>
> Anyway this need a comment from Michael.
>
> commit 8a07dd02d7615d91d65d6235f7232e3f9b5d347f
> Author: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Date:   Wed Oct 14 15:53:06 2015 +0200
>
>     s390/kdump: remove code to create ELF notes in the crashed system
>     
>     The s390 architecture can store the CPU registers of the crashed
> system
>     after the kdump kernel has been started and this is the preferred
> way.
>     Remove the remaining code fragments that deal with storing CPU
> registers
>     while the crashed system is still active.
>     
>     Acked-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
>     Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
>
>
>>> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
>>> index e9de6b4..dbc6e5c 100644
>>> --- a/include/linux/crash_core.h
>>> +++ b/include/linux/crash_core.h
>>> @@ -10,9 +10,16 @@
>>>  #define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
>>>  #define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
>>>  
>>> +/*
>>> + * The per-cpu notes area is a list of notes terminated by a "NULL"
>>> + * note header.  For kdump, the code in vmcore.c runs in the context
>>> + * of the second kernel to combine them into one note.
>>> + */
>>> +#ifndef CRASH_CORE_NOTE_BYTES
>>>  #define CRASH_CORE_NOTE_BYTES	   ((CRASH_CORE_NOTE_HEAD_BYTES * 2) +	\
>>>  				     CRASH_CORE_NOTE_NAME_BYTES +	\
>>>  				     CRASH_CORE_NOTE_DESC_BYTES)
>>> +#endif
>>>  
>>>  #define VMCOREINFO_BYTES	   PAGE_SIZE
>>>  #define VMCOREINFO_NOTE_NAME	   "VMCOREINFO"
>>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>>> index 3ea8275..133df03 100644
>>> --- a/include/linux/kexec.h
>>> +++ b/include/linux/kexec.h
>>> @@ -14,7 +14,6 @@
>>>  
>>>  #if !defined(__ASSEMBLY__)
>>>  
>>> -#include <linux/crash_core.h>
>>>  #include <asm/io.h>
>>>  
>>>  #include <uapi/linux/kexec.h>
>>> @@ -25,6 +24,7 @@
>>>  #include <linux/ioport.h>
>>>  #include <linux/module.h>
>>>  #include <asm/kexec.h>
>>> +#include <linux/crash_core.h>
>>>  
>>>  /* Verify architecture specific macros are defined */
>>>  
>>> @@ -63,15 +63,6 @@
>>>  #define KEXEC_CORE_NOTE_NAME	CRASH_CORE_NOTE_NAME
>>>  
>>>  /*
>>> - * The per-cpu notes area is a list of notes terminated by a "NULL"
>>> - * note header.  For kdump, the code in vmcore.c runs in the context
>>> - * of the second kernel to combine them into one note.
>>> - */
>>> -#ifndef KEXEC_NOTE_BYTES
>>> -#define KEXEC_NOTE_BYTES	CRASH_CORE_NOTE_BYTES
>>> -#endif
>> It is still not clear how does s390 use the crash_notes except this macro.
>> But from code point of view we do need to update this as well after the
>> crash_core splitting.
>>
>> Acked-by: Dave Young <dyoung@redhat.com>
> Hold on the ack because of the new findings, wait for Michael's
> feedback.
>
> Thanks
> Dave

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition
  2017-06-09  2:17 [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition Xunlei Pang
  2017-06-09  2:29 ` Dave Young
  2017-06-09  4:12 ` Hari Bathini
@ 2017-06-11 19:46 ` kbuild test robot
  2017-06-21  9:00 ` Michael Holzheu
  2017-06-21 17:44 ` Michael Holzheu
  4 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2017-06-11 19:46 UTC (permalink / raw)
  To: Xunlei Pang
  Cc: kbuild-all, linux-kernel, kexec, akpm, Eric Biederman,
	Dave Young, Baoquan He, Michael Holzheu, linux-s390, Xunlei Pang,
	Dave Anderson, Hari Bathini, Gustavo Luiz Duarte

[-- Attachment #1: Type: text/plain, Size: 10761 bytes --]

Hi Xunlei,

[auto build test ERROR on next-20170605]
[also build test ERROR on v4.12-rc4]
[cannot apply to linus/master linux/master s390/features v4.9-rc8 v4.9-rc7 v4.9-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Xunlei-Pang/s390-crash-Fix-KEXEC_NOTE_BYTES-definition/20170610-173047
config: arm-sa1100 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   In file included from arch/arm/include/asm/memory.h:378:0,
                    from arch/arm/include/asm/page.h:165,
                    from arch/arm/include/asm/thread_info.h:17,
                    from include/linux/thread_info.h:37,
                    from include/asm-generic/preempt.h:4,
                    from ./arch/arm/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:80,
                    from include/linux/spinlock.h:50,
                    from include/linux/seqlock.h:35,
                    from include/linux/time.h:5,
                    from include/linux/efi.h:16,
                    from arch/arm/kernel/setup.c:10:
   arch/arm/include/asm/kexec.h: In function 'page_to_boot_pfn':
>> include/asm-generic/memory_model.h:63:14: error: implicit declaration of function 'page_to_section' [-Werror=implicit-function-declaration]
     int __sec = page_to_section(__pg);   \
                 ^
   include/asm-generic/memory_model.h:80:21: note: in expansion of macro '__page_to_pfn'
    #define page_to_pfn __page_to_pfn
                        ^~~~~~~~~~~~~
   arch/arm/include/asm/kexec.h:70:9: note: in expansion of macro 'page_to_pfn'
     return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
            ^~~~~~~~~~~
   In file included from include/linux/pid_namespace.h:6:0,
                    from include/linux/ptrace.h:9,
                    from include/uapi/linux/elfcore.h:7,
                    from include/linux/elfcore.h:9,
                    from include/linux/crash_core.h:5,
                    from include/linux/kexec.h:27,
                    from arch/arm/kernel/setup.c:24:
   include/linux/mm.h: At top level:
>> include/linux/mm.h:979:29: error: conflicting types for 'page_to_section'
    static inline unsigned long page_to_section(const struct page *page)
                                ^~~~~~~~~~~~~~~
   In file included from arch/arm/include/asm/memory.h:378:0,
                    from arch/arm/include/asm/page.h:165,
                    from arch/arm/include/asm/thread_info.h:17,
                    from include/linux/thread_info.h:37,
                    from include/asm-generic/preempt.h:4,
                    from ./arch/arm/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:80,
                    from include/linux/spinlock.h:50,
                    from include/linux/seqlock.h:35,
                    from include/linux/time.h:5,
                    from include/linux/efi.h:16,
                    from arch/arm/kernel/setup.c:10:
   include/asm-generic/memory_model.h:63:14: note: previous implicit declaration of 'page_to_section' was here
     int __sec = page_to_section(__pg);   \
                 ^
   include/asm-generic/memory_model.h:80:21: note: in expansion of macro '__page_to_pfn'
    #define page_to_pfn __page_to_pfn
                        ^~~~~~~~~~~~~
   arch/arm/include/asm/kexec.h:70:9: note: in expansion of macro 'page_to_pfn'
     return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
            ^~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   In file included from arch/arm/include/asm/memory.h:378:0,
                    from arch/arm/include/asm/page.h:165,
                    from arch/arm/include/asm/thread_info.h:17,
                    from include/linux/thread_info.h:37,
                    from include/asm-generic/preempt.h:4,
                    from ./arch/arm/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:80,
                    from include/linux/spinlock.h:50,
                    from arch/arm/kernel/traps.c:18:
   arch/arm/include/asm/kexec.h: In function 'page_to_boot_pfn':
>> include/asm-generic/memory_model.h:63:14: error: implicit declaration of function 'page_to_section' [-Werror=implicit-function-declaration]
     int __sec = page_to_section(__pg);   \
                 ^
   include/asm-generic/memory_model.h:80:21: note: in expansion of macro '__page_to_pfn'
    #define page_to_pfn __page_to_pfn
                        ^~~~~~~~~~~~~
   arch/arm/include/asm/kexec.h:70:9: note: in expansion of macro 'page_to_pfn'
     return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
            ^~~~~~~~~~~
   include/asm-generic/memory_model.h:64:23: error: arithmetic on pointer to an incomplete type
     (unsigned long)(__pg - __section_mem_map_addr(__nr_to_section(__sec))); \
                          ^
   include/asm-generic/memory_model.h:80:21: note: in expansion of macro '__page_to_pfn'
    #define page_to_pfn __page_to_pfn
                        ^~~~~~~~~~~~~
   arch/arm/include/asm/kexec.h:70:9: note: in expansion of macro 'page_to_pfn'
     return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
            ^~~~~~~~~~~
   arch/arm/include/asm/kexec.h: In function 'boot_pfn_to_page':
>> include/asm-generic/memory_model.h:69:9: error: invalid use of undefined type 'struct page'
     struct mem_section *__sec = __pfn_to_section(__pfn); \
            ^
   include/asm-generic/memory_model.h:81:21: note: in expansion of macro '__pfn_to_page'
    #define pfn_to_page __pfn_to_page
                        ^~~~~~~~~~~~~
>> arch/arm/include/asm/kexec.h:76:9: note: in expansion of macro 'pfn_to_page'
     return pfn_to_page(boot_pfn - (arch_phys_to_idmap_offset >> PAGE_SHIFT));
            ^~~~~~~~~~~
   In file included from include/linux/pid_namespace.h:6:0,
                    from include/linux/ptrace.h:9,
                    from include/uapi/linux/elfcore.h:7,
                    from include/linux/elfcore.h:9,
                    from include/linux/crash_core.h:5,
                    from include/linux/kexec.h:27,
                    from arch/arm/kernel/traps.c:23:
   include/linux/mm.h: At top level:
>> include/linux/mm.h:979:29: error: conflicting types for 'page_to_section'
    static inline unsigned long page_to_section(const struct page *page)
                                ^~~~~~~~~~~~~~~
   In file included from arch/arm/include/asm/memory.h:378:0,
                    from arch/arm/include/asm/page.h:165,
                    from arch/arm/include/asm/thread_info.h:17,
                    from include/linux/thread_info.h:37,
                    from include/asm-generic/preempt.h:4,
                    from ./arch/arm/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:80,
                    from include/linux/spinlock.h:50,
                    from arch/arm/kernel/traps.c:18:
   include/asm-generic/memory_model.h:63:14: note: previous implicit declaration of 'page_to_section' was here
     int __sec = page_to_section(__pg);   \
                 ^
   include/asm-generic/memory_model.h:80:21: note: in expansion of macro '__page_to_pfn'
    #define page_to_pfn __page_to_pfn
                        ^~~~~~~~~~~~~
   arch/arm/include/asm/kexec.h:70:9: note: in expansion of macro 'page_to_pfn'
     return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
            ^~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/pfn_to_page +76 arch/arm/include/asm/kexec.h

0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  60  #define phys_to_boot_phys phys_to_boot_phys
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  61  
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  62  static inline phys_addr_t boot_phys_to_phys(unsigned long entry)
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  63  {
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  64  	return idmap_to_phys(entry);
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  65  }
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  66  #define boot_phys_to_phys boot_phys_to_phys
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  67  
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  68  static inline unsigned long page_to_boot_pfn(struct page *page)
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  69  {
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  70  	return page_to_pfn(page) + (arch_phys_to_idmap_offset >> PAGE_SHIFT);
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  71  }
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  72  #define page_to_boot_pfn page_to_boot_pfn
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  73  
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  74  static inline struct page *boot_pfn_to_page(unsigned long boot_pfn)
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  75  {
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02 @76  	return pfn_to_page(boot_pfn - (arch_phys_to_idmap_offset >> PAGE_SHIFT));
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  77  }
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  78  #define boot_pfn_to_page boot_pfn_to_page
0719392a arch/arm/include/asm/kexec.h Russell King   2016-08-02  79  
c587e4a6 include/asm-arm/kexec.h      Richard Purdie 2007-02-06  80  #endif /* __ASSEMBLY__ */
c587e4a6 include/asm-arm/kexec.h      Richard Purdie 2007-02-06  81  
c587e4a6 include/asm-arm/kexec.h      Richard Purdie 2007-02-06  82  #endif /* CONFIG_KEXEC */
c587e4a6 include/asm-arm/kexec.h      Richard Purdie 2007-02-06  83  
c587e4a6 include/asm-arm/kexec.h      Richard Purdie 2007-02-06  84  #endif /* _ARM_KEXEC_H */

:::::: The code at line 76 was first introduced by commit
:::::: 0719392a61a9dbc2c850bc7bd1a17efba953fcf5 ARM: kexec: fix kexec for Keystone 2

:::::: TO: Russell King <rmk+kernel@arm.linux.org.uk>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23734 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition
  2017-06-09  2:17 [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition Xunlei Pang
                   ` (2 preceding siblings ...)
  2017-06-11 19:46 ` kbuild test robot
@ 2017-06-21  9:00 ` Michael Holzheu
  2017-06-21 17:44 ` Michael Holzheu
  4 siblings, 0 replies; 9+ messages in thread
From: Michael Holzheu @ 2017-06-21  9:00 UTC (permalink / raw)
  To: Xunlei Pang
  Cc: linux-kernel, kexec, akpm, Eric Biederman, Dave Young,
	Baoquan He, linux-s390, Dave Anderson, Hari Bathini,
	Gustavo Luiz Duarte, Mikhail Zaslonko

Hi Xunlei,

Sorry for the late reply - I was on vacation up to now.
Give us some time to look into this issue.

Michael

Am Fri,  9 Jun 2017 10:17:05 +0800
schrieb Xunlei Pang <xlpang@redhat.com>:

> S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
> is now defined as follows:
>     typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
> It was changed by the CONFIG_CRASH_CORE feature.
> 
> This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
> renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
> 
> Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
> Cc: Dave Young <dyoung@redhat.com>
> Cc: Dave Anderson <anderson@redhat.com>
> Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
> Cc: Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>
> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> ---
>  arch/s390/include/asm/kexec.h |  2 +-
>  include/linux/crash_core.h    |  7 +++++++
>  include/linux/kexec.h         | 11 +----------
>  3 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
> index 2f924bc..352deb8 100644
> --- a/arch/s390/include/asm/kexec.h
> +++ b/arch/s390/include/asm/kexec.h
> @@ -47,7 +47,7 @@
>   * Seven notes plus zero note at the end: prstatus, fpregset, timer,
>   * tod_cmp, tod_reg, control regs, and prefix
>   */
> -#define KEXEC_NOTE_BYTES \
> +#define CRASH_CORE_NOTE_BYTES \
>  	(ALIGN(sizeof(struct elf_note), 4) * 8 + \
>  	 ALIGN(sizeof("CORE"), 4) * 7 + \
>  	 ALIGN(sizeof(struct elf_prstatus), 4) + \
> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index e9de6b4..dbc6e5c 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -10,9 +10,16 @@
>  #define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
>  #define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
> 
> +/*
> + * The per-cpu notes area is a list of notes terminated by a "NULL"
> + * note header.  For kdump, the code in vmcore.c runs in the context
> + * of the second kernel to combine them into one note.
> + */
> +#ifndef CRASH_CORE_NOTE_BYTES
>  #define CRASH_CORE_NOTE_BYTES	   ((CRASH_CORE_NOTE_HEAD_BYTES * 2) +	\
>  				     CRASH_CORE_NOTE_NAME_BYTES +	\
>  				     CRASH_CORE_NOTE_DESC_BYTES)
> +#endif
> 
>  #define VMCOREINFO_BYTES	   PAGE_SIZE
>  #define VMCOREINFO_NOTE_NAME	   "VMCOREINFO"
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 3ea8275..133df03 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -14,7 +14,6 @@
> 
>  #if !defined(__ASSEMBLY__)
> 
> -#include <linux/crash_core.h>
>  #include <asm/io.h>
> 
>  #include <uapi/linux/kexec.h>
> @@ -25,6 +24,7 @@
>  #include <linux/ioport.h>
>  #include <linux/module.h>
>  #include <asm/kexec.h>
> +#include <linux/crash_core.h>
> 
>  /* Verify architecture specific macros are defined */
> 
> @@ -63,15 +63,6 @@
>  #define KEXEC_CORE_NOTE_NAME	CRASH_CORE_NOTE_NAME
> 
>  /*
> - * The per-cpu notes area is a list of notes terminated by a "NULL"
> - * note header.  For kdump, the code in vmcore.c runs in the context
> - * of the second kernel to combine them into one note.
> - */
> -#ifndef KEXEC_NOTE_BYTES
> -#define KEXEC_NOTE_BYTES	CRASH_CORE_NOTE_BYTES
> -#endif
> -
> -/*
>   * This structure is used to hold the arguments that are used when loading
>   * kernel binaries.
>   */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition
  2017-06-09  2:17 [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition Xunlei Pang
                   ` (3 preceding siblings ...)
  2017-06-21  9:00 ` Michael Holzheu
@ 2017-06-21 17:44 ` Michael Holzheu
  2017-06-22  9:12   ` Xunlei Pang
  4 siblings, 1 reply; 9+ messages in thread
From: Michael Holzheu @ 2017-06-21 17:44 UTC (permalink / raw)
  To: Xunlei Pang
  Cc: linux-kernel, kexec, akpm, Eric Biederman, Dave Young,
	Baoquan He, linux-s390, Dave Anderson, Hari Bathini,
	Gustavo Luiz Duarte, Mikhail Zaslonko

Am Fri,  9 Jun 2017 10:17:05 +0800
schrieb Xunlei Pang <xlpang@redhat.com>:

> S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
> is now defined as follows:
>     typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
> It was changed by the CONFIG_CRASH_CORE feature.
> 
> This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
> renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
> 
> Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
> Cc: Dave Young <dyoung@redhat.com>
> Cc: Dave Anderson <anderson@redhat.com>
> Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
> Cc: Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>
> Signed-off-by: Xunlei Pang <xlpang@redhat.com>

Hello Xunlei,

As you already know on s390 we create the ELF header in the new kernel.
Therefore we don't use the per-cpu buffers for ELF notes to store
the register state.

For RHEL7 we still store the registers in machine_kexec.c:add_elf_notes().
Though we also use the ELF header from new kernel ...

We assume your original problem with the "kmem -s" failure
was caused by the memory overwrite due to the invalid size of the
"crash_notes" per-cpu buffers.

Therefore your patch looks good for RHEL7 but for upstream we propose the
patch below.
---
[PATCH] s390/crash: Remove unused KEXEC_NOTE_BYTES

After commmit 692f66f26a4c19 ("crash: move crashkernel parsing and vmcore
related code under CONFIG_CRASH_CORE") the KEXEC_NOTE_BYTES macro is not
used anymore and for s390 we create the ELF header in the new kernel
anyway. Therefore remove the macro.

Reported-by: Xunlei Pang <xpang@redhat.com>
Reviewed-by: Mikhail Zaslonko <zaslonko@linux.vnet.ibm.com>
Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 arch/s390/include/asm/kexec.h | 18 ------------------
 include/linux/crash_core.h    |  5 +++++
 include/linux/kexec.h         |  9 ---------
 3 files changed, 5 insertions(+), 27 deletions(-)

diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
index 2f924bc30e35..dccf24ee26d3 100644
--- a/arch/s390/include/asm/kexec.h
+++ b/arch/s390/include/asm/kexec.h
@@ -41,24 +41,6 @@
 /* The native architecture */
 #define KEXEC_ARCH KEXEC_ARCH_S390
 
-/*
- * Size for s390x ELF notes per CPU
- *
- * Seven notes plus zero note at the end: prstatus, fpregset, timer,
- * tod_cmp, tod_reg, control regs, and prefix
- */
-#define KEXEC_NOTE_BYTES \
-	(ALIGN(sizeof(struct elf_note), 4) * 8 + \
-	 ALIGN(sizeof("CORE"), 4) * 7 + \
-	 ALIGN(sizeof(struct elf_prstatus), 4) + \
-	 ALIGN(sizeof(elf_fpregset_t), 4) + \
-	 ALIGN(sizeof(u64), 4) + \
-	 ALIGN(sizeof(u64), 4) + \
-	 ALIGN(sizeof(u32), 4) + \
-	 ALIGN(sizeof(u64) * 16, 4) + \
-	 ALIGN(sizeof(u32), 4) \
-	)
-
 /* Provide a dummy definition to avoid build failures. */
 static inline void crash_setup_regs(struct pt_regs *newregs,
 					struct pt_regs *oldregs) { }
diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
index 541a197ba4a2..4090a42578a8 100644
--- a/include/linux/crash_core.h
+++ b/include/linux/crash_core.h
@@ -10,6 +10,11 @@
 #define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
 #define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
 
+/*
+ * The per-cpu notes area is a list of notes terminated by a "NULL"
+ * note header.  For kdump, the code in vmcore.c runs in the context
+ * of the second kernel to combine them into one note.
+ */
 #define CRASH_CORE_NOTE_BYTES	   ((CRASH_CORE_NOTE_HEAD_BYTES * 2) +	\
 				     CRASH_CORE_NOTE_NAME_BYTES +	\
 				     CRASH_CORE_NOTE_DESC_BYTES)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index c9481ebcbc0c..65888418fb69 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -63,15 +63,6 @@
 #define KEXEC_CORE_NOTE_NAME	CRASH_CORE_NOTE_NAME
 
 /*
- * The per-cpu notes area is a list of notes terminated by a "NULL"
- * note header.  For kdump, the code in vmcore.c runs in the context
- * of the second kernel to combine them into one note.
- */
-#ifndef KEXEC_NOTE_BYTES
-#define KEXEC_NOTE_BYTES	CRASH_CORE_NOTE_BYTES
-#endif
-
-/*
  * This structure is used to hold the arguments that are used when loading
  * kernel binaries.
  */
-- 
2.11.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition
  2017-06-21 17:44 ` Michael Holzheu
@ 2017-06-22  9:12   ` Xunlei Pang
  0 siblings, 0 replies; 9+ messages in thread
From: Xunlei Pang @ 2017-06-22  9:12 UTC (permalink / raw)
  To: Michael Holzheu, Xunlei Pang
  Cc: linux-kernel, kexec, akpm, Eric Biederman, Dave Young,
	Baoquan He, linux-s390, Dave Anderson, Hari Bathini,
	Gustavo Luiz Duarte, Mikhail Zaslonko

On 06/22/2017 at 01:44 AM, Michael Holzheu wrote:
> Am Fri,  9 Jun 2017 10:17:05 +0800
> schrieb Xunlei Pang <xlpang@redhat.com>:
>
>> S390 KEXEC_NOTE_BYTES is not used by note_buf_t as before, which
>> is now defined as follows:
>>     typedef u32 note_buf_t[CRASH_CORE_NOTE_BYTES/4];
>> It was changed by the CONFIG_CRASH_CORE feature.
>>
>> This patch gets rid of all the old KEXEC_NOTE_BYTES stuff, and
>> renames KEXEC_NOTE_BYTES to CRASH_CORE_NOTE_BYTES for S390.
>>
>> Fixes: 692f66f26a4c ("crash: move crashkernel parsing and vmcore related code under CONFIG_CRASH_CORE")
>> Cc: Dave Young <dyoung@redhat.com>
>> Cc: Dave Anderson <anderson@redhat.com>
>> Cc: Hari Bathini <hbathini@linux.vnet.ibm.com>
>> Cc: Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>
>> Signed-off-by: Xunlei Pang <xlpang@redhat.com>
> Hello Xunlei,
>
> As you already know on s390 we create the ELF header in the new kernel.
> Therefore we don't use the per-cpu buffers for ELF notes to store
> the register state.
>
> For RHEL7 we still store the registers in machine_kexec.c:add_elf_notes().
> Though we also use the ELF header from new kernel ...
>
> We assume your original problem with the "kmem -s" failure
> was caused by the memory overwrite due to the invalid size of the
> "crash_notes" per-cpu buffers.
>
> Therefore your patch looks good for RHEL7 but for upstream we propose the
> patch below.

Hi Michael,

Yes, we already did this way.
Thanks for the confirmation, the patch below looks good to me.

Regards,
Xunlei

> ---
> [PATCH] s390/crash: Remove unused KEXEC_NOTE_BYTES
>
> After commmit 692f66f26a4c19 ("crash: move crashkernel parsing and vmcore
> related code under CONFIG_CRASH_CORE") the KEXEC_NOTE_BYTES macro is not
> used anymore and for s390 we create the ELF header in the new kernel
> anyway. Therefore remove the macro.
>
> Reported-by: Xunlei Pang <xpang@redhat.com>
> Reviewed-by: Mikhail Zaslonko <zaslonko@linux.vnet.ibm.com>
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> ---
>  arch/s390/include/asm/kexec.h | 18 ------------------
>  include/linux/crash_core.h    |  5 +++++
>  include/linux/kexec.h         |  9 ---------
>  3 files changed, 5 insertions(+), 27 deletions(-)
>
> diff --git a/arch/s390/include/asm/kexec.h b/arch/s390/include/asm/kexec.h
> index 2f924bc30e35..dccf24ee26d3 100644
> --- a/arch/s390/include/asm/kexec.h
> +++ b/arch/s390/include/asm/kexec.h
> @@ -41,24 +41,6 @@
>  /* The native architecture */
>  #define KEXEC_ARCH KEXEC_ARCH_S390
>  
> -/*
> - * Size for s390x ELF notes per CPU
> - *
> - * Seven notes plus zero note at the end: prstatus, fpregset, timer,
> - * tod_cmp, tod_reg, control regs, and prefix
> - */
> -#define KEXEC_NOTE_BYTES \
> -	(ALIGN(sizeof(struct elf_note), 4) * 8 + \
> -	 ALIGN(sizeof("CORE"), 4) * 7 + \
> -	 ALIGN(sizeof(struct elf_prstatus), 4) + \
> -	 ALIGN(sizeof(elf_fpregset_t), 4) + \
> -	 ALIGN(sizeof(u64), 4) + \
> -	 ALIGN(sizeof(u64), 4) + \
> -	 ALIGN(sizeof(u32), 4) + \
> -	 ALIGN(sizeof(u64) * 16, 4) + \
> -	 ALIGN(sizeof(u32), 4) \
> -	)
> -
>  /* Provide a dummy definition to avoid build failures. */
>  static inline void crash_setup_regs(struct pt_regs *newregs,
>  					struct pt_regs *oldregs) { }
> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index 541a197ba4a2..4090a42578a8 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -10,6 +10,11 @@
>  #define CRASH_CORE_NOTE_NAME_BYTES ALIGN(sizeof(CRASH_CORE_NOTE_NAME), 4)
>  #define CRASH_CORE_NOTE_DESC_BYTES ALIGN(sizeof(struct elf_prstatus), 4)
>  
> +/*
> + * The per-cpu notes area is a list of notes terminated by a "NULL"
> + * note header.  For kdump, the code in vmcore.c runs in the context
> + * of the second kernel to combine them into one note.
> + */
>  #define CRASH_CORE_NOTE_BYTES	   ((CRASH_CORE_NOTE_HEAD_BYTES * 2) +	\
>  				     CRASH_CORE_NOTE_NAME_BYTES +	\
>  				     CRASH_CORE_NOTE_DESC_BYTES)
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index c9481ebcbc0c..65888418fb69 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -63,15 +63,6 @@
>  #define KEXEC_CORE_NOTE_NAME	CRASH_CORE_NOTE_NAME
>  
>  /*
> - * The per-cpu notes area is a list of notes terminated by a "NULL"
> - * note header.  For kdump, the code in vmcore.c runs in the context
> - * of the second kernel to combine them into one note.
> - */
> -#ifndef KEXEC_NOTE_BYTES
> -#define KEXEC_NOTE_BYTES	CRASH_CORE_NOTE_BYTES
> -#endif
> -
> -/*
>   * This structure is used to hold the arguments that are used when loading
>   * kernel binaries.
>   */

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-06-22  9:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-09  2:17 [PATCH] s390/crash: Fix KEXEC_NOTE_BYTES definition Xunlei Pang
2017-06-09  2:29 ` Dave Young
2017-06-09  7:45   ` Dave Young
2017-06-11  8:43     ` Xunlei Pang
2017-06-09  4:12 ` Hari Bathini
2017-06-11 19:46 ` kbuild test robot
2017-06-21  9:00 ` Michael Holzheu
2017-06-21 17:44 ` Michael Holzheu
2017-06-22  9:12   ` Xunlei Pang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).