linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map
@ 2021-06-30  7:12 Mike Rapoport
  2021-06-30  7:12 ` [PATCH v3 1/4] memblock: free_unused_memmap: use pageblock units instead of MAX_ORDER Mike Rapoport
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Mike Rapoport @ 2021-06-30  7:12 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Andrew Morton, Kefeng Wang, Mike Rapoport, Mike Rapoport,
	Russell King, Tony Lindgren, linux-kernel, linux-mm

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
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/4] memblock: free_unused_memmap: use pageblock units instead of MAX_ORDER
  2021-06-30  7:12 [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map Mike Rapoport
@ 2021-06-30  7:12 ` Mike Rapoport
  2021-06-30  7:12 ` [PATCH v3 2/4] memblock: align freed memory map on pageblock boundaries with SPARSEMEM Mike Rapoport
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Mike Rapoport @ 2021-06-30  7:12 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Andrew Morton, Kefeng Wang, Mike Rapoport, Mike Rapoport,
	Russell King, Tony Lindgren, linux-kernel, linux-mm

From: Mike Rapoport <rppt@linux.ibm.com>

The code that frees unused memory map uses rounds start and end of the
holes that are freed to MAX_ORDER_NR_PAGES to preserve continuity of the
memory map for MAX_ORDER regions.

Lots of core memory management functionality relies on homogeneity of the
memory map within each pageblock which size may differ from MAX_ORDER in
certain configurations.

Although currently, for the architectures that use free_unused_memmap(),
pageblock_order and MAX_ORDER are equivalent, it is cleaner to have common
notation thought mm code.

Replace MAX_ORDER_NR_PAGES with pageblock_nr_pages and update the comments
to make it more clear why the alignment to pageblock boundaries is
required.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---
 mm/memblock.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index afaefa8fc6ab..97fa87541b5f 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1943,11 +1943,11 @@ static void __init free_unused_memmap(void)
 		start = min(start, ALIGN(prev_end, PAGES_PER_SECTION));
 #else
 		/*
-		 * Align down here since the VM subsystem insists that the
-		 * memmap entries are valid from the bank start aligned to
-		 * MAX_ORDER_NR_PAGES.
+		 * Align down here since many operations in VM subsystem
+		 * presume that there are no holes in the memory map inside
+		 * a pageblock
 		 */
-		start = round_down(start, MAX_ORDER_NR_PAGES);
+		start = round_down(start, pageblock_nr_pages);
 #endif
 
 		/*
@@ -1958,11 +1958,11 @@ static void __init free_unused_memmap(void)
 			free_memmap(prev_end, start);
 
 		/*
-		 * Align up here since the VM subsystem insists that the
-		 * memmap entries are valid from the bank end aligned to
-		 * MAX_ORDER_NR_PAGES.
+		 * Align up here since many operations in VM subsystem
+		 * presume that there are no holes in the memory map inside
+		 * a pageblock
 		 */
-		prev_end = ALIGN(end, MAX_ORDER_NR_PAGES);
+		prev_end = ALIGN(end, pageblock_nr_pages);
 	}
 
 #ifdef CONFIG_SPARSEMEM
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/4] memblock: align freed memory map on pageblock boundaries with SPARSEMEM
  2021-06-30  7:12 [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map Mike Rapoport
  2021-06-30  7:12 ` [PATCH v3 1/4] memblock: free_unused_memmap: use pageblock units instead of MAX_ORDER Mike Rapoport
@ 2021-06-30  7:12 ` Mike Rapoport
  2021-06-30  7:12 ` [PATCH v3 3/4] memblock: ensure there is no overflow in memblock_overlaps_region() Mike Rapoport
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Mike Rapoport @ 2021-06-30  7:12 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Andrew Morton, Kefeng Wang, Mike Rapoport, Mike Rapoport,
	Russell King, Tony Lindgren, linux-kernel, linux-mm

From: Mike Rapoport <rppt@linux.ibm.com>

When CONFIG_SPARSEMEM=y the ranges of the memory map that are freed are not
aligned to the pageblock boundaries which breaks assumptions about
homogeneity of the memory map throughout core mm code.

Make sure that the freed memory map is always aligned on pageblock
boundaries regardless of the memory model selection.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---
 mm/memblock.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index 97fa87541b5f..2e25d69739e0 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1941,14 +1941,13 @@ static void __init free_unused_memmap(void)
 		 * due to SPARSEMEM sections which aren't present.
 		 */
 		start = min(start, ALIGN(prev_end, PAGES_PER_SECTION));
-#else
+#endif
 		/*
 		 * Align down here since many operations in VM subsystem
 		 * presume that there are no holes in the memory map inside
 		 * a pageblock
 		 */
 		start = round_down(start, pageblock_nr_pages);
