mbox series

[v3,0/4] memblock, arm: fixes for freeing of the memory map

Message ID 20210630071211.21011-1-rppt@kernel.org (mailing list archive)
Headers show
Series memblock, arm: fixes for freeing of the memory map | expand

Message

Mike Rapoport June 30, 2021, 7:12 a.m. UTC
From: Mike Rapoport <rppt@linux.ibm.com>

Hi,

The coordination between freeing of unused memory map, pfn_valid() and core
mm assumptions about validity of the memory map in various ranges was not
designed for complex layouts of the physical memory with a lot of holes all
over the place.

Kefen Wang reported crashes in move_freepages() on a system with the
following memory layout [1]:

  node   0: [mem 0x0000000080a00000-0x00000000855fffff]
  node   0: [mem 0x0000000086a00000-0x0000000087dfffff]
  node   0: [mem 0x000000008bd00000-0x000000008c4fffff]
  node   0: [mem 0x000000008e300000-0x000000008ecfffff]
  node   0: [mem 0x0000000090d00000-0x00000000bfffffff]
  node   0: [mem 0x00000000cc000000-0x00000000dc9fffff]
  node   0: [mem 0x00000000de700000-0x00000000de9fffff]
  node   0: [mem 0x00000000e0800000-0x00000000e0bfffff]
  node   0: [mem 0x00000000f4b00000-0x00000000f6ffffff]
  node   0: [mem 0x00000000fda00000-0x00000000ffffefff]

The crashes can be mitigated by enabling CONFIG_HOLES_IN_ZONE and
essentially turning pfn_valid_within() to pfn_valid() instead of having it
hardwired to 1, but this would require to keep CONFIG_HOLES_IN_ZONE which
could be removed after arm64 and MIPS stopped using it ([2], [3]).

Alternatively, we can update ARM's implementation of pfn_valid() to take
into accounting rounding of the freed memory map to pageblock boundaries
and make sure it returns true for PFNs that have memory map entries even if
there is no physical memory.

I'm planning to merge this via memblock tree.

v3: 
* Add patch 3/4 to ensure there is no overflow in memblock_overlaps_region()

v2: Link: https://lore.kernel.org/lkml/20210519141436.11961-1-rppt@kernel.org
* Use single memblock_overlaps_region() instead of several
  memblock_is_map_memory() lookups. This makes this series depend on update
  of MEMBLOCK_NOMAP handling in the memory map [2]

v1: Link: https://lore.kernel.org/lkml/20210518090613.21519-1-rppt@kernel.org

[1] https://lore.kernel.org/lkml/2a1592ad-bc9d-4664-fd19-f7448a37edc0@huawei.com
[2] https://lore.kernel.org/lkml/20210511100550.28178-1-rppt@kernel.org
[3] https://lore.kernel.org/lkml/20210418093512.668-1-rppt@kernel.org

Mike Rapoport (4):
  memblock: free_unused_memmap: use pageblock units instead of MAX_ORDER
  memblock: align freed memory map on pageblock boundaries with SPARSEMEM
  memblock: ensure there is no overflow in memblock_overlaps_region()
  arm: extend pfn_valid to take into account freed memory map alignment

 arch/arm/mm/init.c | 13 ++++++++++++-
 mm/memblock.c      | 26 ++++++++++++++------------
 2 files changed, 26 insertions(+), 13 deletions(-)


base-commit: c4681547bcce777daf576925a966ffa824edd09d

Comments

Tony Lindgren June 30, 2021, 8:26 a.m. UTC | #1
Hi,

* Mike Rapoport <rppt@kernel.org> [210630 07:12]:
> From: Mike Rapoport <rppt@linux.ibm.com>
> v3: 
> * Add patch 3/4 to ensure there is no overflow in memblock_overlaps_region()

This series boots for me, so:

Tested-by: Tony Lindgren <tony@atomide.com>
Mark-PK Tsai Nov. 11, 2021, 7:33 a.m. UTC | #2
Hi,

The lts kernel also have this issue. (we use 5.4-lts kernel.)
Currently we patch our custom kernel to select CONFIG_HOLES_IN_ZONE for arch arm.
But I think the formal solution should backport to lts.

Would you help to backport this patch series? (including the below commit)

(024591f9a6e0 arm: ioremap: don't abuse pfn_valid() to check if pfn is in RAM)

Thanks!
Mike Rapoport Nov. 11, 2021, 9:45 a.m. UTC | #3
Hi,

On Thu, Nov 11, 2021 at 03:33:29PM +0800, Mark-PK Tsai wrote:
> Hi,
> 
> The lts kernel also have this issue. (we use 5.4-lts kernel.)
> Currently we patch our custom kernel to select CONFIG_HOLES_IN_ZONE for arch arm.
> But I think the formal solution should backport to lts.
> 
> Would you help to backport this patch series? (including the below commit)

There were a couple of changes between 5.4 and this set, so you'd need to
"apply" the first two patches to arm::free_unused_memmap(). Other than
that, I don't see any pitfalls here.

Feel free to CC me when you post the backported series.
 
> (024591f9a6e0 arm: ioremap: don't abuse pfn_valid() to check if pfn is in RAM)
> 
> Thanks!