On Thu, Aug 21, 2025 at 10:06:27PM +0200, David Hildenbrand wrote:

In an ideal world, we wouldn't have to deal with SPARSEMEM without SPARSEMEM_VMEMMAP, but in particular for 32bit SPARSEMEM_VMEMMAP is considered too costly and consequently not supported.

However, if an architecture does support SPARSEMEM with SPARSEMEM_VMEMMAP, let's forbid the user to disable VMEMMAP: just like we already do for arm64, s390 and x86.

So if SPARSEMEM_VMEMMAP is supported, don't allow to use SPARSEMEM without SPARSEMEM_VMEMMAP.

This implies that the option to not use SPARSEMEM_VMEMMAP will now be gone for loongarch, powerpc, riscv and sparc. All architectures only enable SPARSEMEM_VMEMMAP with 64bit support, so there should not really be a big downside to using the VMEMMAP (quite the contrary).

This is a preparation for not supporting

(1) folio sizes that exceed a single memory section (2) CMA allocations of non-contiguous page ranges

in SPARSEMEM without SPARSEMEM_VMEMMAP configs, whereby we want to limit possible impact as much as possible (e.g., gigantic hugetlb page allocations suddenly fails).

Cc: Huacai Chen chenhuacai@kernel.org Cc: WANG Xuerui kernel@xen0n.name Cc: Madhavan Srinivasan maddy@linux.ibm.com Cc: Michael Ellerman mpe@ellerman.id.au Cc: Nicholas Piggin npiggin@gmail.com Cc: Christophe Leroy christophe.leroy@csgroup.eu Cc: Paul Walmsley paul.walmsley@sifive.com Cc: Palmer Dabbelt palmer@dabbelt.com Cc: Albert Ou aou@eecs.berkeley.edu Cc: Alexandre Ghiti alex@ghiti.fr Cc: "David S. Miller" davem@davemloft.net Cc: Andreas Larsson andreas@gaisler.com Signed-off-by: David Hildenbrand david@redhat.com

Acked-by: Mike Rapoport (Microsoft) rppt@kernel.org

---

mm/Kconfig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/Kconfig b/mm/Kconfig index 4108bcd967848..330d0e698ef96 100644 --- a/mm/Kconfig +++ b/mm/Kconfig @@ -439,9 +439,8 @@ config SPARSEMEM_VMEMMAP_ENABLE bool config SPARSEMEM_VMEMMAP

• bool "Sparse Memory virtual memmap"

• def_bool y depends on SPARSEMEM && SPARSEMEM_VMEMMAP_ENABLE

• default y help SPARSEMEM_VMEMMAP uses a virtually mapped memmap to optimise pfn_to_page and page_to_pfn operations. This is the most

-- 2.50.1

On Thu, Aug 21, 2025 at 10:06:35PM +0200, David Hildenbrand wrote:

Grepping for "prep_compound_page" leaves on clueless how devdax gets its compound pages initialized.

Let's add a comment that might help finding this open-coded prep_compound_page() initialization more easily.

Further, let's be less smart about the ordering of initialization and just perform the prep_compound_head() call after all tail pages were initialized: just like prep_compound_page() does.

No need for a lengthy comment then: again, just like prep_compound_page().

Note that prep_compound_head() already does initialize stuff in page[2] through prep_compound_head() that successive tail page initialization will overwrite: _deferred_list, and on 32bit _entire_mapcount and _pincount. Very likely 32bit does not apply, and likely nobody ever ends up testing whether the _deferred_list is empty.

So it shouldn't be a fix at this point, but certainly something to clean up.

Signed-off-by: David Hildenbrand david@redhat.com