-#endif
 
 		/*
 		 * If we had a previous bank, and there is a space
@@ -1966,8 +1965,10 @@ static void __init free_unused_memmap(void)
 	}
 
 #ifdef CONFIG_SPARSEMEM
-	if (!IS_ALIGNED(prev_end, PAGES_PER_SECTION))
+	if (!IS_ALIGNED(prev_end, PAGES_PER_SECTION)) {
+		prev_end = ALIGN(end, pageblock_nr_pages);
 		free_memmap(prev_end, ALIGN(prev_end, PAGES_PER_SECTION));
+	}
 #endif
 }
 
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 3/4] memblock: ensure there is no overflow in memblock_overlaps_region()
  2021-06-30  7:12 [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map Mike Rapoport
  2021-06-30  7:12 ` [PATCH v3 1/4] memblock: free_unused_memmap: use pageblock units instead of MAX_ORDER Mike Rapoport
  2021-06-30  7:12 ` [PATCH v3 2/4] memblock: align freed memory map on pageblock boundaries with SPARSEMEM Mike Rapoport
@ 2021-06-30  7:12 ` Mike Rapoport
  2021-06-30  7:12 ` [PATCH v3 4/4] arm: extend pfn_valid to take into account freed memory map alignment Mike Rapoport
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Mike Rapoport @ 2021-06-30  7:12 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Andrew Morton, Kefeng Wang, Mike Rapoport, Mike Rapoport,
	Russell King, Tony Lindgren, linux-kernel, linux-mm

From: Mike Rapoport <rppt@linux.ibm.com>

There maybe an overflow in memblock_overlaps_region() if it is called with
base and size such that

	base + size > PHYS_ADDR_MAX

Make sure that memblock_overlaps_region() caps the size to prevent such
overflow and remove now duplicated call to memblock_cap_size() from
memblock_is_region_reserved().

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---
 mm/memblock.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index 2e25d69739e0..67e0e24f8cc9 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -182,6 +182,8 @@ bool __init_memblock memblock_overlaps_region(struct memblock_type *type,
 {
 	unsigned long i;
 
+	memblock_cap_size(base, &size);
+
 	for (i = 0; i < type->cnt; i++)
 		if (memblock_addrs_overlap(base, size, type->regions[i].base,
 					   type->regions[i].size))
@@ -1794,7 +1796,6 @@ bool __init_memblock memblock_is_region_memory(phys_addr_t base, phys_addr_t siz
  */
 bool __init_memblock memblock_is_region_reserved(phys_addr_t base, phys_addr_t size)
 {
-	memblock_cap_size(base, &size);
 	return memblock_overlaps_region(&memblock.reserved, base, size);
 }
 
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 4/4] arm: extend pfn_valid to take into account freed memory map alignment
  2021-06-30  7:12 [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map Mike Rapoport
                   ` (2 preceding siblings ...)
  2021-06-30  7:12 ` [PATCH v3 3/4] memblock: ensure there is no overflow in memblock_overlaps_region() Mike Rapoport
@ 2021-06-30  7:12 ` Mike Rapoport
  2021-07-05  4:22   ` Guenter Roeck
  2021-06-30  8:26 ` [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map Tony Lindgren
  2021-11-11  7:33 ` Mark-PK Tsai
  5 siblings, 1 reply; 12+ messages in thread
From: Mike Rapoport @ 2021-06-30  7:12 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Andrew Morton, Kefeng Wang, Mike Rapoport, Mike Rapoport,
	Russell King, Tony Lindgren, linux-kernel, linux-mm

From: Mike Rapoport <rppt@linux.ibm.com>

When unused memory map is freed the preserved part of the memory map is
extended to match pageblock boundaries because lots of core mm
functionality relies on homogeneity of the memory map within pageblock
boundaries.

Since pfn_valid() is used to check whether there is a valid memory map
entry for a PFN, make it return true also for PFNs that have memory map
entries even if there is no actual memory populated there.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
Tested-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 arch/arm/mm/init.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 9d4744a632c6..6162a070a410 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -125,11 +125,22 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max_low,
 int pfn_valid(unsigned long pfn)
 {
 	phys_addr_t addr = __pfn_to_phys(pfn);
+	unsigned long pageblock_size = PAGE_SIZE * pageblock_nr_pages;
 
 	if (__phys_to_pfn(addr) != pfn)
 		return 0;
 
-	return memblock_is_map_memory(addr);
+	/*
+	 * If address less than pageblock_size bytes away from a present
+	 * memory chunk there still will be a memory map entry for it
+	 * because we round freed memory map to the pageblock boundaries.
+	 */
+	if (memblock_overlaps_region(&memblock.memory,
+				     ALIGN_DOWN(addr, pageblock_size),
+				     pageblock_size))
+		return 1;
+
+	return 0;
 }
 EXPORT_SYMBOL(pfn_valid);
 #endif
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map
  2021-06-30  7:12 [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map Mike Rapoport
                   ` (3 preceding siblings ...)
  2021-06-30  7:12 ` [PATCH v3 4/4] arm: extend pfn_valid to take into account freed memory map alignment Mike Rapoport
@ 2021-06-30  8:26 ` Tony Lindgren
  2021-11-11  7:33 ` Mark-PK Tsai
  5 siblings, 0 replies; 12+ messages in thread
From: Tony Lindgren @ 2021-06-30  8:26 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: linux-arm-kernel, Andrew Morton, Kefeng Wang, Mike Rapoport,
	Russell King, linux-kernel, linux-mm

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>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 4/4] arm: extend pfn_valid to take into account freed memory map alignment
  2021-06-30  7:12 ` [PATCH v3 4/4] arm: extend pfn_valid to take into account freed memory map alignment Mike Rapoport
@ 2021-07-05  4:22   ` Guenter Roeck
  2021-07-05  7:23     ` Mike Rapoport
  0 siblings, 1 reply; 12+ messages in thread
From: Guenter Roeck @ 2021-07-05  4:22 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: linux-arm-kernel, Andrew Morton, Kefeng Wang, Mike Rapoport,
	Russell King, Tony Lindgren, linux-kernel, linux-mm

On Wed, Jun 30, 2021 at 10:12:11AM +0300, Mike Rapoport wrote:
> From: Mike Rapoport <rppt@linux.ibm.com>
> 
> When unused memory map is freed the preserved part of the memory map is
> extended to match pageblock boundaries because lots of core mm
> functionality relies on homogeneity of the memory map within pageblock
> boundaries.
> 
> Since pfn_valid() is used to check whether there is a valid memory map
> entry for a PFN, make it return true also for PFNs that have memory map
> entries even if there is no actual memory populated there.
> 
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> Tested-by: Kefeng Wang <wangkefeng.wang@huawei.com>

With this patch in place, the romulus-bmc emulation in qemu gets the
following traceback:

[    2.863406] WARNING: CPU: 0 PID: 1 at arch/arm/mm/ioremap.c:287 __arm_ioremap_pfn_caller+0xf0/0x1dc
[    2.864812] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-09882-ga180bd1d7e16 #1
[    2.865263] Hardware name: Generic DT based system
[    2.865711] Backtrace: 
[    2.866063] [<80b07e58>] (dump_backtrace) from [<80b080ac>] (show_stack+0x20/0x24)
[    2.866633]  r7:00000009 r6:0000011f r5:60000153 r4:80ddd1c0
[    2.866922] [<80b0808c>] (show_stack) from [<80b18df0>] (dump_stack_lvl+0x58/0x74)
[    2.867117] [<80b18d98>] (dump_stack_lvl) from [<80b18e20>] (dump_stack+0x14/0x1c)
[    2.867309]  r5:80118cac r4:80dc6774
[    2.867404] [<80b18e0c>] (dump_stack) from [<80122fcc>] (__warn+0xe4/0x150)
[    2.867583] [<80122ee8>] (__warn) from [<80b08850>] (warn_slowpath_fmt+0x88/0xc0)
[    2.867774]  r7:0000011f r6:80dc6774 r5:00000000 r4:814c4000
[    2.867917] [<80b087cc>] (warn_slowpath_fmt) from [<80118cac>] (__arm_ioremap_pfn_caller+0xf0/0x1dc)
[    2.868158]  r9:00000001 r8:9ef00000 r7:80e8b0d4 r6:0009ef00 r5:00000000 r4:00100000
[    2.868346] [<80118bbc>] (__arm_ioremap_pfn_caller) from [<80118df8>] (__arm_ioremap_caller+0x60/0x68)
[    2.868581]  r9:9ef00000 r8:821b6dc0 r7:00100000 r6:00000000 r5:815d1010 r4:80118d98
[    2.868761] [<80118d98>] (__arm_ioremap_caller) from [<80118fcc>] (ioremap+0x28/0x30)
[    2.868958] [<80118fa4>] (ioremap) from [<8062871c>] (__devm_ioremap_resource+0x154/0x1c8)
[    2.869169]  r5:815d1010 r4:814c5d2c
[    2.869263] [<806285c8>] (__devm_ioremap_resource) from [<8062899c>] (devm_ioremap_resource+0x14/0x18)
[    2.869495]  r9:9e9f57a0 r8:814c4000 r7:815d1000 r6:815d1010 r5:8177c078 r4:815cf400
[    2.869676] [<80628988>] (devm_ioremap_resource) from [<8091c6e4>] (fsi_master_acf_probe+0x1a8/0x5d8)
[    2.869909] [<8091c53c>] (fsi_master_acf_probe) from [<80723dbc>] (platform_probe+0x68/0xc8)
[    2.870124]  r9:80e9dadc r8:00000000 r7:815d1010 r6:810c1000 r5:815d1010 r4:00000000
[    2.870306] [<80723d54>] (platform_probe) from [<80721208>] (really_probe+0x1cc/0x470)
[    2.870512]  r7:815d1010 r6:810c1000 r5:00000000 r4:815d1010
[    2.870651] [<8072103c>] (really_probe) from [<807215cc>] (__driver_probe_device+0x120/0x1fc)
[    2.870872]  r7:815d1010 r6:810c1000 r5:810c1000 r4:815d1010
[    2.871013] [<807214ac>] (__driver_probe_device) from [<807216e8>] (driver_probe_device+0x40/0xd8)
[    2.871244]  r9:80e9dadc r8:00000000 r7:815d1010 r6:810c1000 r5:812feaa0 r4:812fe994
[    2.871428] [<807216a8>] (driver_probe_device) from [<80721a58>] (__driver_attach+0xa8/0x1d4)
[    2.871647]  r9:80e9dadc r8:00000000 r7:00000000 r6:810c1000 r5:815d1054 r4:815d1010
[    2.871830] [<807219b0>] (__driver_attach) from [<8071ee8c>] (bus_for_each_dev+0x88/0xc8)
[    2.872040]  r7:00000000 r6:814c4000 r5:807219b0 r4:810c1000
[    2.872194] [<8071ee04>] (bus_for_each_dev) from [<80722208>] (driver_attach+0x28/0x30)
[    2.872418]  r7:810a2aa0 r6:00000000 r5:821b6000 r4:810c1000
[    2.872570] [<807221e0>] (driver_attach) from [<8071f80c>] (bus_add_driver+0x114/0x200)
[    2.872788] [<8071f6f8>] (bus_add_driver) from [<80722ec4>] (driver_register+0x98/0x128)
[    2.873011]  r7:81011d0c r6:814c4000 r5:00000000 r4:810c1000
[    2.873167] [<80722e2c>] (driver_register) from [<80725240>] (__platform_driver_register+0x2c/0x34)
[    2.873408]  r5:814dcb80 r4:80f2a764
[    2.873513] [<80725214>] (__platform_driver_register) from [<80f2a784>] (fsi_master_acf_init+0x20/0x28)
[    2.873766] [<80f2a764>] (fsi_master_acf_init) from [<80f014a8>] (do_one_initcall+0x108/0x290)
[    2.874007] [<80f013a0>] (do_one_initcall) from [<80f01840>] (kernel_init_freeable+0x1ac/0x230)
[    2.874248]  r9:80e9dadc r8:80f3987c r7:80f3985c r6:00000007 r5:814dcb80 r4:80f627a4
[    2.874456] [<80f01694>] (kernel_init_freeable) from [<80b19f44>] (kernel_init+0x20/0x138)
[    2.874691]  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:80b19f24
[    2.874894]  r4:00000000
[    2.874977] [<80b19f24>] (kernel_init) from [<80100170>] (ret_from_fork+0x14/0x24)
[    2.875231] Exception stack(0x814c5fb0 to 0x814c5ff8)
[    2.875535] 5fa0:                                     00000000 00000000 00000000 00000000
[    2.875849] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    2.876133] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    2.876363]  r5:80b19f24 r4:00000000
[    2.876683] ---[ end trace b2f74b8536829970 ]---
[    2.876911] fsi-master-acf gpio-fsi: ioremap failed for resource [mem 0x9ef00000-0x9effffff]
[    2.877492] fsi-master-acf gpio-fsi: Error -12 mapping coldfire memory
[    2.877689] fsi-master-acf: probe of gpio-fsi failed with error -12

Reverting it fixes the problem. Also, the ioremap failure is no longer seen
after reverting this patch.

Guenter

---
bisect log:

# bad: [a180bd1d7e16173d965b263c5a536aa40afa2a2a] iov_iter: remove uaccess_kernel() warning from iov_iter_init()
# good: [303392fd5c160822bf778270b28ec5ea50cab2b4] Merge tag 'leds-5.14-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds
git bisect start 'HEAD' '303392fd5c16'
# good: [2bb919b62f6e5959552a90a399d09d683afa3d1d] Merge tag 's390-5.14-1' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
git bisect good 2bb919b62f6e5959552a90a399d09d683afa3d1d
# good: [5390473ec1697b71af0e9d63ef7aaa7ecd27e2c9] rcu: Don't penalize priority boosting when there is nothing to boost
git bisect good 5390473ec1697b71af0e9d63ef7aaa7ecd27e2c9
# good: [641faf1b9064c270a476a424e60063bb05df3ee9] Merge branches 'bitmaprange.2021.05.10c', 'doc.2021.05.10c', 'fixes.2021.05.13a', 'kvfree_rcu.2021.05.10c', 'mmdumpobj.2021.05.10c', 'nocb.2021.05.12a', 'srcu.2021.05.12a', 'tasks.2021.05.18a' and 'torture.2021.05.10c' into HEAD
git bisect good 641faf1b9064c270a476a424e60063bb05df3ee9
# good: [b930226f3db870cfb683c2744aeb0d29deb4cddc] kcsan: Document "value changed" line
git bisect good b930226f3db870cfb683c2744aeb0d29deb4cddc
# bad: [a412897fb546fbb291095be576165ce757eff70b] Merge tag 'memblock-v5.14-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/rppt/memblock
git bisect bad a412897fb546fbb291095be576165ce757eff70b
# good: [f921f53e089a12a192808ac4319f28727b35dc0f] memblock: align freed memory map on pageblock boundaries with SPARSEMEM
git bisect good f921f53e089a12a192808ac4319f28727b35dc0f
# bad: [a4d5613c4dc6d413e0733e37db9d116a2a36b9f3] arm: extend pfn_valid to take into account freed memory map alignment
git bisect bad a4d5613c4dc6d413e0733e37db9d116a2a36b9f3
# good: [023accf5cdc1e504a9b04187ec23ff156fe53d90] memblock: ensure there is no overflow in memblock_overlaps_region()
git bisect good 023accf5cdc1e504a9b04187ec23ff156fe53d90
# first bad commit: [a4d5613c4dc6d413e0733e37db9d116a2a36b9f3] arm: extend pfn_valid to take into account freed memory map alignment

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 4/4] arm: extend pfn_valid to take into account freed memory map alignment
  2021-07-05  4:22   ` Guenter Roeck
@ 2021-07-05  7:23     ` Mike Rapoport
  2021-07-05 14:55       ` Guenter Roeck
  2021-07-09  4:56       ` Alexey Minnekhanov
  0 siblings, 2 replies; 12+ messages in thread
From: Mike Rapoport @ 2021-07-05  7:23 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-arm-kernel, Andrew Morton, Kefeng Wang, Mike Rapoport,
	Russell King, Tony Lindgren, linux-kernel, linux-mm

Hi Guenter,

On Sun, Jul 04, 2021 at 09:22:36PM -0700, Guenter Roeck wrote:
> On Wed, Jun 30, 2021 at 10:12:11AM +0300, Mike Rapoport wrote:
> > From: Mike Rapoport <rppt@linux.ibm.com>
> > 
> > When unused memory map is freed the preserved part of the memory map is
> > extended to match pageblock boundaries because lots of core mm
> > functionality relies on homogeneity of the memory map within pageblock
> > boundaries.
> > 
> > Since pfn_valid() is used to check whether there is a valid memory map
> > entry for a PFN, make it return true also for PFNs that have memory map
> > entries even if there is no actual memory populated there.
> > 
> > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> > Tested-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> 
> With this patch in place, the romulus-bmc emulation in qemu gets the
> following traceback:
> 
> [    2.863406] WARNING: CPU: 0 PID: 1 at arch/arm/mm/ioremap.c:287 __arm_ioremap_pfn_caller+0xf0/0x1dc
...
> [    2.876683] ---[ end trace b2f74b8536829970 ]---
> [    2.876911] fsi-master-acf gpio-fsi: ioremap failed for resource [mem 0x9ef00000-0x9effffff]
> [    2.877492] fsi-master-acf gpio-fsi: Error -12 mapping coldfire memory
> [    2.877689] fsi-master-acf: probe of gpio-fsi failed with error -12
> 
> Reverting it fixes the problem. Also, the ioremap failure is no longer seen
> after reverting this patch.

I believe this should fix it:

From e2213b7e804daf0d31d47502379916f0542398bb Mon Sep 17 00:00:00 2001
From: Mike Rapoport <rppt@linux.ibm.com>
Date: Mon, 5 Jul 2021 08:43:10 +0300
Subject: [PATCH] arm: ioremap: don't abuse pfn_valid() to check if pfn is in RAM

The semantics of pfn_valid() is to check presence of the memory map for a
PFN and not whether a PFN is in RAM. The memory map may be present for a
hole in the physical memory and if such hole corresponds to an MMIO range,
__arm_ioremap_pfn_caller() will produce a WARN() and fail:

[    2.863406] WARNING: CPU: 0 PID: 1 at arch/arm/mm/ioremap.c:287 __arm_ioremap_pfn_caller+0xf0/0x1dc
[    2.864812] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-09882-ga180bd1d7e16 #1
[    2.865263] Hardware name: Generic DT based system
[    2.865711] Backtrace:
[    2.866063] [<80b07e58>] (dump_backtrace) from [<80b080ac>] (show_stack+0x20/0x24)
[    2.866633]  r7:00000009 r6:0000011f r5:60000153 r4:80ddd1c0
[    2.866922] [<80b0808c>] (show_stack) from [<80b18df0>] (dump_stack_lvl+0x58/0x74)
[    2.867117] [<80b18d98>] (dump_stack_lvl) from [<80b18e20>] (dump_stack+0x14/0x1c)
[    2.867309]  r5:80118cac r4:80dc6774
[    2.867404] [<80b18e0c>] (dump_stack) from [<80122fcc>] (__warn+0xe4/0x150)
[    2.867583] [<80122ee8>] (__warn) from [<80b08850>] (warn_slowpath_fmt+0x88/0xc0)
[    2.867774]  r7:0000011f r6:80dc6774 r5:00000000 r4:814c4000
[    2.867917] [<80b087cc>] (warn_slowpath_fmt) from [<80118cac>] (__arm_ioremap_pfn_caller+0xf0/0x1dc)
[    2.868158]  r9:00000001 r8:9ef00000 r7:80e8b0d4 r6:0009ef00 r5:00000000 r4:00100000
[    2.868346] [<80118bbc>] (__arm_ioremap_pfn_caller) from [<80118df8>] (__arm_ioremap_caller+0x60/0x68)
[    2.868581]  r9:9ef00000 r8:821b6dc0 r7:00100000 r6:00000000 r5:815d1010 r4:80118d98
[    2.868761] [<80118d98>] (__arm_ioremap_caller) from [<80118fcc>] (ioremap+0x28/0x30)
[    2.868958] [<80118fa4>] (ioremap) from [<8062871c>] (__devm_ioremap_resource+0x154/0x1c8)
[    2.869169]  r5:815d1010 r4:814c5d2c
[    2.869263] [<806285c8>] (__devm_ioremap_resource) from [<8062899c>] (devm_ioremap_resource+0x14/0x18)
[    2.869495]  r9:9e9f57a0 r8:814c4000 r7:815d1000 r6:815d1010 r5:8177c078 r4:815cf400
[    2.869676] [<80628988>] (devm_ioremap_resource) from [<8091c6e4>] (fsi_master_acf_probe+0x1a8/0x5d8)
[    2.869909] [<8091c53c>] (fsi_master_acf_probe) from [<80723dbc>] (platform_probe+0x68/0xc8)
[    2.870124]  r9:80e9dadc r8:00000000 r7:815d1010 r6:810c1000 r5:815d1010 r4:00000000
[    2.870306] [<80723d54>] (platform_probe) from [<80721208>] (really_probe+0x1cc/0x470)
[    2.870512]  r7:815d1010 r6:810c1000 r5:00000000 r4:815d1010
[    2.870651] [<8072103c>] (really_probe) from [<807215cc>] (__driver_probe_device+0x120/0x1fc)
[    2.870872]  r7:815d1010 r6:810c1000 r5:810c1000 r4:815d1010
[    2.871013] [<807214ac>] (__driver_probe_device) from [<807216e8>] (driver_probe_device+0x40/0xd8)
[    2.871244]  r9:80e9dadc r8:00000000 r7:815d1010 r6:810c1000 r5:812feaa0 r4:812fe994
[    2.871428] [<807216a8>] (driver_probe_device) from [<80721a58>] (__driver_attach+0xa8/0x1d4)
[    2.871647]  r9:80e9dadc r8:00000000 r7:00000000 r6:810c1000 r5:815d1054 r4:815d1010
[    2.871830] [<807219b0>] (__driver_attach) from [<8071ee8c>] (bus_for_each_dev+0x88/0xc8)
[    2.872040]  r7:00000000 r6:814c4000 r5:807219b0 r4:810c1000
[    2.872194] [<8071ee04>] (bus_for_each_dev) from [<80722208>] (driver_attach+0x28/0x30)
[    2.872418]  r7:810a2aa0 r6:00000000 r5:821b6000 r4:810c1000
[    2.872570] [<807221e0>] (driver_attach) from [<8071f80c>] (bus_add_driver+0x114/0x200)
[    2.872788] [<8071f6f8>] (bus_add_driver) from [<80722ec4>] (driver_register+0x98/0x128)
[    2.873011]  r7:81011d0c r6:814c4000 r5:00000000 r4:810c1000
[    2.873167] [<80722e2c>] (driver_register) from [<80725240>] (__platform_driver_register+0x2c/0x34)
[    2.873408]  r5:814dcb80 r4:80f2a764
[    2.873513] [<80725214>] (__platform_driver_register) from [<80f2a784>] (fsi_master_acf_init+0x20/0x28)
[    2.873766] [<80f2a764>] (fsi_master_acf_init) from [<80f014a8>] (do_one_initcall+0x108/0x290)
[    2.874007] [<80f013a0>] (do_one_initcall) from [<80f01840>] (kernel_init_freeable+0x1ac/0x230)
[    2.874248]  r9:80e9dadc r8:80f3987c r7:80f3985c r6:00000007 r5:814dcb80 r4:80f627a4
[    2.874456] [<80f01694>] (kernel_init_freeable) from [<80b19f44>] (kernel_init+0x20/0x138)
[    2.874691]  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:80b19f24
[    2.874894]  r4:00000000
[    2.874977] [<80b19f24>] (kernel_init) from [<80100170>] (ret_from_fork+0x14/0x24)
[    2.875231] Exception stack(0x814c5fb0 to 0x814c5ff8)
[    2.875535] 5fa0:                                     00000000 00000000 00000000 00000000
[    2.875849] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    2.876133] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    2.876363]  r5:80b19f24 r4:00000000
[    2.876683] ---[ end trace b2f74b8536829970 ]---
[    2.876911] fsi-master-acf gpio-fsi: ioremap failed for resource [mem 0x9ef00000-0x9effffff]
[    2.877492] fsi-master-acf gpio-fsi: Error -12 mapping coldfire memory
[    2.877689] fsi-master-acf: probe of gpio-fsi failed with error -12

Use memblock_is_map_memory() instead of pfn_valid() to check if a PFN is in
RAM or not.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
---
 arch/arm/mm/ioremap.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index 000e8210000b..80fb5a4a5c05 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -27,6 +27,7 @@
 #include <linux/vmalloc.h>
 #include <linux/io.h>
 #include <linux/sizes.h>
+#include <linux/memblock.h>
 
 #include <asm/cp15.h>
 #include <asm/cputype.h>
@@ -284,7 +285,8 @@ static void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
 	 * Don't allow RAM to be mapped with mismatched attributes - this
 	 * causes problems with ARMv6+
 	 */
-	if (WARN_ON(pfn_valid(pfn) && mtype != MT_MEMORY_RW))
+	if (WARN_ON(memblock_is_map_memory(PFN_PHYS(pfn)) &&
+		    mtype != MT_MEMORY_RW))
 		return NULL;
 
 	area = get_vm_area_caller(size, VM_IOREMAP, caller);
-- 
2.28.0

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 4/4] arm: extend pfn_valid to take into account freed memory map alignment
  2021-07-05  7:23     ` Mike Rapoport
@ 2021-07-05 14:55       ` Guenter Roeck
  2021-07-09  4:56       ` Alexey Minnekhanov
  1 sibling, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2021-07-05 14:55 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: linux-arm-kernel, Andrew Morton, Kefeng Wang, Mike Rapoport,
	Russell King, Tony Lindgren, linux-kernel, linux-mm

On 7/5/21 12:23 AM, Mike Rapoport wrote:
> Hi Guenter,
> 
> On Sun, Jul 04, 2021 at 09:22:36PM -0700, Guenter Roeck wrote:
>> On Wed, Jun 30, 2021 at 10:12:11AM +0300, Mike Rapoport wrote:
>>> From: Mike Rapoport <rppt@linux.ibm.com>
>>>
>>> When unused memory map is freed the preserved part of the memory map is
>>> extended to match pageblock boundaries because lots of core mm
>>> functionality relies on homogeneity of the memory map within pageblock
>>> boundaries.
>>>
>>> Since pfn_valid() is used to check whether there is a valid memory map
>>> entry for a PFN, make it return true also for PFNs that have memory map
>>> entries even if there is no actual memory populated there.
>>>
>>> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
>>> Tested-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>
>> With this patch in place, the romulus-bmc emulation in qemu gets the
>> following traceback:
>>
>> [    2.863406] WARNING: CPU: 0 PID: 1 at arch/arm/mm/ioremap.c:287 __arm_ioremap_pfn_caller+0xf0/0x1dc
> ...
>> [    2.876683] ---[ end trace b2f74b8536829970 ]---
>> [    2.876911] fsi-master-acf gpio-fsi: ioremap failed for resource [mem 0x9ef00000-0x9effffff]
>> [    2.877492] fsi-master-acf gpio-fsi: Error -12 mapping coldfire memory
>> [    2.877689] fsi-master-acf: probe of gpio-fsi failed with error -12
>>
>> Reverting it fixes the problem. Also, the ioremap failure is no longer seen
>> after reverting this patch.
> 
> I believe this should fix it:
> 
Yes, it does.

>>From e2213b7e804daf0d31d47502379916f0542398bb Mon Sep 17 00:00:00 2001
> From: Mike Rapoport <rppt@linux.ibm.com>
> Date: Mon, 5 Jul 2021 08:43:10 +0300
> Subject: [PATCH] arm: ioremap: don't abuse pfn_valid() to check if pfn is in RAM
> 
> The semantics of pfn_valid() is to check presence of the memory map for a
> PFN and not whether a PFN is in RAM. The memory map may be present for a
> hole in the physical memory and if such hole corresponds to an MMIO range,
> __arm_ioremap_pfn_caller() will produce a WARN() and fail:
> 
> [    2.863406] WARNING: CPU: 0 PID: 1 at arch/arm/mm/ioremap.c:287 __arm_ioremap_pfn_caller+0xf0/0x1dc
> [    2.864812] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-09882-ga180bd1d7e16 #1
> [    2.865263] Hardware name: Generic DT based system
> [    2.865711] Backtrace:
> [    2.866063] [<80b07e58>] (dump_backtrace) from [<80b080ac>] (show_stack+0x20/0x24)
> [    2.866633]  r7:00000009 r6:0000011f r5:60000153 r4:80ddd1c0
> [    2.866922] [<80b0808c>] (show_stack) from [<80b18df0>] (dump_stack_lvl+0x58/0x74)
> [    2.867117] [<80b18d98>] (dump_stack_lvl) from [<80b18e20>] (dump_stack+0x14/0x1c)
> [    2.867309]  r5:80118cac r4:80dc6774
> [    2.867404] [<80b18e0c>] (dump_stack) from [<80122fcc>] (__warn+0xe4/0x150)
> [    2.867583] [<80122ee8>] (__warn) from [<80b08850>] (warn_slowpath_fmt+0x88/0xc0)
> [    2.867774]  r7:0000011f r6:80dc6774 r5:00000000 r4:814c4000
> [    2.867917] [<80b087cc>] (warn_slowpath_fmt) from [<80118cac>] (__arm_ioremap_pfn_caller+0xf0/0x1dc)
> [    2.868158]  r9:00000001 r8:9ef00000 r7:80e8b0d4 r6:0009ef00 r5:00000000 r4:00100000
> [    2.868346] [<80118bbc>] (__arm_ioremap_pfn_caller) from [<80118df8>] (__arm_ioremap_caller+0x60/0x68)
> [    2.868581]  r9:9ef00000 r8:821b6dc0 r7:00100000 r6:00000000 r5:815d1010 r4:80118d98
> [    2.868761] [<80118d98>] (__arm_ioremap_caller) from [<80118fcc>] (ioremap+0x28/0x30)
> [    2.868958] [<80118fa4>] (ioremap) from [<8062871c>] (__devm_ioremap_resource+0x154/0x1c8)
> [    2.869169]  r5:815d1010 r4:814c5d2c
> [    2.869263] [<806285c8>] (__devm_ioremap_resource) from [<8062899c>] (devm_ioremap_resource+0x14/0x18)
> [    2.869495]  r9:9e9f57a0 r8:814c4000 r7:815d1000 r6:815d1010 r5:8177c078 r4:815cf400
> [    2.869676] [<80628988>] (devm_ioremap_resource) from [<8091c6e4>] (fsi_master_acf_probe+0x1a8/0x5d8)
> [    2.869909] [<8091c53c>] (fsi_master_acf_probe) from [<80723dbc>] (platform_probe+0x68/0xc8)
> [    2.870124]  r9:80e9dadc r8:00000000 r7:815d1010 r6:810c1000 r5:815d1010 r4:00000000
> [    2.870306] [<80723d54>] (platform_probe) from [<80721208>] (really_probe+0x1cc/0x470)
> [    2.870512]  r7:815d1010 r6:810c1000 r5:00000000 r4:815d1010
> [    2.870651] [<8072103c>] (really_probe) from [<807215cc>] (__driver_probe_device+0x120/0x1fc)
> [    2.870872]  r7:815d1010 r6:810c1000 r5:810c1000 r4:815d1010
> [    2.871013] [<807214ac>] (__driver_probe_device) from [<807216e8>] (driver_probe_device+0x40/0xd8)
> [    2.871244]  r9:80e9dadc r8:00000000 r7:815d1010 r6:810c1000 r5:812feaa0 r4:812fe994
> [    2.871428] [<807216a8>] (driver_probe_device) from [<80721a58>] (__driver_attach+0xa8/0x1d4)
> [    2.871647]  r9:80e9dadc r8:00000000 r7:00000000 r6:810c1000 r5:815d1054 r4:815d1010
> [    2.871830] [<807219b0>] (__driver_attach) from [<8071ee8c>] (bus_for_each_dev+0x88/0xc8)
> [    2.872040]  r7:00000000 r6:814c4000 r5:807219b0 r4:810c1000
> [    2.872194] [<8071ee04>] (bus_for_each_dev) from [<80722208>] (driver_attach+0x28/0x30)
> [    2.872418]  r7:810a2aa0 r6:00000000 r5:821b6000 r4:810c1000
> [    2.872570] [<807221e0>] (driver_attach) from [<8071f80c>] (bus_add_driver+0x114/0x200)
> [    2.872788] [<8071f6f8>] (bus_add_driver) from [<80722ec4>] (driver_register+0x98/0x128)
> [    2.873011]  r7:81011d0c r6:814c4000 r5:00000000 r4:810c1000
> [    2.873167] [<80722e2c>] (driver_register) from [<80725240>] (__platform_driver_register+0x2c/0x34)
> [    2.873408]  r5:814dcb80 r4:80f2a764
> [    2.873513] [<80725214>] (__platform_driver_register) from [<80f2a784>] (fsi_master_acf_init+0x20/0x28)
> [    2.873766] [<80f2a764>] (fsi_master_acf_init) from [<80f014a8>] (do_one_initcall+0x108/0x290)
> [    2.874007] [<80f013a0>] (do_one_initcall) from [<80f01840>] (kernel_init_freeable+0x1ac/0x230)
> [    2.874248]  r9:80e9dadc r8:80f3987c r7:80f3985c r6:00000007 r5:814dcb80 r4:80f627a4
> [    2.874456] [<80f01694>] (kernel_init_freeable) from [<80b19f44>] (kernel_init+0x20/0x138)
> [    2.874691]  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:80b19f24
> [    2.874894]  r4:00000000
> [    2.874977] [<80b19f24>] (kernel_init) from [<80100170>] (ret_from_fork+0x14/0x24)
> [    2.875231] Exception stack(0x814c5fb0 to 0x814c5ff8)
> [    2.875535] 5fa0:                                     00000000 00000000 00000000 00000000
> [    2.875849] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    2.876133] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [    2.876363]  r5:80b19f24 r4:00000000
> [    2.876683] ---[ end trace b2f74b8536829970 ]---
> [    2.876911] fsi-master-acf gpio-fsi: ioremap failed for resource [mem 0x9ef00000-0x9effffff]
> [    2.877492] fsi-master-acf gpio-fsi: Error -12 mapping coldfire memory
> [    2.877689] fsi-master-acf: probe of gpio-fsi failed with error -12
> 
> Use memblock_is_map_memory() instead of pfn_valid() to check if a PFN is in
> RAM or not.
> 
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>

