Re: [PATCH v4 7/7] mm/mm_init: Use for_each_valid_pfn() in init_unavailable_range()

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

Andrew - can we drop this from mm-new? It's breaking it.

David, this seems to break on qemu boot for me in Andrew's mm-new branch,
bisected to this commit.

Splat on a basic x86 defconfig variant:

[    0.029481] BUG: unable to handle page fault for address: ffffea0003000034
[    0.029840] #PF: supervisor write access in kernel mode
[    0.030089] #PF: error_code(0x0002) - not-present page
[    0.030327] PGD 26ccc3067 P4D 26ccc3067 PUD 26ccc2067 PMD 0
[    0.030599] Oops: Oops: 0002 [#1] SMP NOPTI
[    0.030794] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.15.0-rc2+ #9 PREEMPT(undef)
[    0.031177] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.3-1-1 04/01/2014
[    0.031610] RIP: 0010:__init_single_page+0xa/0x50

__init_single_page+0xa/0x50:
arch_atomic_set at arch/x86/include/asm/atomic.h:28
(inlined by) raw_atomic_set at include/linux/atomic/atomic-arch-fallback.h:503
(inlined by) atomic_set at include/linux/atomic/atomic-instrumented.h:68
(inlined by) set_page_count at include/linux/page_ref.h:99
(inlined by) init_page_count at include/linux/page_ref.h:115
(inlined by) __init_single_page at mm/mm_init.c:586

^-- faddr2line decode

[    0.031832] Code: ff e9 0a 06 e4 fe 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 317
[    0.032710] RSP: 0000:ffffffff82a03da8 EFLAGS: 00010016
[    0.032954] RAX: 0000000000000000 RBX: 00000000000c0000 RCX: 0000000000000000
[    0.033287] RDX: 0200000000000000 RSI: 00000000000c0000 RDI: ffffea0003000000
[    0.033614] RBP: 0000000000100000 R08: 0000000000000000 R09: ffffea0009b30000
[    0.034186] R10: 0000000000000000 R11: 0000000000100000 R12: 0000000000000002
[    0.034519] R13: 0000000000000000 R14: 0000000000000023 R15: 0000000003000000
[    0.034856] FS:  0000000000000000(0000) GS:0000000000000000(0000) knlGS:0000000000000000
[    0.035240] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.035509] CR2: ffffea0003000034 CR3: 0000000002a32000 CR4: 00000000000000b0
[    0.035846] Call Trace:
[    0.035961]  <TASK>
[    0.036070]  ? init_unavailable_range+0x42/0xb0

	for_each_valid_pfn(pfn, spfn, epfn) {
  		__init_single_page(pfn_to_page(pfn), pfn, zone, node); <--- this is here.

[    0.036284]  ? free_area_init+0xd70/0xe30
[    0.036473]  ? zone_sizes_init+0x44/0x50
[    0.036657]  ? setup_arch+0x9a8/0xa80
[    0.036831]  ? start_kernel+0x58/0x6c0
[    0.037010]  ? x86_64_start_reservations+0x24/0x30
[    0.037236]  ? x86_64_start_kernel+0x8c/0x90
[    0.037439]  ? common_startup_64+0x13e/0x148
[    0.037642]  </TASK>

Cheers, Lorenzo

On Wed, Apr 23, 2025 at 02:33:43PM +0100, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> Currently, memmap_init initializes pfn_hole with 0 instead of
> ARCH_PFN_OFFSET. Then init_unavailable_range will start iterating each
> page from the page at address zero to the first available page, but it
> won't do anything for pages below ARCH_PFN_OFFSET because pfn_valid
> won't pass.
>
> If ARCH_PFN_OFFSET is very large (e.g., something like 2^64-2GiB if the
> kernel is used as a library and loaded at a very high address), the
> pointless iteration for pages below ARCH_PFN_OFFSET will take a very
> long time, and the kernel will look stuck at boot time.
>
> Use for_each_valid_pfn() to skip the pointless iterations.
>
> Reported-by: Ruihan Li <lrh2000@pku.edu.cn>
> Suggested-by: Mike Rapoport <rppt@kernel.org>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> Tested-by: Ruihan Li <lrh2000@pku.edu.cn>
> ---
>  mm/mm_init.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index 41884f2155c4..0d1a4546825c 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -845,11 +845,7 @@ static void __init init_unavailable_range(unsigned long spfn,
>  	unsigned long pfn;
>  	u64 pgcnt = 0;
>
> -	for (pfn = spfn; pfn < epfn; pfn++) {
> -		if (!pfn_valid(pageblock_start_pfn(pfn))) {
> -			pfn = pageblock_end_pfn(pfn) - 1;
> -			continue;
> -		}
> +	for_each_valid_pfn(pfn, spfn, epfn) {
>  		__init_single_page(pfn_to_page(pfn), pfn, zone, node);
>  		__SetPageReserved(pfn_to_page(pfn));
>  		pgcnt++;
> --
> 2.49.0
>
>