mm/mm_init.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/mm/mm_init.c b/mm/mm_init.c index 5c21b3af216b2..708466c5b2cc9 100644 --- a/mm/mm_init.c +++ b/mm/mm_init.c @@ -1091,6 +1091,10 @@ static void __ref memmap_init_compound(struct page *head, unsigned long pfn, end_pfn = head_pfn + nr_pages; unsigned int order = pgmap->vmemmap_shift;

• /*

• * This is an open-coded prep_compound_page() whereby we avoid
  

• * walking pages twice by initializing them in the same go.
  

• */
  

While on it, can you also mention that prep_compound_page() is not used to properly set page zone link?

With this

Reviewed-by: Mike Rapoport (Microsoft) rppt@kernel.org

__SetPageHead(head); for (pfn = head_pfn + 1; pfn < end_pfn; pfn++) { struct page *page = pfn_to_page(pfn); @@ -1098,15 +1102,8 @@ static void __ref memmap_init_compound(struct page *head, __init_zone_device_page(page, pfn, zone_idx, nid, pgmap); prep_compound_tail(head, pfn - head_pfn); set_page_count(page, 0);

• /*
  

•  * The first tail page stores important compound page info.
  

•  * Call prep_compound_head() after the first tail page has
  

•  * been initialized, to not have the data overwritten.
  

•  */
  

• if (pfn == head_pfn + 1)
  

• 	prep_compound_head(head, order);
  

  }

• prep_compound_head(head, order);

} void __ref memmap_init_zone_device(struct zone *zone, -- 2.50.1

On 25.08.25 16:59, Mike Rapoport wrote:

On Mon, Aug 25, 2025 at 04:38:03PM +0200, David Hildenbrand wrote:

...

On 25.08.25 16:32, Mike Rapoport wrote:

...

On Mon, Aug 25, 2025 at 02:48:58PM +0200, David Hildenbrand wrote:

...

On 23.08.25 10:59, Mike Rapoport wrote:

...

On Fri, Aug 22, 2025 at 08:24:31AM +0200, David Hildenbrand wrote:

...

On 22.08.25 06:09, Mika Penttilä wrote: > > On 8/21/25 23:06, David Hildenbrand wrote: > >> All pages were already initialized and set to PageReserved() with a >> refcount of 1 by MM init code. > > Just to be sure, how is this working with MEMBLOCK_RSRV_NOINIT, where MM is supposed not to > initialize struct pages?

Excellent point, I did not know about that one.

Spotting that we don't do the same for the head page made me assume that it's just a misuse of __init_single_page().

But the nasty thing is that we use memblock_reserved_mark_noinit() to only mark the tail pages ...

And even nastier thing is that when CONFIG_DEFERRED_STRUCT_PAGE_INIT is disabled struct pages are initialized regardless of memblock_reserved_mark_noinit().

I think this patch should go in before your updates:

Shouldn't we fix this in memblock code?

Hacking around that in the memblock_reserved_mark_noinit() user sound wrong -- and nothing in the doc of memblock_reserved_mark_noinit() spells that behavior out.

We can surely update the docs, but unfortunately I don't see how to avoid hacking around it in hugetlb. Since it's used to optimise HVO even further to the point hugetlb open codes memmap initialization, I think it's fair that it should deal with all possible configurations.

Remind me, why can't we support memblock_reserved_mark_noinit() when CONFIG_DEFERRED_STRUCT_PAGE_INIT is disabled?

When CONFIG_DEFERRED_STRUCT_PAGE_INIT is disabled we initialize the entire memmap early (setup_arch()->free_area_init()), and we may have a bunch of memblock_reserved_mark_noinit() afterwards

Oh, you mean that we get effective memblock modifications after already initializing the memmap.

That sounds ... interesting :)

So yeah, we have to document this for memblock_reserved_mark_noinit().

Is it also a problem for kexec_handover?

We should do something like:

diff --git a/mm/memblock.c b/mm/memblock.c index 154f1d73b61f2..ed4c563d72c32 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -1091,13 +1091,16 @@ int __init_memblock memblock_clear_nomap(phys_addr_t base, phys_addr_t size)

/** * memblock_reserved_mark_noinit - Mark a reserved memory region with flag - * MEMBLOCK_RSRV_NOINIT which results in the struct pages not being initialized - * for this region. + * MEMBLOCK_RSRV_NOINIT which allows for the "struct pages" corresponding + * to this region not getting initialized, because the caller will take + * care of it. * @base: the base phys addr of the region * @size: the size of the region * - * struct pages will not be initialized for reserved memory regions marked with - * %MEMBLOCK_RSRV_NOINIT. + * "struct pages" will not be initialized for reserved memory regions marked + * with %MEMBLOCK_RSRV_NOINIT if this function is called before initialization + * code runs. Without CONFIG_DEFERRED_STRUCT_PAGE_INIT, it is more likely + * that this function is not effective. * * Return: 0 on success, -errno on failure. */

Optimizing the hugetlb code could be done, but I am not sure how high the priority is (nobody complained so far about the double init).

...

We should do something like:

diff --git a/mm/memblock.c b/mm/memblock.c index 154f1d73b61f2..ed4c563d72c32 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -1091,13 +1091,16 @@ int __init_memblock memblock_clear_nomap(phys_addr_t base, phys_addr_t size) /**

• memblock_reserved_mark_noinit - Mark a reserved memory region with flag

• • MEMBLOCK_RSRV_NOINIT which results in the struct pages not being initialized
• • for this region.

• • MEMBLOCK_RSRV_NOINIT which allows for the "struct pages" corresponding
• • to this region not getting initialized, because the caller will take
• • care of it.
  • @base: the base phys addr of the region
  • @size: the size of the region

• • struct pages will not be initialized for reserved memory regions marked with
• • %MEMBLOCK_RSRV_NOINIT.

• • "struct pages" will not be initialized for reserved memory regions marked
• • with %MEMBLOCK_RSRV_NOINIT if this function is called before initialization
• • code runs. Without CONFIG_DEFERRED_STRUCT_PAGE_INIT, it is more likely
• • that this function is not effective.
  • Return: 0 on success, -errno on failure.
  */

I have a different version :) diff --git a/include/linux/memblock.h b/include/linux/memblock.h index b96746376e17..d20d091c6343 100644 --- a/include/linux/memblock.h +++ b/include/linux/memblock.h @@ -40,8 +40,9 @@ extern unsigned long long max_possible_pfn;