Tested-by: Guenter Roeck <linux@roeck-us.net>

Thanks!
Guenter

> ---
>   arch/arm/mm/ioremap.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
> index 000e8210000b..80fb5a4a5c05 100644
> --- a/arch/arm/mm/ioremap.c
> +++ b/arch/arm/mm/ioremap.c
> @@ -27,6 +27,7 @@
>   #include <linux/vmalloc.h>
>   #include <linux/io.h>
>   #include <linux/sizes.h>
> +#include <linux/memblock.h>
>   
>   #include <asm/cp15.h>
>   #include <asm/cputype.h>
> @@ -284,7 +285,8 @@ static void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
>   	 * Don't allow RAM to be mapped with mismatched attributes - this
>   	 * causes problems with ARMv6+
>   	 */
> -	if (WARN_ON(pfn_valid(pfn) && mtype != MT_MEMORY_RW))
> +	if (WARN_ON(memblock_is_map_memory(PFN_PHYS(pfn)) &&
> +		    mtype != MT_MEMORY_RW))
>   		return NULL;
>   
>   	area = get_vm_area_caller(size, VM_IOREMAP, caller);
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 4/4] arm: extend pfn_valid to take into account freed memory map alignment
  2021-07-05  7:23     ` Mike Rapoport
  2021-07-05 14:55       ` Guenter Roeck
