Currently, any usable memory area beyond page_offset is removed by adding the
memory sizes from each memblock. That may not work for sparse memory
as memory regions can be very far apart resulting incorrect removal of some
usable memory.
Just use the start of the first memory block and the end of the last memory
block to compute the size of the total memory that can be used.
Signed-off-by: Atish Patra <[email protected]>
---
arch/riscv/mm/init.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 787c75f751a5..188281fc2816 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -147,7 +147,6 @@ void __init setup_bootmem(void)
{
struct memblock_region *reg;
phys_addr_t mem_size = 0;
- phys_addr_t total_mem = 0;
phys_addr_t mem_start, end = 0;
phys_addr_t vmlinux_end = __pa_symbol(&_end);
phys_addr_t vmlinux_start = __pa_symbol(&_start);
@@ -155,18 +154,17 @@ void __init setup_bootmem(void)
/* Find the memory region containing the kernel */
for_each_memblock(memory, reg) {
end = reg->base + reg->size;
- if (!total_mem)
+ if (!mem_start)
mem_start = reg->base;
if (reg->base <= vmlinux_start && vmlinux_end <= end)
BUG_ON(reg->size == 0);
- total_mem = total_mem + reg->size;
}
/*
* Remove memblock from the end of usable area to the
* end of region
*/
- mem_size = min(total_mem, (phys_addr_t)-PAGE_OFFSET);
+ mem_size = min(end - mem_start, (phys_addr_t)-PAGE_OFFSET);
if (mem_start + mem_size < end)
memblock_remove(mem_start + mem_size,
end - mem_start - mem_size);
--
2.24.0
Hello Atish,
On Fri, Sep 11, 2020 at 05:23:41PM -0700, Atish Patra wrote:
> Currently, any usable memory area beyond page_offset is removed by adding the
> memory sizes from each memblock. That may not work for sparse memory
> as memory regions can be very far apart resulting incorrect removal of some
> usable memory.
If I understand correctly, the memory with physical addresses larger
than (-PAGE_OFFSET) cannot be used. Since it was aready
memblock_add()'ed during device tree parsing, you need to remove it from
memblock.
For that you can use memblock_enforce_memory_limit(-PAGE_OFFSET).
> Just use the start of the first memory block and the end of the last memory
> block to compute the size of the total memory that can be used.
>
> Signed-off-by: Atish Patra <[email protected]>
> ---
> arch/riscv/mm/init.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 787c75f751a5..188281fc2816 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -147,7 +147,6 @@ void __init setup_bootmem(void)
> {
> struct memblock_region *reg;
> phys_addr_t mem_size = 0;
> - phys_addr_t total_mem = 0;
> phys_addr_t mem_start, end = 0;
> phys_addr_t vmlinux_end = __pa_symbol(&_end);
> phys_addr_t vmlinux_start = __pa_symbol(&_start);
> @@ -155,18 +154,17 @@ void __init setup_bootmem(void)
> /* Find the memory region containing the kernel */
> for_each_memblock(memory, reg) {
> end = reg->base + reg->size;
> - if (!total_mem)
> + if (!mem_start)
> mem_start = reg->base;
> if (reg->base <= vmlinux_start && vmlinux_end <= end)
> BUG_ON(reg->size == 0);
> - total_mem = total_mem + reg->size;
> }
>
> /*
> * Remove memblock from the end of usable area to the
> * end of region
> */
> - mem_size = min(total_mem, (phys_addr_t)-PAGE_OFFSET);
> + mem_size = min(end - mem_start, (phys_addr_t)-PAGE_OFFSET);
> if (mem_start + mem_size < end)
> memblock_remove(mem_start + mem_size,
> end - mem_start - mem_size);
> --
> 2.24.0
>
--
Sincerely yours,
Mike.
On Sat, Sep 12, 2020 at 3:45 AM Mike Rapoport <[email protected]> wrote:
>
> Hello Atish,
>
> On Fri, Sep 11, 2020 at 05:23:41PM -0700, Atish Patra wrote:
> > Currently, any usable memory area beyond page_offset is removed by adding the
> > memory sizes from each memblock. That may not work for sparse memory
> > as memory regions can be very far apart resulting incorrect removal of some
> > usable memory.
>
> If I understand correctly, the memory with physical addresses larger
> than (-PAGE_OFFSET) cannot be used. Since it was aready
> memblock_add()'ed during device tree parsing, you need to remove it from
> memblock.
>
IIRC, the original intention was to fix MAXPHYSMEM_2GB option for RV64
for the medlow model.
That's why the patch removed any memory beyond -PAGE_OFFSET.
> For that you can use memblock_enforce_memory_limit(-PAGE_OFFSET).
>
Thanks. I think we can just call memblock_enforce_memory_limit without
tracking the total memory size
and whether maximum memory described in DT is greater than -PAGE_OFFSET.
@Anup Patel Was there any other reason for this change originally?
> > Just use the start of the first memory block and the end of the last memory
> > block to compute the size of the total memory that can be used.
> >
> > Signed-off-by: Atish Patra <[email protected]>
> > ---
> > arch/riscv/mm/init.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 787c75f751a5..188281fc2816 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -147,7 +147,6 @@ void __init setup_bootmem(void)
> > {
> > struct memblock_region *reg;
> > phys_addr_t mem_size = 0;
> > - phys_addr_t total_mem = 0;
> > phys_addr_t mem_start, end = 0;
> > phys_addr_t vmlinux_end = __pa_symbol(&_end);
> > phys_addr_t vmlinux_start = __pa_symbol(&_start);
> > @@ -155,18 +154,17 @@ void __init setup_bootmem(void)
> > /* Find the memory region containing the kernel */
> > for_each_memblock(memory, reg) {
> > end = reg->base + reg->size;
> > - if (!total_mem)
> > + if (!mem_start)
> > mem_start = reg->base;
> > if (reg->base <= vmlinux_start && vmlinux_end <= end)
> > BUG_ON(reg->size == 0);
> > - total_mem = total_mem + reg->size;
> > }
> >
> > /*
> > * Remove memblock from the end of usable area to the
> > * end of region
> > */
> > - mem_size = min(total_mem, (phys_addr_t)-PAGE_OFFSET);
> > + mem_size = min(end - mem_start, (phys_addr_t)-PAGE_OFFSET);
> > if (mem_start + mem_size < end)
> > memblock_remove(mem_start + mem_size,
> > end - mem_start - mem_size);
> > --
> > 2.24.0
> >
>
> --
> Sincerely yours,
> Mike.
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Regards,
Atish
On Sat, Sep 12, 2020 at 4:15 PM Mike Rapoport <[email protected]> wrote:
>
> Hello Atish,
>
> On Fri, Sep 11, 2020 at 05:23:41PM -0700, Atish Patra wrote:
> > Currently, any usable memory area beyond page_offset is removed by adding the
> > memory sizes from each memblock. That may not work for sparse memory
> > as memory regions can be very far apart resulting incorrect removal of some
> > usable memory.
>
> If I understand correctly, the memory with physical addresses larger
> than (-PAGE_OFFSET) cannot be used. Since it was aready
> memblock_add()'ed during device tree parsing, you need to remove it from
> memblock.
-PAGE_OFFSET represents the size of memory addressable by the kernel and
not the last physical address.
mem_start + (-PAGE_OFFSET) will be the last physical address usable by kernel.
>
> For that you can use memblock_enforce_memory_limit(-PAGE_OFFSET).
I think we should use memblock_enforce_memory_limit(mem_start - PAGE_OFFSET).
Regards,
Anup
>
> > Just use the start of the first memory block and the end of the last memory
> > block to compute the size of the total memory that can be used.
> >
> > Signed-off-by: Atish Patra <[email protected]>
> > ---
> > arch/riscv/mm/init.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > index 787c75f751a5..188281fc2816 100644
> > --- a/arch/riscv/mm/init.c
> > +++ b/arch/riscv/mm/init.c
> > @@ -147,7 +147,6 @@ void __init setup_bootmem(void)
> > {
> > struct memblock_region *reg;
> > phys_addr_t mem_size = 0;
> > - phys_addr_t total_mem = 0;
> > phys_addr_t mem_start, end = 0;
> > phys_addr_t vmlinux_end = __pa_symbol(&_end);
> > phys_addr_t vmlinux_start = __pa_symbol(&_start);
> > @@ -155,18 +154,17 @@ void __init setup_bootmem(void)
> > /* Find the memory region containing the kernel */
> > for_each_memblock(memory, reg) {
> > end = reg->base + reg->size;
> > - if (!total_mem)
> > + if (!mem_start)
> > mem_start = reg->base;
> > if (reg->base <= vmlinux_start && vmlinux_end <= end)
> > BUG_ON(reg->size == 0);
> > - total_mem = total_mem + reg->size;
> > }
> >
> > /*
> > * Remove memblock from the end of usable area to the
> > * end of region
> > */
> > - mem_size = min(total_mem, (phys_addr_t)-PAGE_OFFSET);
> > + mem_size = min(end - mem_start, (phys_addr_t)-PAGE_OFFSET);
> > if (mem_start + mem_size < end)
> > memblock_remove(mem_start + mem_size,
> > end - mem_start - mem_size);
> > --
> > 2.24.0
> >
>
> --
> Sincerely yours,
> Mike.
On Mon, Sep 14, 2020 at 5:18 AM Atish Patra <[email protected]> wrote:
>
> On Sat, Sep 12, 2020 at 3:45 AM Mike Rapoport <[email protected]> wrote:
> >
> > Hello Atish,
> >
> > On Fri, Sep 11, 2020 at 05:23:41PM -0700, Atish Patra wrote:
> > > Currently, any usable memory area beyond page_offset is removed by adding the
> > > memory sizes from each memblock. That may not work for sparse memory
> > > as memory regions can be very far apart resulting incorrect removal of some
> > > usable memory.
> >
> > If I understand correctly, the memory with physical addresses larger
> > than (-PAGE_OFFSET) cannot be used. Since it was aready
> > memblock_add()'ed during device tree parsing, you need to remove it from
> > memblock.
> >
>
> IIRC, the original intention was to fix MAXPHYSMEM_2GB option for RV64
> for the medlow model.
> That's why the patch removed any memory beyond -PAGE_OFFSET.
>
> > For that you can use memblock_enforce_memory_limit(-PAGE_OFFSET).
> >
> Thanks. I think we can just call memblock_enforce_memory_limit without
> tracking the total memory size
> and whether maximum memory described in DT is greater than -PAGE_OFFSET.
>
> @Anup Patel Was there any other reason for this change originally?
No other reason. We just wanted to ensure that amount of memory addressable
by kernel (i.e. -PAGE_OFFSET) is also considered when removing memblock.
Regards,
Anup
>
> > > Just use the start of the first memory block and the end of the last memory
> > > block to compute the size of the total memory that can be used.
> > >
> > > Signed-off-by: Atish Patra <[email protected]>
> > > ---
> > > arch/riscv/mm/init.c | 6 ++----
> > > 1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > index 787c75f751a5..188281fc2816 100644
> > > --- a/arch/riscv/mm/init.c
> > > +++ b/arch/riscv/mm/init.c
> > > @@ -147,7 +147,6 @@ void __init setup_bootmem(void)
> > > {
> > > struct memblock_region *reg;
> > > phys_addr_t mem_size = 0;
> > > - phys_addr_t total_mem = 0;
> > > phys_addr_t mem_start, end = 0;
> > > phys_addr_t vmlinux_end = __pa_symbol(&_end);
> > > phys_addr_t vmlinux_start = __pa_symbol(&_start);
> > > @@ -155,18 +154,17 @@ void __init setup_bootmem(void)
> > > /* Find the memory region containing the kernel */
> > > for_each_memblock(memory, reg) {
> > > end = reg->base + reg->size;
> > > - if (!total_mem)
> > > + if (!mem_start)
> > > mem_start = reg->base;
> > > if (reg->base <= vmlinux_start && vmlinux_end <= end)
> > > BUG_ON(reg->size == 0);
> > > - total_mem = total_mem + reg->size;
> > > }
> > >
> > > /*
> > > * Remove memblock from the end of usable area to the
> > > * end of region
> > > */
> > > - mem_size = min(total_mem, (phys_addr_t)-PAGE_OFFSET);
> > > + mem_size = min(end - mem_start, (phys_addr_t)-PAGE_OFFSET);
> > > if (mem_start + mem_size < end)
> > > memblock_remove(mem_start + mem_size,
> > > end - mem_start - mem_size);
> > > --
> > > 2.24.0
> > >
> >
> > --
> > Sincerely yours,
> > Mike.
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
>
> --
> Regards,
> Atish
On Mon, Sep 14, 2020 at 4:52 AM Anup Patel <[email protected]> wrote:
>
> On Mon, Sep 14, 2020 at 5:18 AM Atish Patra <[email protected]> wrote:
> >
> > On Sat, Sep 12, 2020 at 3:45 AM Mike Rapoport <[email protected]> wrote:
> > >
> > > Hello Atish,
> > >
> > > On Fri, Sep 11, 2020 at 05:23:41PM -0700, Atish Patra wrote:
> > > > Currently, any usable memory area beyond page_offset is removed by adding the
> > > > memory sizes from each memblock. That may not work for sparse memory
> > > > as memory regions can be very far apart resulting incorrect removal of some
> > > > usable memory.
> > >
> > > If I understand correctly, the memory with physical addresses larger
> > > than (-PAGE_OFFSET) cannot be used. Since it was aready
> > > memblock_add()'ed during device tree parsing, you need to remove it from
> > > memblock.
> > >
> >
> > IIRC, the original intention was to fix MAXPHYSMEM_2GB option for RV64
> > for the medlow model.
> > That's why the patch removed any memory beyond -PAGE_OFFSET.
> >
> > > For that you can use memblock_enforce_memory_limit(-PAGE_OFFSET).
> > >
> > Thanks. I think we can just call memblock_enforce_memory_limit without
> > tracking the total memory size
> > and whether maximum memory described in DT is greater than -PAGE_OFFSET.
> >
> > @Anup Patel Was there any other reason for this change originally?
>
> No other reason. We just wanted to ensure that amount of memory addressable
> by kernel (i.e. -PAGE_OFFSET) is also considered when removing memblock.
>
It looks like we have an agreement here then. I will update the patch
to directly call
memblock_enforce_memory_limit as suggested by Mike.
> Regards,
> Anup
>
> >
> > > > Just use the start of the first memory block and the end of the last memory
> > > > block to compute the size of the total memory that can be used.
> > > >
> > > > Signed-off-by: Atish Patra <[email protected]>
> > > > ---
> > > > arch/riscv/mm/init.c | 6 ++----
> > > > 1 file changed, 2 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > > index 787c75f751a5..188281fc2816 100644
> > > > --- a/arch/riscv/mm/init.c
> > > > +++ b/arch/riscv/mm/init.c
> > > > @@ -147,7 +147,6 @@ void __init setup_bootmem(void)
> > > > {
> > > > struct memblock_region *reg;
> > > > phys_addr_t mem_size = 0;
> > > > - phys_addr_t total_mem = 0;
> > > > phys_addr_t mem_start, end = 0;
> > > > phys_addr_t vmlinux_end = __pa_symbol(&_end);
> > > > phys_addr_t vmlinux_start = __pa_symbol(&_start);
> > > > @@ -155,18 +154,17 @@ void __init setup_bootmem(void)
> > > > /* Find the memory region containing the kernel */
> > > > for_each_memblock(memory, reg) {
> > > > end = reg->base + reg->size;
> > > > - if (!total_mem)
> > > > + if (!mem_start)
> > > > mem_start = reg->base;
> > > > if (reg->base <= vmlinux_start && vmlinux_end <= end)
> > > > BUG_ON(reg->size == 0);
> > > > - total_mem = total_mem + reg->size;
> > > > }
> > > >
> > > > /*
> > > > * Remove memblock from the end of usable area to the
> > > > * end of region
> > > > */
> > > > - mem_size = min(total_mem, (phys_addr_t)-PAGE_OFFSET);
> > > > + mem_size = min(end - mem_start, (phys_addr_t)-PAGE_OFFSET);
> > > > if (mem_start + mem_size < end)
> > > > memblock_remove(mem_start + mem_size,
> > > > end - mem_start - mem_size);
> > > > --
> > > > 2.24.0
> > > >
> > >
> > > --
> > > Sincerely yours,
> > > Mike.
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > [email protected]
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> >
> >
> >
> > --
> > Regards,
> > Atish
--
Regards,
Atish