• via a driver, and never indicated in the firmware-provided memory map as
• system RAM. This corresponds to IORESOURCE_SYSRAM_DRIVER_MANAGED in the
• kernel resource tree.

• • @MEMBLOCK_RSRV_NOINIT: memory region for which struct pages are
• • not initialized (only for reserved regions).

• • @MEMBLOCK_RSRV_NOINIT: memory region for which struct pages don't have
• • PG_Reserved set and are completely not initialized when
• • %CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled (only for reserved regions).
  • @MEMBLOCK_RSRV_KERN: memory region that is reserved for kernel use,
  • either explictitly with memblock_reserve_kern() or via memblock
  • allocation APIs. All memblock allocations set this flag.

diff --git a/mm/memblock.c b/mm/memblock.c index 154f1d73b61f..02de5ffb085b 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -1091,13 +1091,15 @@ int __init_memblock memblock_clear_nomap(phys_addr_t base, phys_addr_t size) /**

• memblock_reserved_mark_noinit - Mark a reserved memory region with flag

• • MEMBLOCK_RSRV_NOINIT which results in the struct pages not being initialized
• • for this region.

• • MEMBLOCK_RSRV_NOINIT
• • @base: the base phys addr of the region
  • @size: the size of the region

• • struct pages will not be initialized for reserved memory regions marked with
• • %MEMBLOCK_RSRV_NOINIT.

• • The struct pages for the reserved regions marked %MEMBLOCK_RSRV_NOINIT will
• • not have %PG_Reserved flag set.
• • When %CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled, setting this flags also
• • completly bypasses the initialization of struct pages for this region.

s/completly/completely.

I don't quite understand the interaction with PG_Reserved and why anybody using this function should care.

So maybe you can rephrase in a way that is easier to digest, and rather focuses on what callers of this function are supposed to do vs. have the liberty of not doing?

On 25.08.25 18:58, Mike Rapoport wrote:

On Mon, Aug 25, 2025 at 06:23:48PM +0200, David Hildenbrand wrote:

...

I don't quite understand the interaction with PG_Reserved and why anybody using this function should care.

So maybe you can rephrase in a way that is easier to digest, and rather focuses on what callers of this function are supposed to do vs. have the liberty of not doing?

How about diff --git a/include/linux/memblock.h b/include/linux/memblock.h index b96746376e17..fcda8481de9a 100644 --- a/include/linux/memblock.h +++ b/include/linux/memblock.h @@ -40,8 +40,9 @@ extern unsigned long long max_possible_pfn;

• via a driver, and never indicated in the firmware-provided memory map as
• system RAM. This corresponds to IORESOURCE_SYSRAM_DRIVER_MANAGED in the
• kernel resource tree.

• • @MEMBLOCK_RSRV_NOINIT: memory region for which struct pages are
• • not initialized (only for reserved regions).

• • @MEMBLOCK_RSRV_NOINIT: reserved memory region for which struct pages are not
• • fully initialized. Users of this flag are responsible to properly initialize
• • struct pages of this region
  • @MEMBLOCK_RSRV_KERN: memory region that is reserved for kernel use,
  • either explictitly with memblock_reserve_kern() or via memblock
  • allocation APIs. All memblock allocations set this flag.

diff --git a/mm/memblock.c b/mm/memblock.c index 154f1d73b61f..46b411fb3630 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -1091,13 +1091,20 @@ int __init_memblock memblock_clear_nomap(phys_addr_t base, phys_addr_t size) /**

• memblock_reserved_mark_noinit - Mark a reserved memory region with flag

• • MEMBLOCK_RSRV_NOINIT which results in the struct pages not being initialized
• • for this region.

• • MEMBLOCK_RSRV_NOINIT
• • @base: the base phys addr of the region
  • @size: the size of the region

• • struct pages will not be initialized for reserved memory regions marked with
• • %MEMBLOCK_RSRV_NOINIT.

• • The struct pages for the reserved regions marked %MEMBLOCK_RSRV_NOINIT will
• • not be fully initialized to allow the caller optimize their initialization.
• • When %CONFIG_DEFERRED_STRUCT_PAGE_INIT is enabled, setting this flag
• • completely bypasses the initialization of struct pages for such region.
• • When %CONFIG_DEFERRED_STRUCT_PAGE_INIT is disabled, struct pages in this
• • region will be initialized with default values but won't be marked as
• • reserved.

Sounds good.

I am surprised regarding "reserved", but I guess that's because we don't end up calling "reserve_bootmem_region()" on these regions in memmap_init_reserved_pages().

On 21 Aug 2025, at 16:06, David Hildenbrand wrote:

Let's limit the maximum folio size in problematic kernel config where the memmap is allocated per memory section (SPARSEMEM without SPARSEMEM_VMEMMAP) to a single memory section.

Currently, only a single architectures supports ARCH_HAS_GIGANTIC_PAGE but not SPARSEMEM_VMEMMAP: sh.

Fortunately, the biggest hugetlb size sh supports is 64 MiB (HUGETLB_PAGE_SIZE_64MB) and the section size is at least 64 MiB (SECTION_SIZE_BITS == 26), so their use case is not degraded.

As folios and memory sections are naturally aligned to their order-2 size in memory, consequently a single folio can no longer span multiple memory sections on these problematic kernel configs.

nth_page() is no longer required when operating within a single compound page / folio.

Signed-off-by: David Hildenbrand david@redhat.com

include/linux/mm.h | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h index 77737cbf2216a..48a985e17ef4e 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2053,11 +2053,25 @@ static inline long folio_nr_pages(const struct folio *folio) return folio_large_nr_pages(folio); }

-/* Only hugetlbfs can allocate folios larger than MAX_ORDER */ -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE -#define MAX_FOLIO_ORDER PUD_ORDER -#else +#if !defined(CONFIG_ARCH_HAS_GIGANTIC_PAGE) +/*