@ 2021-07-09  4:56       ` Alexey Minnekhanov
  1 sibling, 0 replies; 12+ messages in thread
From: Alexey Minnekhanov @ 2021-07-09  4:56 UTC (permalink / raw)
  To: Mike Rapoport, Guenter Roeck
  Cc: linux-arm-kernel, Andrew Morton, Kefeng Wang, Mike Rapoport,
	Russell King, Tony Lindgren, linux-kernel, linux-mm


05.07.2021 10:23, Mike Rapoport wrote:
> 
> I believe this should fix it:
> 
>>From e2213b7e804daf0d31d47502379916f0542398bb Mon Sep 17 00:00:00 2001
> From: Mike Rapoport <rppt@linux.ibm.com>
> Date: Mon, 5 Jul 2021 08:43:10 +0300
> Subject: [PATCH] arm: ioremap: don't abuse pfn_valid() to check if pfn is in RAM
> 
> The semantics of pfn_valid() is to check presence of the memory map for a
> PFN and not whether a PFN is in RAM. The memory map may be present for a
> hole in the physical memory and if such hole corresponds to an MMIO range,
> __arm_ioremap_pfn_caller() will produce a WARN() and fail:
> 
> [    2.863406] WARNING: CPU: 0 PID: 1 at arch/arm/mm/ioremap.c:287 __arm_ioremap_pfn_caller+0xf0/0x1dc
> [    2.864812] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.13.0-09882-ga180bd1d7e16 #1
> [    2.865263] Hardware name: Generic DT based system
> [    2.865711] Backtrace:
> [    2.866063] [<80b07e58>] (dump_backtrace) from [<80b080ac>] (show_stack+0x20/0x24)
> [    2.866633]  r7:00000009 r6:0000011f r5:60000153 r4:80ddd1c0
> [    2.866922] [<80b0808c>] (show_stack) from [<80b18df0>] (dump_stack_lvl+0x58/0x74)
> [    2.867117] [<80b18d98>] (dump_stack_lvl) from [<80b18e20>] (dump_stack+0x14/0x1c)
> [    2.867309]  r5:80118cac r4:80dc6774
> [    2.867404] [<80b18e0c>] (dump_stack) from [<80122fcc>] (__warn+0xe4/0x150)
> [    2.867583] [<80122ee8>] (__warn) from [<80b08850>] (warn_slowpath_fmt+0x88/0xc0)
> [    2.867774]  r7:0000011f r6:80dc6774 r5:00000000 r4:814c4000
> [    2.867917] [<80b087cc>] (warn_slowpath_fmt) from [<80118cac>] (__arm_ioremap_pfn_caller+0xf0/0x1dc)
> [    2.868158]  r9:00000001 r8:9ef00000 r7:80e8b0d4 r6:0009ef00 r5:00000000 r4:00100000
> [    2.868346] [<80118bbc>] (__arm_ioremap_pfn_caller) from [<80118df8>] (__arm_ioremap_caller+0x60/0x68)
> [    2.868581]  r9:9ef00000 r8:821b6dc0 r7:00100000 r6:00000000 r5:815d1010 r4:80118d98
> [    2.868761] [<80118d98>] (__arm_ioremap_caller) from [<80118fcc>] (ioremap+0x28/0x30)
> [    2.868958] [<80118fa4>] (ioremap) from [<8062871c>] (__devm_ioremap_resource+0x154/0x1c8)
> [    2.869169]  r5:815d1010 r4:814c5d2c
> [    2.869263] [<806285c8>] (__devm_ioremap_resource) from [<8062899c>] (devm_ioremap_resource+0x14/0x18)
> [    2.869495]  r9:9e9f57a0 r8:814c4000 r7:815d1000 r6:815d1010 r5:8177c078 r4:815cf400
> [    2.869676] [<80628988>] (devm_ioremap_resource) from [<8091c6e4>] (fsi_master_acf_probe+0x1a8/0x5d8)
> [    2.869909] [<8091c53c>] (fsi_master_acf_probe) from [<80723dbc>] (platform_probe+0x68/0xc8)
> [    2.870124]  r9:80e9dadc r8:00000000 r7:815d1010 r6:810c1000 r5:815d1010 r4:00000000
> [    2.870306] [<80723d54>] (platform_probe) from [<80721208>] (really_probe+0x1cc/0x470)
> [    2.870512]  r7:815d1010 r6:810c1000 r5:00000000 r4:815d1010
> [    2.870651] [<8072103c>] (really_probe) from [<807215cc>] (__driver_probe_device+0x120/0x1fc)
> [    2.870872]  r7:815d1010 r6:810c1000 r5:810c1000 r4:815d1010
> [    2.871013] [<807214ac>] (__driver_probe_device) from [<807216e8>] (driver_probe_device+0x40/0xd8)
> [    2.871244]  r9:80e9dadc r8:00000000 r7:815d1010 r6:810c1000 r5:812feaa0 r4:812fe994
> [    2.871428] [<807216a8>] (driver_probe_device) from [<80721a58>] (__driver_attach+0xa8/0x1d4)
> [    2.871647]  r9:80e9dadc r8:00000000 r7:00000000 r6:810c1000 r5:815d1054 r4:815d1010
> [    2.871830] [<807219b0>] (__driver_attach) from [<8071ee8c>] (bus_for_each_dev+0x88/0xc8)
> [    2.872040]  r7:00000000 r6:814c4000 r5:807219b0 r4:810c1000
> [    2.872194] [<8071ee04>] (bus_for_each_dev) from [<80722208>] (driver_attach+0x28/0x30)
> [    2.872418]  r7:810a2aa0 r6:00000000 r5:821b6000 r4:810c1000
> [    2.872570] [<807221e0>] (driver_attach) from [<8071f80c>] (bus_add_driver+0x114/0x200)
> [    2.872788] [<8071f6f8>] (bus_add_driver) from [<80722ec4>] (driver_register+0x98/0x128)
> [    2.873011]  r7:81011d0c r6:814c4000 r5:00000000 r4:810c1000
> [    2.873167] [<80722e2c>] (driver_register) from [<80725240>] (__platform_driver_register+0x2c/0x34)
> [    2.873408]  r5:814dcb80 r4:80f2a764
> [    2.873513] [<80725214>] (__platform_driver_register) from [<80f2a784>] (fsi_master_acf_init+0x20/0x28)
> [    2.873766] [<80f2a764>] (fsi_master_acf_init) from [<80f014a8>] (do_one_initcall+0x108/0x290)
> [    2.874007] [<80f013a0>] (do_one_initcall) from [<80f01840>] (kernel_init_freeable+0x1ac/0x230)
> [    2.874248]  r9:80e9dadc r8:80f3987c r7:80f3985c r6:00000007 r5:814dcb80 r4:80f627a4
> [    2.874456] [<80f01694>] (kernel_init_freeable) from [<80b19f44>] (kernel_init+0x20/0x138)
> [    2.874691]  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:80b19f24
> [    2.874894]  r4:00000000
> [    2.874977] [<80b19f24>] (kernel_init) from [<80100170>] (ret_from_fork+0x14/0x24)
> [    2.875231] Exception stack(0x814c5fb0 to 0x814c5ff8)
> [    2.875535] 5fa0:                                     00000000 00000000 00000000 00000000
> [    2.875849] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [    2.876133] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [    2.876363]  r5:80b19f24 r4:00000000
> [    2.876683] ---[ end trace b2f74b8536829970 ]---
> [    2.876911] fsi-master-acf gpio-fsi: ioremap failed for resource [mem 0x9ef00000-0x9effffff]
> [    2.877492] fsi-master-acf gpio-fsi: Error -12 mapping coldfire memory
> [    2.877689] fsi-master-acf: probe of gpio-fsi failed with error -12
> 
> Use memblock_is_map_memory() instead of pfn_valid() to check if a PFN is in
> RAM or not.
> 
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
>   arch/arm/mm/ioremap.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
> index 000e8210000b..80fb5a4a5c05 100644
> --- a/arch/arm/mm/ioremap.c
> +++ b/arch/arm/mm/ioremap.c
> @@ -27,6 +27,7 @@
>   #include <linux/vmalloc.h>
>   #include <linux/io.h>
>   #include <linux/sizes.h>
> +#include <linux/memblock.h>
>   
>   #include <asm/cp15.h>
>   #include <asm/cputype.h>
> @@ -284,7 +285,8 @@ static void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
>   	 * Don't allow RAM to be mapped with mismatched attributes - this
>   	 * causes problems with ARMv6+
>   	 */
> -	if (WARN_ON(pfn_valid(pfn) && mtype != MT_MEMORY_RW))
> +	if (WARN_ON(memblock_is_map_memory(PFN_PHYS(pfn)) &&
> +		    mtype != MT_MEMORY_RW))
>   		return NULL;
>   
>   	area = get_vm_area_caller(size, VM_IOREMAP, caller);
> 

I got similar stack trace on Qualcomm msm8974-based board [1]
and can confirm that the above patch fixes the problem.

[1] https://paste.sr.ht/~minlexx/9d1eb96034b85eb8d1d86a46435d111a32e87775

-- 
Regards
Alexey Minnekhanov
postmarketOS developer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map
  2021-06-30  7:12 [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map Mike Rapoport
                   ` (4 preceding siblings ...)
  2021-06-30  8:26 ` [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map Tony Lindgren
@ 2021-11-11  7:33 ` Mark-PK Tsai
  2021-11-11  9:45   ` Mike Rapoport
  5 siblings, 1 reply; 12+ messages in thread
From: Mark-PK Tsai @ 2021-11-11  7:33 UTC (permalink / raw)
  To: rppt
  Cc: akpm, linux-arm-kernel, linux-kernel, linux-mm, linux, rppt,
	tony, wangkefeng.wang, mark-pk.tsai, yj.chiang

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!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map
  2021-11-11  7:33 ` Mark-PK Tsai
