Re: [PATCH makedumpfile] handle mem_section as either a pointer or an array

2018-05-14 Thread Thadeu Lima de Souza Cascardo 
On Mon, May 14, 2018 at 07:40:21AM +, Masaki Tachibana wrote:
> Hi Thadeu,
> 
> So sorry for the late reply.
> I will merge the patch into V1.6.4 with the following modifying 
> because I have already accepted "Always use bigger SECTION_MAP_MASK" patch.
> 
> 63,68c63,64
> <   if (info->kernel_version < KERNEL_VERSION(4, 13, 0))
> < - map &= SECTION_MAP_MASK_4_12;
> < + mask = SECTION_MAP_MASK_4_12;
> <   else
> < - map &= SECTION_MAP_MASK;
> < + mask = SECTION_MAP_MASK;
> ---
> > - map &= SECTION_MAP_MASK;
> > + mask = SECTION_MAP_MASK;
> 

Ack. That should be enough for a fixup.
Thanks.
Cascardo.

> 
> Thanks
> Tachibana
> 
> > -Original Message-
> > From: kexec [mailto:kexec-boun...@lists.infradead.org] On Behalf Of Masaki 
> > Tachibana
> > Sent: Monday, March 05, 2018 6:16 PM
> > To: Thadeu Lima de Souza Cascardo 
> > Cc: Hayashi Masahiko() ; 
> > kexec@lists.infradead.org
> > Subject: RE: [PATCH makedumpfile] handle mem_section as either a pointer or 
> > an array
> > 
> > Hi Thadeu,
> > 
> > Sorry for the late reply.
> > "handle mem_section as either a pointer or an array" patch and
> > "Always use bigger SECTION_MAP_MASK" patch modify the same line.
> > I would like to reply about both patches by the end of the next week.
> > 
> > 
> > Thanks
> > Tachibana
> > 
> > 
> > > -Original Message-
> > > From: kexec [mailto:kexec-boun...@lists.infradead.org] On Behalf Of 
> > > Thadeu Lima de Souza Cascardo
> > > Sent: Friday, March 02, 2018 11:33 PM
> > > To: kexec@lists.infradead.org
> > > Subject: Re: [PATCH makedumpfile] handle mem_section as either a pointer 
> > > or an array
> > >
> > > Any comments or reviews on the patch below?
> > >
> > > Thanks.
> > > Cascardo.
> > >
> > > On Mon, Feb 19, 2018 at 05:04:59PM -0300, Thadeu Lima de Souza Cascardo 
> > > wrote:
> > > > Some kernel versions that have been recently shipped have mem_section 
> > > > point to
> > > > a pointer to the array, instead of pointing directly to the array. That 
> > > > only
> > > > happens on SPARSEMEM_EXTREME configurations.
> > > >
> > > > As dwarf information might not be present that would have allowed us to 
> > > > detect
> > > > which type it is, we need to try it either as an array or as the 
> > > > pointer to the
> > > > array. Then, we validate all section_mem_map: they must either be 
> > > > present or
> > > > null. If any problems are found when traversing it, consider it 
> > > > invalid. Only
> > > > one way may be valid. Otherwise, fail.
> > > >
> > > > This has been tested with both types of kernels and succeeded in 
> > > > producing a
> > > > compressed dump that could be analyzed with crash 7.2.1.
> > > >
> > > > Signed-off-by: Thadeu Lima de Souza Cascardo 
> > > > ---
> > > >  makedumpfile.c | 153 
> > > > +++--
> > > >  makedumpfile.h |   1 +
> > > >  2 files changed, 118 insertions(+), 36 deletions(-)
> > > >
> > > > diff --git a/makedumpfile.c b/makedumpfile.c
> > > > index ed138d3..cd3fa4d 100644
> > > > --- a/makedumpfile.c
> > > > +++ b/makedumpfile.c
> > > > @@ -3297,7 +3297,7 @@ get_mm_discontigmem(void)
> > > > return TRUE;
> > > >  }
> > > >
> > > > -unsigned long
> > > > +static unsigned long
> > > >  nr_to_section(unsigned long nr, unsigned long *mem_sec)
> > > >  {
> > > > unsigned long addr;
> > > > @@ -3311,17 +3311,17 @@ nr_to_section(unsigned long nr, unsigned long 
> > > > *mem_sec)
> > > > addr = SYMBOL(mem_section) + (nr * SIZE(mem_section));
> > > > }
> > > >
> > > > -   if (!is_kvaddr(addr))
> > > > -   return NOT_KV_ADDR;
> > > > -
> > > > return addr;
> > > >  }
> > > >
> > > > -unsigned long
> > > > -section_mem_map_addr(unsigned long addr)
> > > > +static unsigned long
> > > > +section_mem_map_addr(unsigned long addr, unsigned long *map_mask)
> > > >  {
> > > > char *mem_section;
> > > > unsigned long map;
> > > > +   unsigned long mask;
> > > > +
> > > > +   *map_mask = 0;
> > > >
> > > > if (!is_kvaddr(addr))
> > > > return NOT_KV_ADDR;
> > > > @@ -3338,15 +3338,19 @@ section_mem_map_addr(unsigned long addr)
> > > > }
> > > > map = ULONG(mem_section + OFFSET(mem_section.section_mem_map));
> > > > if (info->kernel_version < KERNEL_VERSION(4, 13, 0))
> > > > -   map &= SECTION_MAP_MASK_4_12;
> > > > +   mask = SECTION_MAP_MASK_4_12;
> > > > else
> > > > -   map &= SECTION_MAP_MASK;
> > > > +   mask = SECTION_MAP_MASK;
> > > > +   *map_mask = map & ~mask;
> > > > +   if (map == 0x0)
> > > > +   *map_mask |= SECTION_MARKED_PRESENT;
> > > > +   map &= mask;
> > > > free(mem_section);
> > > >
> > > > return map;
> > > >  }
> > > >
> > > >

