Re: [PATCH 06/15] powerpc: fadamp: simplify fadump_reserve_crash_area()

2020-08-02 Thread Michael Ellerman
Mike Rapoport  writes:
> On Thu, Jul 30, 2020 at 10:15:13PM +1000, Michael Ellerman wrote:
>> Mike Rapoport  writes:
>> > From: Mike Rapoport 
>> >
>> > fadump_reserve_crash_area() reserves memory from a specified base address
>> > till the end of the RAM.
>> >
>> > Replace iteration through the memblock.memory with a single call to
>> > memblock_reserve() with appropriate  that will take care of proper memory
>>  ^
>>  parameters?
>> > reservation.
>> >
>> > Signed-off-by: Mike Rapoport 
>> > ---
>> >  arch/powerpc/kernel/fadump.c | 20 +---
>> >  1 file changed, 1 insertion(+), 19 deletions(-)
>> 
>> I think this looks OK to me, but I don't have a setup to test it easily.
>> I've added Hari to Cc who might be able to.
>> 
>> But I'll give you an ack in the hope that it works :)
>
> Actually, I did some digging in the git log and the traversal was added
> there on purpose by the commit b71a693d3db3 ("powerpc/fadump: exclude
> memory holes while reserving memory in second kernel")
> Presuming this is still reqruired I'm going to drop this patch and will
> simply replace for_each_memblock() with for_each_mem_range() in v2.

Thanks.

cheers


Re: [PATCH 06/15] powerpc: fadamp: simplify fadump_reserve_crash_area()

2020-08-01 Thread Hari Bathini




On 01/08/20 3:48 pm, Mike Rapoport wrote:

On Thu, Jul 30, 2020 at 10:15:13PM +1000, Michael Ellerman wrote:

Mike Rapoport  writes:

From: Mike Rapoport 

fadump_reserve_crash_area() reserves memory from a specified base address
till the end of the RAM.

Replace iteration through the memblock.memory with a single call to
memblock_reserve() with appropriate  that will take care of proper memory

  ^
  parameters?

reservation.

Signed-off-by: Mike Rapoport 
---
  arch/powerpc/kernel/fadump.c | 20 +---
  1 file changed, 1 insertion(+), 19 deletions(-)


I think this looks OK to me, but I don't have a setup to test it easily.
I've added Hari to Cc who might be able to.

But I'll give you an ack in the hope that it works :)


Actually, I did some digging in the git log and the traversal was added
there on purpose by the commit b71a693d3db3 ("powerpc/fadump: exclude
memory holes while reserving memory in second kernel")


I was about to comment on the same :)
memblock_reserve() was being used until we ran into the issue talked 
about in the above commit...



Presuming this is still reqruired I'm going to drop this patch and will


Yeah, it is still required..


simply replace for_each_memblock() with for_each_mem_range() in v2.


Sounds right.

  

Acked-by: Michael Ellerman 



diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 78ab9a6ee6ac..2446a61e3c25 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -1658,25 +1658,7 @@ int __init fadump_reserve_mem(void)
  /* Preserve everything above the base address */
  static void __init fadump_reserve_crash_area(u64 base)
  {
-   struct memblock_region *reg;
-   u64 mstart, msize;
-
-   for_each_memblock(memory, reg) {
-   mstart = reg->base;
-   msize  = reg->size;
-
-   if ((mstart + msize) < base)
-   continue;
-
-   if (mstart < base) {
-   msize -= (base - mstart);
-   mstart = base;
-   }
-
-   pr_info("Reserving %lluMB of memory at %#016llx for preserving crash 
data",
-   (msize >> 20), mstart);
-   memblock_reserve(mstart, msize);
-   }
+   memblock_reserve(base, memblock_end_of_DRAM() - base);
  }
  
  unsigned long __init arch_reserved_kernel_pages(void)

--
2.26.2




Thanks
Hari


Re: [PATCH 06/15] powerpc: fadamp: simplify fadump_reserve_crash_area()

2020-08-01 Thread Mike Rapoport
On Thu, Jul 30, 2020 at 10:15:13PM +1000, Michael Ellerman wrote:
> Mike Rapoport  writes:
> > From: Mike Rapoport 
> >
> > fadump_reserve_crash_area() reserves memory from a specified base address
> > till the end of the RAM.
> >
> > Replace iteration through the memblock.memory with a single call to
> > memblock_reserve() with appropriate  that will take care of proper memory
>  ^
>  parameters?
> > reservation.
> >
> > Signed-off-by: Mike Rapoport 
> > ---
> >  arch/powerpc/kernel/fadump.c | 20 +---
> >  1 file changed, 1 insertion(+), 19 deletions(-)
> 
> I think this looks OK to me, but I don't have a setup to test it easily.
> I've added Hari to Cc who might be able to.
> 
> But I'll give you an ack in the hope that it works :)

Actually, I did some digging in the git log and the traversal was added
there on purpose by the commit b71a693d3db3 ("powerpc/fadump: exclude
memory holes while reserving memory in second kernel")
Presuming this is still reqruired I'm going to drop this patch and will
simply replace for_each_memblock() with for_each_mem_range() in v2.
 
> Acked-by: Michael Ellerman 
> 
> 
> > diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> > index 78ab9a6ee6ac..2446a61e3c25 100644
> > --- a/arch/powerpc/kernel/fadump.c
> > +++ b/arch/powerpc/kernel/fadump.c
> > @@ -1658,25 +1658,7 @@ int __init fadump_reserve_mem(void)
> >  /* Preserve everything above the base address */
> >  static void __init fadump_reserve_crash_area(u64 base)
> >  {
> > -   struct memblock_region *reg;
> > -   u64 mstart, msize;
> > -
> > -   for_each_memblock(memory, reg) {
> > -   mstart = reg->base;
> > -   msize  = reg->size;
> > -
> > -   if ((mstart + msize) < base)
> > -   continue;
> > -
> > -   if (mstart < base) {
> > -   msize -= (base - mstart);
> > -   mstart = base;
> > -   }
> > -
> > -   pr_info("Reserving %lluMB of memory at %#016llx for preserving 
> > crash data",
> > -   (msize >> 20), mstart);
> > -   memblock_reserve(mstart, msize);
> > -   }
> > +   memblock_reserve(base, memblock_end_of_DRAM() - base);
> >  }
> >  
> >  unsigned long __init arch_reserved_kernel_pages(void)
> > -- 
> > 2.26.2

-- 
Sincerely yours,
Mike.


Re: [PATCH 06/15] powerpc: fadamp: simplify fadump_reserve_crash_area()

2020-07-30 Thread Michael Ellerman
Mike Rapoport  writes:
> From: Mike Rapoport 
>
> fadump_reserve_crash_area() reserves memory from a specified base address
> till the end of the RAM.
>
> Replace iteration through the memblock.memory with a single call to
> memblock_reserve() with appropriate  that will take care of proper memory
 ^
 parameters?
> reservation.
>
> Signed-off-by: Mike Rapoport 
> ---
>  arch/powerpc/kernel/fadump.c | 20 +---
>  1 file changed, 1 insertion(+), 19 deletions(-)

I think this looks OK to me, but I don't have a setup to test it easily.
I've added Hari to Cc who might be able to.

But I'll give you an ack in the hope that it works :)

Acked-by: Michael Ellerman 


> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 78ab9a6ee6ac..2446a61e3c25 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -1658,25 +1658,7 @@ int __init fadump_reserve_mem(void)
>  /* Preserve everything above the base address */
>  static void __init fadump_reserve_crash_area(u64 base)
>  {
> - struct memblock_region *reg;
> - u64 mstart, msize;
> -
> - for_each_memblock(memory, reg) {
> - mstart = reg->base;
> - msize  = reg->size;
> -
> - if ((mstart + msize) < base)
> - continue;
> -
> - if (mstart < base) {
> - msize -= (base - mstart);
> - mstart = base;
> - }
> -
> - pr_info("Reserving %lluMB of memory at %#016llx for preserving 
> crash data",
> - (msize >> 20), mstart);
> - memblock_reserve(mstart, msize);
> - }
> + memblock_reserve(base, memblock_end_of_DRAM() - base);
>  }
>  
>  unsigned long __init arch_reserved_kernel_pages(void)
> -- 
> 2.26.2


[PATCH 06/15] powerpc: fadamp: simplify fadump_reserve_crash_area()

2020-07-27 Thread Mike Rapoport
From: Mike Rapoport 

fadump_reserve_crash_area() reserves memory from a specified base address
till the end of the RAM.

Replace iteration through the memblock.memory with a single call to
memblock_reserve() with appropriate  that will take care of proper memory
reservation.

Signed-off-by: Mike Rapoport 
---
 arch/powerpc/kernel/fadump.c | 20 +---
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index 78ab9a6ee6ac..2446a61e3c25 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -1658,25 +1658,7 @@ int __init fadump_reserve_mem(void)
 /* Preserve everything above the base address */
 static void __init fadump_reserve_crash_area(u64 base)
 {
-   struct memblock_region *reg;
-   u64 mstart, msize;
-
-   for_each_memblock(memory, reg) {
-   mstart = reg->base;
-   msize  = reg->size;
-
-   if ((mstart + msize) < base)
-   continue;
-
-   if (mstart < base) {
-   msize -= (base - mstart);
-   mstart = base;
-   }
-
-   pr_info("Reserving %lluMB of memory at %#016llx for preserving 
crash data",
-   (msize >> 20), mstart);
-   memblock_reserve(mstart, msize);
-   }
+   memblock_reserve(base, memblock_end_of_DRAM() - base);
 }
 
 unsigned long __init arch_reserved_kernel_pages(void)
-- 
2.26.2