@ 2021-11-11  9:45   ` Mike Rapoport
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Rapoport @ 2021-11-11  9:45 UTC (permalink / raw)
  To: Mark-PK Tsai
  Cc: akpm, linux-arm-kernel, linux-kernel, linux-mm, linux, rppt,
	tony, wangkefeng.wang, yj.chiang

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!

-- 
Sincerely yours,
Mike.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-11-11  9:47 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-30  7:12 [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map Mike Rapoport
2021-06-30  7:12 ` [PATCH v3 1/4] memblock: free_unused_memmap: use pageblock units instead of MAX_ORDER Mike Rapoport
2021-06-30  7:12 ` [PATCH v3 2/4] memblock: align freed memory map on pageblock boundaries with SPARSEMEM Mike Rapoport
2021-06-30  7:12 ` [PATCH v3 3/4] memblock: ensure there is no overflow in memblock_overlaps_region() Mike Rapoport
2021-06-30  7:12 ` [PATCH v3 4/4] arm: extend pfn_valid to take into account freed memory map alignment Mike Rapoport
2021-07-05  4:22   ` Guenter Roeck
2021-07-05  7:23     ` Mike Rapoport
2021-07-05 14:55       ` Guenter Roeck
2021-07-09  4:56       ` Alexey Minnekhanov
2021-06-30  8:26 ` [PATCH v3 0/4] memblock, arm: fixes for freeing of the memory map Tony Lindgren
2021-11-11  7:33 ` Mark-PK Tsai
2021-11-11  9:45   ` Mike Rapoport

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).