• • We don't expect any folios that exceed buddy sizes (and consequently
• • memory sections).
• */

#define MAX_FOLIO_ORDER MAX_PAGE_ORDER +#elif defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP) +/*

• • Only pages within a single memory section are guaranteed to be
• • contiguous. By limiting folios to a single memory section, all folio
• • pages are guaranteed to be contiguous.
• */

+#define MAX_FOLIO_ORDER PFN_SECTION_SHIFT +#else +/*

• • There is no real limit on the folio size. We limit them to the maximum we
• • currently expect.

The comment about hugetlbfs is helpful here, since the other folios are still limited by buddy allocator’s MAX_ORDER.

• */

+#define MAX_FOLIO_ORDER PUD_ORDER #endif

#define MAX_FOLIO_NR_PAGES (1UL << MAX_FOLIO_ORDER)

2.50.1

Otherwise, Reviewed-by: Zi Yan ziy@nvidia.com

Best Regards, Yan, Zi

On 21 Aug 2025, at 16:49, David Hildenbrand wrote:

On 21.08.25 22:46, Zi Yan wrote:

...

On 21 Aug 2025, at 16:06, David Hildenbrand wrote:

...

Let's limit the maximum folio size in problematic kernel config where the memmap is allocated per memory section (SPARSEMEM without SPARSEMEM_VMEMMAP) to a single memory section.

Currently, only a single architectures supports ARCH_HAS_GIGANTIC_PAGE but not SPARSEMEM_VMEMMAP: sh.

Fortunately, the biggest hugetlb size sh supports is 64 MiB (HUGETLB_PAGE_SIZE_64MB) and the section size is at least 64 MiB (SECTION_SIZE_BITS == 26), so their use case is not degraded.

As folios and memory sections are naturally aligned to their order-2 size in memory, consequently a single folio can no longer span multiple memory sections on these problematic kernel configs.

nth_page() is no longer required when operating within a single compound page / folio.

Signed-off-by: David Hildenbrand david@redhat.com

include/linux/mm.h | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h index 77737cbf2216a..48a985e17ef4e 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2053,11 +2053,25 @@ static inline long folio_nr_pages(const struct folio *folio) return folio_large_nr_pages(folio); }

-/* Only hugetlbfs can allocate folios larger than MAX_ORDER */ -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE -#define MAX_FOLIO_ORDER PUD_ORDER -#else +#if !defined(CONFIG_ARCH_HAS_GIGANTIC_PAGE) +/*