RE: [PATCH makedumpfile] handle mem_section as either a pointer or an array

2018-05-14 Thread Masaki Tachibana 
Hi Thadeu,

So sorry for the late reply.
I will merge the patch into V1.6.4 with the following modifying 
because I have already accepted "Always use bigger SECTION_MAP_MASK" patch.

63,68c63,64
<   if (info->kernel_version < KERNEL_VERSION(4, 13, 0))
< - map &= SECTION_MAP_MASK_4_12;
< + mask = SECTION_MAP_MASK_4_12;
<   else
< - map &= SECTION_MAP_MASK;
< + mask = SECTION_MAP_MASK;
---
> - map &= SECTION_MAP_MASK;
> + mask = SECTION_MAP_MASK;


Thanks
Tachibana

> -Original Message-
> From: kexec [mailto:kexec-boun...@lists.infradead.org] On Behalf Of Masaki 
> Tachibana
> Sent: Monday, March 05, 2018 6:16 PM
> To: Thadeu Lima de Souza Cascardo 
> Cc: Hayashi Masahiko() ; kexec@lists.infradead.org
> Subject: RE: [PATCH makedumpfile] handle mem_section as either a pointer or 
> an array
> 
> Hi Thadeu,
> 
> Sorry for the late reply.
> "handle mem_section as either a pointer or an array" patch and
> "Always use bigger SECTION_MAP_MASK" patch modify the same line.
> I would like to reply about both patches by the end of the next week.
> 
> 
> Thanks
> Tachibana
> 
> 
> > -Original Message-
> > From: kexec [mailto:kexec-boun...@lists.infradead.org] On Behalf Of Thadeu 
> > Lima de Souza Cascardo
> > Sent: Friday, March 02, 2018 11:33 PM
> > To: kexec@lists.infradead.org
> > Subject: Re: [PATCH makedumpfile] handle mem_section as either a pointer or 
> > an array
> >
> > Any comments or reviews on the patch below?
> >
> > Thanks.
> > Cascardo.
> >
> > On Mon, Feb 19, 2018 at 05:04:59PM -0300, Thadeu Lima de Souza Cascardo 
> > wrote:
> > > Some kernel versions that have been recently shipped have mem_section 
> > > point to
> > > a pointer to the array, instead of pointing directly to the array. That 
> > > only
> > > happens on SPARSEMEM_EXTREME configurations.
> > >
> > > As dwarf information might not be present that would have allowed us to 
> > > detect
> > > which type it is, we need to try it either as an array or as the pointer 
> > > to the
> > > array. Then, we validate all section_mem_map: they must either be present 
> > > or
> > > null. If any problems are found when traversing it, consider it invalid. 
> > > Only
> > > one way may be valid. Otherwise, fail.
> > >
> > > This has been tested with both types of kernels and succeeded in 
> > > producing a
> > > compressed dump that could be analyzed with crash 7.2.1.
> > >
> > > Signed-off-by: Thadeu Lima de Souza Cascardo 
> > > ---
> > >  makedumpfile.c | 153 
> > > +++--
> > >  makedumpfile.h |   1 +
> > >  2 files changed, 118 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/makedumpfile.c b/makedumpfile.c
> > > index ed138d3..cd3fa4d 100644
> > > --- a/makedumpfile.c
> > > +++ b/makedumpfile.c
> > > @@ -3297,7 +3297,7 @@ get_mm_discontigmem(void)
> > >   return TRUE;
> > >  }
> > >
> > > -unsigned long
> > > +static unsigned long
> > >  nr_to_section(unsigned long nr, unsigned long *mem_sec)
> > >  {
> > >   unsigned long addr;
> > > @@ -3311,17 +3311,17 @@ nr_to_section(unsigned long nr, unsigned long 
> > > *mem_sec)
> > >   addr = SYMBOL(mem_section) + (nr * SIZE(mem_section));
> > >   }
> > >
> > > - if (!is_kvaddr(addr))
> > > - return NOT_KV_ADDR;
> > > -
> > >   return addr;
> > >  }
> > >
> > > -unsigned long
> > > -section_mem_map_addr(unsigned long addr)
> > > +static unsigned long
> > > +section_mem_map_addr(unsigned long addr, unsigned long *map_mask)
> > >  {
> > >   char *mem_section;
> > >   unsigned long map;
> > > + unsigned long mask;
> > > +
> > > + *map_mask = 0;
> > >
> > >   if (!is_kvaddr(addr))
> > >   return NOT_KV_ADDR;
> > > @@ -3338,15 +3338,19 @@ section_mem_map_addr(unsigned long addr)
> > >   }
> > >   map = ULONG(mem_section + OFFSET(mem_section.section_mem_map));
> > >   if (info->kernel_version < KERNEL_VERSION(4, 13, 0))
> > > - map &= SECTION_MAP_MASK_4_12;
> > > + mask = SECTION_MAP_MASK_4_12;
> > >   else
> > > - map &= SECTION_MAP_MASK;
> > > + mask = SECTION_MAP_MASK;
> > > + *map_mask = map & ~mask;
> > > + if (map == 0x0)
> > > + *map_mask |= SECTION_MARKED_PRESENT;
> > > + map &= mask;
> > >   free(mem_section);
> > >
> > >   return map;
> > >  }
> > >
> > > -unsigned long
> > > +static unsigned long
> > >  sparse_decode_mem_map(unsigned long coded_mem_map, unsigned long 
> > > section_nr)
> > >  {
> > >   unsigned long mem_map;
> > > @@ -3354,17 +3358,110 @@ sparse_decode_mem_map(unsigned long 
> > > coded_mem_map, unsigned long section_nr)
> > >   mem_map =  coded_mem_map +
> > >   (SECTION_NR_TO_PFN(section_nr) * SIZE(page));
> > >
> > > - if (!is_kvaddr(mem_map))
> > > - return NOT_KV_ADDR;
> > >   return mem_map;
> > >  }
> > > +
> > > +/*
> > > + * On some kernels, mem_section may be a pointer or an array, when

RE: [PATCH makedumpfile] handle mem_section as either a pointer or an array

2018-03-05 Thread Masaki Tachibana 
Hi Thadeu,

Sorry for the late reply.
"handle mem_section as either a pointer or an array" patch and
"Always use bigger SECTION_MAP_MASK" patch modify the same line.
I would like to reply about both patches by the end of the next week.


Thanks
Tachibana


> -Original Message-
> From: kexec [mailto:kexec-boun...@lists.infradead.org] On Behalf Of Thadeu 
> Lima de Souza Cascardo
> Sent: Friday, March 02, 2018 11:33 PM
> To: kexec@lists.infradead.org
> Subject: Re: [PATCH makedumpfile] handle mem_section as either a pointer or 
> an array
> 
> Any comments or reviews on the patch below?
> 
> Thanks.
> Cascardo.
> 
> On Mon, Feb 19, 2018 at 05:04:59PM -0300, Thadeu Lima de Souza Cascardo wrote:
> > Some kernel versions that have been recently shipped have mem_section point 
> > to
> > a pointer to the array, instead of pointing directly to the array. That only
> > happens on SPARSEMEM_EXTREME configurations.
> >
> > As dwarf information might not be present that would have allowed us to 
> > detect
> > which type it is, we need to try it either as an array or as the pointer to 
> > the
> > array. Then, we validate all section_mem_map: they must either be present or
> > null. If any problems are found when traversing it, consider it invalid. 
> > Only
> > one way may be valid. Otherwise, fail.
> >
> > This has been tested with both types of kernels and succeeded in producing a
> > compressed dump that could be analyzed with crash 7.2.1.
> >
> > Signed-off-by: Thadeu Lima de Souza Cascardo 
> > ---
> >  makedumpfile.c | 153 
> > +++--
> >  makedumpfile.h |   1 +
> >  2 files changed, 118 insertions(+), 36 deletions(-)
> >
> > diff --git a/makedumpfile.c b/makedumpfile.c
> > index ed138d3..cd3fa4d 100644
> > --- a/makedumpfile.c
> > +++ b/makedumpfile.c
> > @@ -3297,7 +3297,7 @@ get_mm_discontigmem(void)
> > return TRUE;
> >  }
> >
> > -unsigned long
> > +static unsigned long
> >  nr_to_section(unsigned long nr, unsigned long *mem_sec)
> >  {
> > unsigned long addr;
> > @@ -3311,17 +3311,17 @@ nr_to_section(unsigned long nr, unsigned long 
> > *mem_sec)
> > addr = SYMBOL(mem_section) + (nr * SIZE(mem_section));
> > }
> >
> > -   if (!is_kvaddr(addr))
> > -   return NOT_KV_ADDR;
> > -
> > return addr;
> >  }
> >
> > -unsigned long
> > -section_mem_map_addr(unsigned long addr)
> > +static unsigned long
> > +section_mem_map_addr(unsigned long addr, unsigned long *map_mask)
> >  {
> > char *mem_section;
> > unsigned long map;
> > +   unsigned long mask;
> > +
> > +   *map_mask = 0;
> >
> > if (!is_kvaddr(addr))
> > return NOT_KV_ADDR;
> > @@ -3338,15 +3338,19 @@ section_mem_map_addr(unsigned long addr)
> > }
> > map = ULONG(mem_section + OFFSET(mem_section.section_mem_map));
> > if (info->kernel_version < KERNEL_VERSION(4, 13, 0))
> > -   map &= SECTION_MAP_MASK_4_12;
> > +   mask = SECTION_MAP_MASK_4_12;
> > else
> > -   map &= SECTION_MAP_MASK;
> > +   mask = SECTION_MAP_MASK;
> > +   *map_mask = map & ~mask;
> > +   if (map == 0x0)
> > +   *map_mask |= SECTION_MARKED_PRESENT;
> > +   map &= mask;
> > free(mem_section);
> >
> > return map;
> >  }
> >
> > -unsigned long
> > +static unsigned long
> >  sparse_decode_mem_map(unsigned long coded_mem_map, unsigned long 
> > section_nr)
> >  {
> > unsigned long mem_map;
> > @@ -3354,17 +3358,110 @@ sparse_decode_mem_map(unsigned long coded_mem_map, 
> > unsigned long section_nr)
> > mem_map =  coded_mem_map +
> > (SECTION_NR_TO_PFN(section_nr) * SIZE(page));
> >
> > -   if (!is_kvaddr(mem_map))
> > -   return NOT_KV_ADDR;
> > return mem_map;
> >  }
> > +
> > +/*
> > + * On some kernels, mem_section may be a pointer or an array, when
> > + * SPARSEMEM_EXTREME is on.
> > + *
> > + * We assume that section_mem_map is either 0 or has the present bit set.
> > + *
> > + */
> > +
> > +static int
> > +validate_mem_section(unsigned long *mem_sec,
> > +unsigned long mem_section_ptr, unsigned int 
> > mem_section_size,
> > +unsigned long *mem_maps, unsigned int num_section)
> > +{
> > +   unsigned int section_nr;
> > +   unsigned long map_mask;
> > +   unsigned long section, mem_map;
> > +   if (!readmem(VADDR, mem_section_ptr, mem_sec, mem_section_size)) {
> > +   ERRMSG("Can't read mem_section array.\n");
> > +   return FALSE;
> > +   }
> > +   for (section_nr = 0; section_nr < num_section; section_nr++) {
> > +   section = nr_to_section(section_nr, mem_sec);
> > +   if (section == NOT_KV_ADDR) {
> > +   mem_map = NOT_MEMMAP_ADDR;
> > +   } else {
> > +   mem_map = section_mem_map_addr(section, _mask);
> > +   if (!(map_mask & SECTION_MARKED_PRESENT)) {
> > +   return FALSE;
> > +   }
> > +

Re: [PATCH makedumpfile] handle mem_section as either a pointer or an array

2018-03-02 Thread Thadeu Lima de Souza Cascardo 
Any comments or reviews on the patch below?

Thanks.
Cascardo.

On Mon, Feb 19, 2018 at 05:04:59PM -0300, Thadeu Lima de Souza Cascardo wrote:
> Some kernel versions that have been recently shipped have mem_section point to
> a pointer to the array, instead of pointing directly to the array. That only
> happens on SPARSEMEM_EXTREME configurations.
> 
> As dwarf information might not be present that would have allowed us to detect
> which type it is, we need to try it either as an array or as the pointer to 
> the
> array. Then, we validate all section_mem_map: they must either be present or
> null. If any problems are found when traversing it, consider it invalid. Only
> one way may be valid. Otherwise, fail.
> 
> This has been tested with both types of kernels and succeeded in producing a
> compressed dump that could be analyzed with crash 7.2.1.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo 
> ---
>  makedumpfile.c | 153 
> +++--
>  makedumpfile.h |   1 +
>  2 files changed, 118 insertions(+), 36 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index ed138d3..cd3fa4d 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -3297,7 +3297,7 @@ get_mm_discontigmem(void)
>   return TRUE;
>  }
>  
> -unsigned long
> +static unsigned long
>  nr_to_section(unsigned long nr, unsigned long *mem_sec)
>  {
>   unsigned long addr;
> @@ -3311,17 +3311,17 @@ nr_to_section(unsigned long nr, unsigned long 
> *mem_sec)
>   addr = SYMBOL(mem_section) + (nr * SIZE(mem_section));
>   }
>  
> - if (!is_kvaddr(addr))
> - return NOT_KV_ADDR;
> -
>   return addr;
>  }
>  
> -unsigned long
> -section_mem_map_addr(unsigned long addr)
> +static unsigned long
> +section_mem_map_addr(unsigned long addr, unsigned long *map_mask)
>  {
>   char *mem_section;
>   unsigned long map;
> + unsigned long mask;
> +
> + *map_mask = 0;
>  
>   if (!is_kvaddr(addr))
>   return NOT_KV_ADDR;
> @@ -3338,15 +3338,19 @@ section_mem_map_addr(unsigned long addr)
>   }
>   map = ULONG(mem_section + OFFSET(mem_section.section_mem_map));
>   if (info->kernel_version < KERNEL_VERSION(4, 13, 0))
> - map &= SECTION_MAP_MASK_4_12;
> + mask = SECTION_MAP_MASK_4_12;
>   else
> - map &= SECTION_MAP_MASK;
> + mask = SECTION_MAP_MASK;
> + *map_mask = map & ~mask;
> + if (map == 0x0)
> + *map_mask |= SECTION_MARKED_PRESENT;
> + map &= mask;
>   free(mem_section);
>  
>   return map;
>  }
>  
> -unsigned long
> +static unsigned long
>  sparse_decode_mem_map(unsigned long coded_mem_map, unsigned long section_nr)
>  {
>   unsigned long mem_map;
> @@ -3354,17 +3358,110 @@ sparse_decode_mem_map(unsigned long coded_mem_map, 
> unsigned long section_nr)
>   mem_map =  coded_mem_map +
>   (SECTION_NR_TO_PFN(section_nr) * SIZE(page));
>  
> - if (!is_kvaddr(mem_map))
> - return NOT_KV_ADDR;
>   return mem_map;
>  }
> +
> +/*
> + * On some kernels, mem_section may be a pointer or an array, when
> + * SPARSEMEM_EXTREME is on.
> + *
> + * We assume that section_mem_map is either 0 or has the present bit set.
> + *
> + */
> +
> +static int
> +validate_mem_section(unsigned long *mem_sec,
> +  unsigned long mem_section_ptr, unsigned int 
> mem_section_size,
> +  unsigned long *mem_maps, unsigned int num_section)
> +{
> + unsigned int section_nr;
> + unsigned long map_mask;
> + unsigned long section, mem_map;
> + if (!readmem(VADDR, mem_section_ptr, mem_sec, mem_section_size)) {
> + ERRMSG("Can't read mem_section array.\n");
> + return FALSE;
> + }
> + for (section_nr = 0; section_nr < num_section; section_nr++) {
> + section = nr_to_section(section_nr, mem_sec);
> + if (section == NOT_KV_ADDR) {
> + mem_map = NOT_MEMMAP_ADDR;
> + } else {
> + mem_map = section_mem_map_addr(section, _mask);
> + if (!(map_mask & SECTION_MARKED_PRESENT)) {
> + return FALSE;
> + }
> + if (mem_map == 0) {
> + mem_map = NOT_MEMMAP_ADDR;
> + } else {
> + mem_map = sparse_decode_mem_map(mem_map,
> + section_nr);
> + if (!is_kvaddr(mem_map)) {
> + return FALSE;
> + }
> + }
> + }
> + mem_maps[section_nr] = mem_map;
> + }
> + return TRUE;
> +}
> +
> +static int
> +get_mem_section(unsigned int mem_section_size, unsigned long *mem_maps,
> + unsigned int num_section)
> +{
> +