RE: [PATCH v4 1/4] tree-wide: Retrieve 'MAX_PHYSMEM_BITS' from vmcoreinfo (if available)
Hi Bhupesh, > -Original Message- > > > -Original Message- > > > This patch adds a common feature for archs (except arm64, for which > > > similar support is added via subsequent patch) to retrieve > > > 'MAX_PHYSMEM_BITS' from vmcoreinfo (if available). > > > > We already have the calibrate_machdep_info() function, which sets > > info->max_physmem_bits from vmcoreinfo, so practically we don't need > > to add this patch for the benefit. I meant that we already have an arch-independent setter for info->max_physmem_bits: 3714 int 3715 calibrate_machdep_info(void) 3716 { 3717 if (NUMBER(MAX_PHYSMEM_BITS) > 0) 3718 info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); 3719 3720 if (NUMBER(SECTION_SIZE_BITS) > 0) 3721 info->section_size_bits = NUMBER(SECTION_SIZE_BITS); 3722 3723 return TRUE; 3724 } so if NUMBER(MAX_PHYSMEM_BITS) appears, it is automatically used in makedumpfile without this patch 1/4. Thanks, Kazu > > Since other user-space tools like crash use the 'MAX_PHYSMEM_BITS' value as > well > it was agreed with the arm64 maintainers that it would be a good > approach to export the > same in vmcoreinfo and not use different methods to determine the same > in user-space. > > Take an example of the PPC makedumpfile implementation for example. It > uses the following complex method of dtereming > 'info->max_physmem_bits': > int > set_ppc64_max_physmem_bits(void) > { > long array_len = ARRAY_LENGTH(mem_section); > /* > * The older ppc64 kernels uses _MAX_PHYSMEM_BITS as 42 and the > * newer kernels 3.7 onwards uses 46 bits. > */ > > info->max_physmem_bits = _MAX_PHYSMEM_BITS_ORIG ; > if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) > || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( > return TRUE; > > info->max_physmem_bits = _MAX_PHYSMEM_BITS_3_7; > if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) > || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( > return TRUE; > > info->max_physmem_bits = _MAX_PHYSMEM_BITS_4_19; > if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) > || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( > return TRUE; > > info->max_physmem_bits = _MAX_PHYSMEM_BITS_4_20; > if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) > || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( > return TRUE; > > return FALSE; > } > > This might need modification and introduction of another > _MAX_PHYSMEM_BITS_x_y macro when this changes for a newer kernel > version. > > I think this makes the code error-prone and hard to read. Its much > better to replace it with: > /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ > if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) { > info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); > return TRUE; > } else { > .. > } > > I think it will reduce future reworks (as per kernel versions) and > also reduce issues while backporting makedumpfile to older kernels. > > What do you think? > > Regards, > Bhupesh > > > I recently posted a kernel patch (see [0]) which appends > > > 'MAX_PHYSMEM_BITS' to vmcoreinfo in the core code itself rather than > > > in arch-specific code, so that user-space code can also benefit from > > > this addition to the vmcoreinfo and use it as a standard way of > > > determining 'SECTIONS_SHIFT' value in 'makedumpfile' utility. > > > > > > This patch ensures backward compatibility for kernel versions in which > > > 'MAX_PHYSMEM_BITS' is not available in vmcoreinfo. > > > > > > [0]. http://lists.infradead.org/pipermail/kexec/2019-November/023960.html > > > > > > Cc: Kazuhito Hagio > > > Cc: John Donnelly > > > Cc: kexec@lists.infradead.org > > > Signed-off-by: Bhupesh Sharma > > > --- > > > arch/arm.c | 8 +++- > > > arch/ia64.c| 7 ++- > > > arch/ppc.c | 8 +++- > > > arch/ppc64.c | 49 - > > > arch/s390x.c | 29 ++--- > > > arch/sparc64.c | 9 +++-- > > > arch/x86.c | 34 -- > > > arch/x86_64.c | 27 --- > > > 8 files changed, 109 insertions(+), 62 deletions(-) > > > > > > diff --git a/arch/arm.c b/arch/arm.c > > > index af7442ac70bf..33536fc4dfc9 100644 > > > --- a/arch/arm.c > > > +++ b/arch/arm.c > > > @@ -81,7 +81,13 @@ int > > > get_machdep_info_arm(void) > > > { > > > info->page_offset = SYMBOL(_stext) & 0xUL; > > > - info->max_physmem_bits = _MAX_PHYSMEM_BITS; > > > + > > > + /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ > > > + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) > > > + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); > > > + el
Re: [PATCH v4 1/4] tree-wide: Retrieve 'MAX_PHYSMEM_BITS' from vmcoreinfo (if available)
Hi Kazu, On Wed, Dec 4, 2019 at 11:05 PM Kazuhito Hagio wrote: > > Hi Bhupesh, > > Sorry for the late reply. No problem. > > -Original Message- > > This patch adds a common feature for archs (except arm64, for which > > similar support is added via subsequent patch) to retrieve > > 'MAX_PHYSMEM_BITS' from vmcoreinfo (if available). > > We already have the calibrate_machdep_info() function, which sets > info->max_physmem_bits from vmcoreinfo, so practically we don't need > to add this patch for the benefit. Since other user-space tools like crash use the 'MAX_PHYSMEM_BITS' value as well it was agreed with the arm64 maintainers that it would be a good approach to export the same in vmcoreinfo and not use different methods to determine the same in user-space. Take an example of the PPC makedumpfile implementation for example. It uses the following complex method of dtereming 'info->max_physmem_bits': int set_ppc64_max_physmem_bits(void) { long array_len = ARRAY_LENGTH(mem_section); /* * The older ppc64 kernels uses _MAX_PHYSMEM_BITS as 42 and the * newer kernels 3.7 onwards uses 46 bits. */ info->max_physmem_bits = _MAX_PHYSMEM_BITS_ORIG ; if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( return TRUE; info->max_physmem_bits = _MAX_PHYSMEM_BITS_3_7; if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( return TRUE; info->max_physmem_bits = _MAX_PHYSMEM_BITS_4_19; if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( return TRUE; info->max_physmem_bits = _MAX_PHYSMEM_BITS_4_20; if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( return TRUE; return FALSE; } This might need modification and introduction of another _MAX_PHYSMEM_BITS_x_y macro when this changes for a newer kernel version. I think this makes the code error-prone and hard to read. Its much better to replace it with: /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) { info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); return TRUE; } else { .. } I think it will reduce future reworks (as per kernel versions) and also reduce issues while backporting makedumpfile to older kernels. What do you think? Regards, Bhupesh > > I recently posted a kernel patch (see [0]) which appends > > 'MAX_PHYSMEM_BITS' to vmcoreinfo in the core code itself rather than > > in arch-specific code, so that user-space code can also benefit from > > this addition to the vmcoreinfo and use it as a standard way of > > determining 'SECTIONS_SHIFT' value in 'makedumpfile' utility. > > > > This patch ensures backward compatibility for kernel versions in which > > 'MAX_PHYSMEM_BITS' is not available in vmcoreinfo. > > > > [0]. http://lists.infradead.org/pipermail/kexec/2019-November/023960.html > > > > Cc: Kazuhito Hagio > > Cc: John Donnelly > > Cc: kexec@lists.infradead.org > > Signed-off-by: Bhupesh Sharma > > --- > > arch/arm.c | 8 +++- > > arch/ia64.c| 7 ++- > > arch/ppc.c | 8 +++- > > arch/ppc64.c | 49 - > > arch/s390x.c | 29 ++--- > > arch/sparc64.c | 9 +++-- > > arch/x86.c | 34 -- > > arch/x86_64.c | 27 --- > > 8 files changed, 109 insertions(+), 62 deletions(-) > > > > diff --git a/arch/arm.c b/arch/arm.c > > index af7442ac70bf..33536fc4dfc9 100644 > > --- a/arch/arm.c > > +++ b/arch/arm.c > > @@ -81,7 +81,13 @@ int > > get_machdep_info_arm(void) > > { > > info->page_offset = SYMBOL(_stext) & 0xUL; > > - info->max_physmem_bits = _MAX_PHYSMEM_BITS; > > + > > + /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ > > + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) > > + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); > > + else > > + info->max_physmem_bits = _MAX_PHYSMEM_BITS; > > + > > info->kernel_start = SYMBOL(_stext); > > info->section_size_bits = _SECTION_SIZE_BITS; > > > > diff --git a/arch/ia64.c b/arch/ia64.c > > index 6c33cc7c8288..fb44dda47172 100644 > > --- a/arch/ia64.c > > +++ b/arch/ia64.c > > @@ -85,7 +85,12 @@ get_machdep_info_ia64(void) > > } > > > > info->section_size_bits = _SECTION_SIZE_BITS; > > - info->max_physmem_bits = _MAX_PHYSMEM_BITS; > > + > > + /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ > > + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) > > + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); > > +
RE: [PATCH v4 1/4] tree-wide: Retrieve 'MAX_PHYSMEM_BITS' from vmcoreinfo (if available)
Hi Bhupesh, Sorry for the late reply. > -Original Message- > This patch adds a common feature for archs (except arm64, for which > similar support is added via subsequent patch) to retrieve > 'MAX_PHYSMEM_BITS' from vmcoreinfo (if available). We already have the calibrate_machdep_info() function, which sets info->max_physmem_bits from vmcoreinfo, so practically we don't need to add this patch for the benefit. Thanks, Kazu > > I recently posted a kernel patch (see [0]) which appends > 'MAX_PHYSMEM_BITS' to vmcoreinfo in the core code itself rather than > in arch-specific code, so that user-space code can also benefit from > this addition to the vmcoreinfo and use it as a standard way of > determining 'SECTIONS_SHIFT' value in 'makedumpfile' utility. > > This patch ensures backward compatibility for kernel versions in which > 'MAX_PHYSMEM_BITS' is not available in vmcoreinfo. > > [0]. http://lists.infradead.org/pipermail/kexec/2019-November/023960.html > > Cc: Kazuhito Hagio > Cc: John Donnelly > Cc: kexec@lists.infradead.org > Signed-off-by: Bhupesh Sharma > --- > arch/arm.c | 8 +++- > arch/ia64.c| 7 ++- > arch/ppc.c | 8 +++- > arch/ppc64.c | 49 - > arch/s390x.c | 29 ++--- > arch/sparc64.c | 9 +++-- > arch/x86.c | 34 -- > arch/x86_64.c | 27 --- > 8 files changed, 109 insertions(+), 62 deletions(-) > > diff --git a/arch/arm.c b/arch/arm.c > index af7442ac70bf..33536fc4dfc9 100644 > --- a/arch/arm.c > +++ b/arch/arm.c > @@ -81,7 +81,13 @@ int > get_machdep_info_arm(void) > { > info->page_offset = SYMBOL(_stext) & 0xUL; > - info->max_physmem_bits = _MAX_PHYSMEM_BITS; > + > + /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ > + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) > + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); > + else > + info->max_physmem_bits = _MAX_PHYSMEM_BITS; > + > info->kernel_start = SYMBOL(_stext); > info->section_size_bits = _SECTION_SIZE_BITS; > > diff --git a/arch/ia64.c b/arch/ia64.c > index 6c33cc7c8288..fb44dda47172 100644 > --- a/arch/ia64.c > +++ b/arch/ia64.c > @@ -85,7 +85,12 @@ get_machdep_info_ia64(void) > } > > info->section_size_bits = _SECTION_SIZE_BITS; > - info->max_physmem_bits = _MAX_PHYSMEM_BITS; > + > + /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ > + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) > + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); > + else > + info->max_physmem_bits = _MAX_PHYSMEM_BITS; > > return TRUE; > } > diff --git a/arch/ppc.c b/arch/ppc.c > index 37c6a3b60cd3..ed9447427a30 100644 > --- a/arch/ppc.c > +++ b/arch/ppc.c > @@ -31,7 +31,13 @@ get_machdep_info_ppc(void) > unsigned long vmlist, vmap_area_list, vmalloc_start; > > info->section_size_bits = _SECTION_SIZE_BITS; > - info->max_physmem_bits = _MAX_PHYSMEM_BITS; > + > + /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ > + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) > + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); > + else > + info->max_physmem_bits = _MAX_PHYSMEM_BITS; > + > info->page_offset = __PAGE_OFFSET; > > if (SYMBOL(_stext) != NOT_FOUND_SYMBOL) > diff --git a/arch/ppc64.c b/arch/ppc64.c > index 9d8f2525f608..a3984eebdced 100644 > --- a/arch/ppc64.c > +++ b/arch/ppc64.c > @@ -466,30 +466,37 @@ int > set_ppc64_max_physmem_bits(void) > { > long array_len = ARRAY_LENGTH(mem_section); > - /* > - * The older ppc64 kernels uses _MAX_PHYSMEM_BITS as 42 and the > - * newer kernels 3.7 onwards uses 46 bits. > - */ > - > - info->max_physmem_bits = _MAX_PHYSMEM_BITS_ORIG ; > - if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) > - || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( > - return TRUE; > - > - info->max_physmem_bits = _MAX_PHYSMEM_BITS_3_7; > - if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) > - || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( > - return TRUE; > > - info->max_physmem_bits = _MAX_PHYSMEM_BITS_4_19; > - if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) > - || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( > + /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ > + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) { > + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); > return TRUE; > + } else { > + /* > + * The older ppc64 kernels uses _MAX_PHYSMEM_BITS as 42 and the > + * newer kernels 3.7 onwards uses 46 bits. > +
[PATCH v4 1/4] tree-wide: Retrieve 'MAX_PHYSMEM_BITS' from vmcoreinfo (if available)
This patch adds a common feature for archs (except arm64, for which similar support is added via subsequent patch) to retrieve 'MAX_PHYSMEM_BITS' from vmcoreinfo (if available). I recently posted a kernel patch (see [0]) which appends 'MAX_PHYSMEM_BITS' to vmcoreinfo in the core code itself rather than in arch-specific code, so that user-space code can also benefit from this addition to the vmcoreinfo and use it as a standard way of determining 'SECTIONS_SHIFT' value in 'makedumpfile' utility. This patch ensures backward compatibility for kernel versions in which 'MAX_PHYSMEM_BITS' is not available in vmcoreinfo. [0]. http://lists.infradead.org/pipermail/kexec/2019-November/023960.html Cc: Kazuhito Hagio Cc: John Donnelly Cc: kexec@lists.infradead.org Signed-off-by: Bhupesh Sharma --- arch/arm.c | 8 +++- arch/ia64.c| 7 ++- arch/ppc.c | 8 +++- arch/ppc64.c | 49 - arch/s390x.c | 29 ++--- arch/sparc64.c | 9 +++-- arch/x86.c | 34 -- arch/x86_64.c | 27 --- 8 files changed, 109 insertions(+), 62 deletions(-) diff --git a/arch/arm.c b/arch/arm.c index af7442ac70bf..33536fc4dfc9 100644 --- a/arch/arm.c +++ b/arch/arm.c @@ -81,7 +81,13 @@ int get_machdep_info_arm(void) { info->page_offset = SYMBOL(_stext) & 0xUL; - info->max_physmem_bits = _MAX_PHYSMEM_BITS; + + /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); + else + info->max_physmem_bits = _MAX_PHYSMEM_BITS; + info->kernel_start = SYMBOL(_stext); info->section_size_bits = _SECTION_SIZE_BITS; diff --git a/arch/ia64.c b/arch/ia64.c index 6c33cc7c8288..fb44dda47172 100644 --- a/arch/ia64.c +++ b/arch/ia64.c @@ -85,7 +85,12 @@ get_machdep_info_ia64(void) } info->section_size_bits = _SECTION_SIZE_BITS; - info->max_physmem_bits = _MAX_PHYSMEM_BITS; + + /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); + else + info->max_physmem_bits = _MAX_PHYSMEM_BITS; return TRUE; } diff --git a/arch/ppc.c b/arch/ppc.c index 37c6a3b60cd3..ed9447427a30 100644 --- a/arch/ppc.c +++ b/arch/ppc.c @@ -31,7 +31,13 @@ get_machdep_info_ppc(void) unsigned long vmlist, vmap_area_list, vmalloc_start; info->section_size_bits = _SECTION_SIZE_BITS; - info->max_physmem_bits = _MAX_PHYSMEM_BITS; + + /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); + else + info->max_physmem_bits = _MAX_PHYSMEM_BITS; + info->page_offset = __PAGE_OFFSET; if (SYMBOL(_stext) != NOT_FOUND_SYMBOL) diff --git a/arch/ppc64.c b/arch/ppc64.c index 9d8f2525f608..a3984eebdced 100644 --- a/arch/ppc64.c +++ b/arch/ppc64.c @@ -466,30 +466,37 @@ int set_ppc64_max_physmem_bits(void) { long array_len = ARRAY_LENGTH(mem_section); - /* -* The older ppc64 kernels uses _MAX_PHYSMEM_BITS as 42 and the -* newer kernels 3.7 onwards uses 46 bits. -*/ - - info->max_physmem_bits = _MAX_PHYSMEM_BITS_ORIG ; - if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) - || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( - return TRUE; - - info->max_physmem_bits = _MAX_PHYSMEM_BITS_3_7; - if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) - || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( - return TRUE; - info->max_physmem_bits = _MAX_PHYSMEM_BITS_4_19; - if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) - || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( + /* Check if we can get MAX_PHYSMEM_BITS from vmcoreinfo */ + if (NUMBER(MAX_PHYSMEM_BITS) != NOT_FOUND_NUMBER) { + info->max_physmem_bits = NUMBER(MAX_PHYSMEM_BITS); return TRUE; + } else { + /* +* The older ppc64 kernels uses _MAX_PHYSMEM_BITS as 42 and the +* newer kernels 3.7 onwards uses 46 bits. +*/ - info->max_physmem_bits = _MAX_PHYSMEM_BITS_4_20; - if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT_EXTREME())) - || (array_len == (NR_MEM_SECTIONS() / _SECTIONS_PER_ROOT( - return TRUE; + info->max_physmem_bits = _MAX_PHYSMEM_BITS_ORIG ; + if ((array_len == (NR_MEM_SECTIONS() / _SECTIONS_PE