• • We don't expect any folios that exceed buddy sizes (and consequently
• • memory sections).
• */ #define MAX_FOLIO_ORDER MAX_PAGE_ORDER

+#elif defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP) +/*

• • Only pages within a single memory section are guaranteed to be
• • contiguous. By limiting folios to a single memory section, all folio
• • pages are guaranteed to be contiguous.
• */

+#define MAX_FOLIO_ORDER PFN_SECTION_SHIFT +#else +/*

• • There is no real limit on the folio size. We limit them to the maximum we
• • currently expect.

The comment about hugetlbfs is helpful here, since the other folios are still limited by buddy allocator’s MAX_ORDER.

Yeah, but the old comment was wrong (there is DAX).

I can add here "currently expect (e.g., hugetlfs, dax)."

Sounds good.

Best Regards, Yan, Zi

On 21.08.25 22:55, Zi Yan wrote:

On 21 Aug 2025, at 16:06, David Hildenbrand wrote:

...

Now that a single folio/compound page can no longer span memory sections in problematic kernel configurations, we can stop using nth_page().

While at it, turn both macros into static inline functions and add kernel doc for folio_page_idx().

Signed-off-by: David Hildenbrand david@redhat.com

include/linux/mm.h | 16 ++++++++++++++-- include/linux/page-flags.h | 5 ++++- 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h index 48a985e17ef4e..ef360b72cb05c 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -210,10 +210,8 @@ extern unsigned long sysctl_admin_reserve_kbytes;

#if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP) #define nth_page(page,n) pfn_to_page(page_to_pfn((page)) + (n)) -#define folio_page_idx(folio, p) (page_to_pfn(p) - folio_pfn(folio)) #else #define nth_page(page,n) ((page) + (n)) -#define folio_page_idx(folio, p) ((p) - &(folio)->page) #endif

/* to align the pointer to the (next) page boundary */ @@ -225,6 +223,20 @@ extern unsigned long sysctl_admin_reserve_kbytes; /* test whether an address (unsigned long or pointer) is aligned to PAGE_SIZE */ #define PAGE_ALIGNED(addr) IS_ALIGNED((unsigned long)(addr), PAGE_SIZE)

+/**

• • folio_page_idx - Return the number of a page in a folio.
• • @folio: The folio.
• • @page: The folio page.
• • This function expects that the page is actually part of the folio.
• • The returned number is relative to the start of the folio.
• */

+static inline unsigned long folio_page_idx(const struct folio *folio,

• const struct page *page)
  

+{

• return page - &folio->page;

+}

• static inline struct folio *lru_to_folio(struct list_head *head) { return list_entry((head)->prev, struct folio, lru);

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h index d53a86e68c89b..080ad10c0defc 100644 --- a/include/linux/page-flags.h +++ b/include/linux/page-flags.h @@ -316,7 +316,10 @@ static __always_inline unsigned long _compound_head(const struct page *page)

• check that the page number lies within @folio; the caller is presumed
• to have a reference to the page.

*/ -#define folio_page(folio, n) nth_page(&(folio)->page, n) +static inline struct page *folio_page(struct folio *folio, unsigned long nr) +{

• return &folio->page + nr;

+}

Maybe s/nr/n/ or s/nr/nth/, since it returns the nth page within a folio.

Yeah, it's even called "n" in the kernel docs ...

Since you have added kernel doc for folio_page_idx(), it does not hurt to have something similar for folio_page(). :)

... which we already have! (see above the macro) :)

Thanks!

On 8/21/25 21:06, David Hildenbrand wrote:

We always provide a single dst page, it's unclear why the io_copy_cache complexity is required.

Because it'll need to be pulled outside the loop to reuse the page for multiple copies, i.e. packing multiple fragments of the same skb into it. Not finished, and currently it's wasting memory.

Why not do as below? Pages there never cross boundaries of their folios.

Do you want it to be taken into the io_uring tree?

diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c index e5ff49f3425e..18c12f4b56b6 100644 --- a/io_uring/zcrx.c +++ b/io_uring/zcrx.c @@ -975,9 +975,9 @@ static ssize_t io_copy_page(struct io_copy_cache *cc, struct page *src_page,

if (folio_test_partial_kmap(page_folio(dst_page)) || folio_test_partial_kmap(page_folio(src_page))) { - dst_page = nth_page(dst_page, dst_offset / PAGE_SIZE); + dst_page += dst_offset / PAGE_SIZE; dst_offset = offset_in_page(dst_offset); - src_page = nth_page(src_page, src_offset / PAGE_SIZE); + src_page += src_offset / PAGE_SIZE; src_offset = offset_in_page(src_offset); n = min(PAGE_SIZE - src_offset, PAGE_SIZE - dst_offset); n = min(n, len);

On 8/22/25 14:59, David Hildenbrand wrote:

On 22.08.25 13:32, Pavel Begunkov wrote:

...

On 8/21/25 21:06, David Hildenbrand wrote:

...

We always provide a single dst page, it's unclear why the io_copy_cache complexity is required.

Because it'll need to be pulled outside the loop to reuse the page for multiple copies, i.e. packing multiple fragments of the same skb into it. Not finished, and currently it's wasting memory.

Okay, so what you're saying is that there will be follow-up work that will actually make this structure useful.

Exactly

...

Why not do as below? Pages there never cross boundaries of their folios. > Do you want it to be taken into the io_uring tree?

This should better all go through the MM tree where we actually guarantee contiguous pages within a folio. (see the cover letter)

Makes sense. No objection, hopefully it won't cause too many conflicts.

...

diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c index e5ff49f3425e..18c12f4b56b6 100644 --- a/io_uring/zcrx.c +++ b/io_uring/zcrx.c @@ -975,9 +975,9 @@ static ssize_t io_copy_page(struct io_copy_cache *cc, struct page *src_page,            if (folio_test_partial_kmap(page_folio(dst_page)) ||                folio_test_partial_kmap(page_folio(src_page))) { -            dst_page = nth_page(dst_page, dst_offset / PAGE_SIZE); +            dst_page += dst_offset / PAGE_SIZE;                dst_offset = offset_in_page(dst_offset); -            src_page = nth_page(src_page, src_offset / PAGE_SIZE); +            src_page += src_offset / PAGE_SIZE;

Yeah, I can do that in the next version given that you have plans on extending that code soon.

If we go with this version:

Reviewed-by: Pavel Begunkov asml.silence@gmail.com

Hi David,

On Thu, Aug 21, 2025 at 10:06:47PM +0200, David Hildenbrand wrote:

Let's disallow handing out PFN ranges with non-contiguous pages, so we can remove the nth-page usage in __cma_alloc(), and so any callers don't have to worry about that either when wanting to blindly iterate pages.

This is really only a problem in configs with SPARSEMEM but without SPARSEMEM_VMEMMAP, and only when we would cross memory sections in some cases.

Will this cause harm? Probably not, because it's mostly 32bit that does not support SPARSEMEM_VMEMMAP. If this ever becomes a problem we could look into allocating the memmap for the memory sections spanned by a single CMA region in one go from memblock.

Signed-off-by: David Hildenbrand david@redhat.com

include/linux/mm.h | 6 ++++++ mm/cma.c | 36 +++++++++++++++++++++++------------- mm/util.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h index ef360b72cb05c..f59ad1f9fc792 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -209,9 +209,15 @@ extern unsigned long sysctl_user_reserve_kbytes; extern unsigned long sysctl_admin_reserve_kbytes; #if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP) +bool page_range_contiguous(const struct page *page, unsigned long nr_pages); #define nth_page(page,n) pfn_to_page(page_to_pfn((page)) + (n)) #else #define nth_page(page,n) ((page) + (n)) +static inline bool page_range_contiguous(const struct page *page,

• unsigned long nr_pages)
  

+{

• return true;

+} #endif /* to align the pointer to the (next) page boundary */ diff --git a/mm/cma.c b/mm/cma.c index 2ffa4befb99ab..1119fa2830008 100644 --- a/mm/cma.c +++ b/mm/cma.c @@ -780,10 +780,8 @@ static int cma_range_alloc(struct cma *cma, struct cma_memrange *cmr, unsigned long count, unsigned int align, struct page **pagep, gfp_t gfp) {

• unsigned long mask, offset;
• unsigned long pfn = -1;
• unsigned long start = 0; unsigned long bitmap_maxno, bitmap_no, bitmap_count;

• unsigned long start, pfn, mask, offset; int ret = -EBUSY; struct page *page = NULL;

@@ -795,7 +793,7 @@ static int cma_range_alloc(struct cma *cma, struct cma_memrange *cmr, if (bitmap_count > bitmap_maxno) goto out;

• for (;;) {

• for (start = 0; ; start = bitmap_no + mask + 1) { spin_lock_irq(&cma->lock); /*
  • If the request is larger than the available number

@@ -812,6 +810,22 @@ static int cma_range_alloc(struct cma *cma, struct cma_memrange *cmr, spin_unlock_irq(&cma->lock); break; }

• pfn = cmr->base_pfn + (bitmap_no << cma->order_per_bit);
  

• page = pfn_to_page(pfn);
  

• /*
  

•  * Do not hand out page ranges that are not contiguous, so
  

•  * callers can just iterate the pages without having to worry
  

•  * about these corner cases.
  

•  */
  

• if (!page_range_contiguous(page, count)) {
  

• 	spin_unlock_irq(&cma->lock);
  

• 	pr_warn_ratelimited("%s: %s: skipping incompatible area [0x%lx-0x%lx]",
  

• 			    __func__, cma->name, pfn, pfn + count - 1);
  

• 	continue;
  

• }
  

• bitmap_set(cmr->bitmap, bitmap_no, bitmap_count); cma->available_count -= count; /*

@@ -821,29 +835,25 @@ static int cma_range_alloc(struct cma *cma, struct cma_memrange *cmr, */ spin_unlock_irq(&cma->lock);

• pfn = cmr->base_pfn + (bitmap_no << cma->order_per_bit);
  

  mutex_lock(&cma->alloc_mutex); ret = alloc_contig_range(pfn, pfn + count, ACR_FLAGS_CMA, gfp); mutex_unlock(&cma->alloc_mutex);

• if (ret == 0) {
  

• 	page = pfn_to_page(pfn);
  

• if (!ret)
  break;
  

• }
  

cma_clear_bitmap(cma, cmr, pfn, count); if (ret != -EBUSY) break; pr_debug("%s(): memory range at pfn 0x%lx %p is busy, retrying\n",

• 	 __func__, pfn, pfn_to_page(pfn));
  

• 	 __func__, pfn, page);
  

trace_cma_alloc_busy_retry(cma->name, pfn, pfn_to_page(pfn),

Nitpick: I think you already have the page here.

			   count, align);

• /* try again with a bit different memory target */
  

• start = bitmap_no + mask + 1;
  

  }

out:

• *pagep = page;

• if (!ret)

• *pagep = page;
  

  return ret;

} @@ -882,7 +892,7 @@ static struct page *__cma_alloc(struct cma *cma, unsigned long count, */ if (page) { for (i = 0; i < count; i++)

• 	page_kasan_tag_reset(nth_page(page, i));
  

• 	page_kasan_tag_reset(page + i);
  

Had a look at it, not very familiar with CMA, but the changes look equivalent to what was before. Not sure that's worth a Reviewed-by tag, but here it in case you want to add it:

Reviewed-by: Alexandru Elisei alexandru.elisei@arm.com

Just so I can better understand the problem being fixed, I guess you can have two consecutive pfns with non-consecutive associated struct page if you have two adjacent memory sections spanning the same physical memory region, is that correct?

Thanks, Alex

} if (ret && !(gfp & __GFP_NOWARN)) { diff --git a/mm/util.c b/mm/util.c index d235b74f7aff7..0bf349b19b652 100644 --- a/mm/util.c +++ b/mm/util.c @@ -1280,4 +1280,37 @@ unsigned int folio_pte_batch(struct folio *folio, pte_t *ptep, pte_t pte, { return folio_pte_batch_flags(folio, NULL, ptep, &pte, max_nr, 0); }

+#if defined(CONFIG_SPARSEMEM) && !defined(CONFIG_SPARSEMEM_VMEMMAP) +/**

• • page_range_contiguous - test whether the page range is contiguous
• • @page: the start of the page range.
• • @nr_pages: the number of pages in the range.
• • Test whether the page range is contiguous, such that they can be iterated
• • naively, corresponding to iterating a contiguous PFN range.
• • This function should primarily only be used for debug checks, or when
• • working with page ranges that are not naturally contiguous (e.g., pages
• • within a folio are).
• • Returns true if contiguous, otherwise false.
• */

+bool page_range_contiguous(const struct page *page, unsigned long nr_pages) +{

• const unsigned long start_pfn = page_to_pfn(page);
• const unsigned long end_pfn = start_pfn + nr_pages;
• unsigned long pfn;
• /*

• * The memmap is allocated per memory section. We need to check
  

• * each involved memory section once.
  

• */
  

• for (pfn = ALIGN(start_pfn, PAGES_PER_SECTION);

•     pfn < end_pfn; pfn += PAGES_PER_SECTION)
  

• if (unlikely(page + (pfn - start_pfn) != pfn_to_page(pfn)))
  

• 	return false;
  

• return true;

+} +#endif

#endif /* CONFIG_MMU */

2.50.1

On 21.08.2025 22:06, David Hildenbrand wrote:

dma_common_contiguous_remap() is used to remap an "allocated contiguous region". Within a single allocation, there is no need to use nth_page() anymore.

Neither the buddy, nor hugetlb, nor CMA will hand out problematic page ranges.

Cc: Marek Szyprowski m.szyprowski@samsung.com Cc: Robin Murphy robin.murphy@arm.com Signed-off-by: David Hildenbrand david@redhat.com

Acked-by: Marek Szyprowski m.szyprowski@samsung.com

---

kernel/dma/remap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/dma/remap.c b/kernel/dma/remap.c index 9e2afad1c6152..b7c1c0c92d0c8 100644 --- a/kernel/dma/remap.c +++ b/kernel/dma/remap.c @@ -49,7 +49,7 @@ void *dma_common_contiguous_remap(struct page *page, size_t size, if (!pages) return NULL; for (i = 0; i < count; i++)

• pages[i] = nth_page(page, i);
  

• pages[i] = page++;
  

  vaddr = vmap(pages, count, VM_DMA_COHERENT, prot); kvfree(pages);

Best regards

On 21.08.2025 22:06, David Hildenbrand wrote:

The expectation is that there is currently no user that would pass in non-contigous page ranges: no allocator, not even VMA, will hand these out.

The only problematic part would be if someone would provide a range obtained directly from memblock, or manually merge problematic ranges. If we find such cases, we should fix them to create separate SG entries.

Let's check in sg_set_page() that this is really the case. No need to check in sg_set_folio(), as pages in a folio are guaranteed to be contiguous.

We can now drop the nth_page() usage in sg_page_iter_page().

Signed-off-by: David Hildenbrand david@redhat.com

Acked-by: Marek Szyprowski m.szyprowski@samsung.com

---

include/linux/scatterlist.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 6f8a4965f9b98..8196949dfc82c 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -6,6 +6,7 @@ #include <linux/types.h> #include <linux/bug.h> #include <linux/mm.h> +#include <linux/mm_inline.h> #include <asm/io.h> struct scatterlist { @@ -158,6 +159,7 @@ static inline void sg_assign_page(struct scatterlist *sg, struct page *page) static inline void sg_set_page(struct scatterlist *sg, struct page *page, unsigned int len, unsigned int offset) {

• VM_WARN_ON_ONCE(!page_range_contiguous(page, ALIGN(len + offset, PAGE_SIZE) / PAGE_SIZE)); sg_assign_page(sg, page); sg->offset = offset; sg->length = len;

@@ -600,7 +602,7 @@ void __sg_page_iter_start(struct sg_page_iter *piter, */ static inline struct page *sg_page_iter_page(struct sg_page_iter *piter) {

• return nth_page(sg_page(piter->sg), piter->sg_pgoffset);

• return sg_page(piter->sg) + piter->sg_pgoffset; }

/**

Best regards

On 8/22/25 05:06, David Hildenbrand wrote:

It's no longer required to use nth_page() when iterating pages within a single SG entry, so let's drop the nth_page() usage.

Cc: Damien Le Moal dlemoal@kernel.org Cc: Niklas Cassel cassel@kernel.org Signed-off-by: David Hildenbrand david@redhat.com

---

drivers/ata/libata-sff.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c index 7fc407255eb46..9f5d0f9f6d686 100644 --- a/drivers/ata/libata-sff.c +++ b/drivers/ata/libata-sff.c @@ -614,7 +614,7 @@ static void ata_pio_sector(struct ata_queued_cmd *qc) offset = qc->cursg->offset + qc->cursg_ofs; /* get the current page and offset */

• page = nth_page(page, (offset >> PAGE_SHIFT));

• page += offset / PAGE_SHIFT;

Shouldn't this be "offset >> PAGE_SHIFT" ?

offset %= PAGE_SIZE; /* don't overrun current sg */ @@ -631,7 +631,7 @@ static void ata_pio_sector(struct ata_queued_cmd *qc) unsigned int split_len = PAGE_SIZE - offset; ata_pio_xfer(qc, page, offset, split_len);

• ata_pio_xfer(qc, nth_page(page, 1), 0, count - split_len);
  

• ata_pio_xfer(qc, page + 1, 0, count - split_len);
  

  } else { ata_pio_xfer(qc, page, offset, count); }

@@ -751,7 +751,7 @@ static int __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes) offset = sg->offset + qc->cursg_ofs; /* get the current page and offset */

• page = nth_page(page, (offset >> PAGE_SHIFT));

• page += offset / PAGE_SIZE;

Same here, though this seems correct too.

offset %= PAGE_SIZE; /* don't overrun current sg */