Re: [PATCH v6 2/2] mm: use mapping_max_folio_order() for force_thp_readahead order

[Pedro Falcato <pfalcato@suse.de>]

On Fri, May 29, 2026 at 01:19:03PM +0100, Usama Arif wrote:
> 
> 
> On 29/05/2026 11:01, Pedro Falcato wrote:
> > On Thu, May 28, 2026 at 09:55:20AM -0700, Usama Arif wrote:
> >> The force_thp_readahead path in do_sync_mmap_readahead() is gated on
> >> HPAGE_PMD_ORDER <= MAX_PAGECACHE_ORDER and always requests
> >> HPAGE_PMD_ORDER / HPAGE_PMD_NR. On configurations where HPAGE_PMD_ORDER
> >> exceeds MAX_PAGECACHE_ORDER, notably arm64 with a 64K base page size,
> >> VM_HUGEPAGE mappings cannot use this path and fall back to the non-forced
> >> mmap readahead path even when the mapping supports useful large folios.
> >>
> >> Keep the existing PMD-sized behavior when HPAGE_PMD_ORDER fits in the
> >> page cache. When it does not, enable forced readahead for mappings that
> >> support large folios and request an order capped by both
> >> mapping_max_folio_order(mapping) and 2MB.
> >>
> >> 2MB is chosen as the cap because it matches the PMD size on x86_64
> >> and on arm64 with 4K or 16K base pages, so the size/memory-pressure
> > 
> > 16K base page size's PMDs should be 32M, no? Am I misunderstanding what
> > you mean here?
> > 
> 
> Yes, should have just said 4K. Messed up when rewriting the commit
> message. Andrew Thanks for merging in mm-new, would it be possible
> to remove "or 16K" from the commit message. I can send a new revision
> as well if that is easier and preferred.
> 
> >> tradeoff for folios of that size is already well understood. On arm64
> >> with a 64K base page size, 2MB is also the contiguous-PTE (contpte)
> >> block size, so the resulting folios coalesce into a single TLB entry
> >> and reduce TLB pressure on the readahead path.
> >>
> >> The final allocation order may still be clamped by page_cache_ra_order()
> >> to the mapping and request geometry, but this gives VM_HUGEPAGE mappings
> >> on such configurations a large-folio readahead request instead of
> >> dropping back to base-page readahead.
> >>
> >> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> >> ---
> >>  mm/filemap.c | 27 +++++++++++++++++++--------
> >>  1 file changed, 19 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/mm/filemap.c b/mm/filemap.c
> >> index a16b33e0fc71..bfb891d9da1f 100644
> >> --- a/mm/filemap.c
> >> +++ b/mm/filemap.c
> >> @@ -3312,14 +3312,23 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
> >>  	struct file *fpin = NULL;
> >>  	vm_flags_t vm_flags = vmf->vma->vm_flags;
> >>  	bool force_thp_readahead = false;
> >> +	unsigned int thp_order = 0;
> >>  	unsigned short mmap_miss;
> >>  
> >>  	ractl._max_index = vmf->vma->vm_pgoff + vma_pages(vmf->vma) - 1;
> >>  
> >>  	/* Use the readahead code, even if readahead is disabled */
> >> -	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
> >> -	    (vm_flags & VM_HUGEPAGE) && HPAGE_PMD_ORDER <= MAX_PAGECACHE_ORDER)
> >> -		force_thp_readahead = true;
> >> +	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && (vm_flags & VM_HUGEPAGE)) {
> >> +		if (HPAGE_PMD_ORDER <= MAX_PAGECACHE_ORDER) {
> >> +			force_thp_readahead = true;
> >> +			thp_order = HPAGE_PMD_ORDER;
> >> +		} else if (mapping_large_folio_support(mapping)) {
> >> +			force_thp_readahead = true;
> >> +			thp_order = min_t(unsigned int,
> >> +					  mapping_max_folio_order(mapping),
> >> +					  get_order(SZ_2M));
> > 
> > This looks somewhat arbitrary to me (as does the old logic). It seems more
> > natural (correct?) to do this:
> > 
> > 	if (THP_ENABLED && (vm_flags & VM_HUGEPAGE)) {
> > 		if (mapping_large_folio_support(mapping)) {
> > 			force_thp_readahead = true;
> > 			/*
> > 			 * Cap THP readahead to either the max folio order the
> > 			 * mapping supports, or the max order the page cache
> > 			 * supports (useless to try more), or the hugepage PMD
> > 			 * order (CPU can't benefit from larger).
> > 			 */
> > 			thp_order = min3(mapping_max_folio_order(mapping),
> > 					MAX_PAGECACHE_ORDER, HPAGE_PMD_ORDER);
> > 		}
> > 	}
> > 
> 
> So this can result in extreme memory pressure. For example, on 64K base page size,
> with HPAGE_PMD_ORDER = 13 (512M) and MAX_PAGECACHE_ORDER = 11 (128M), this will evaluate
> THP order to 128M. I think Jan mentioned in previous versions that there are already
> reports of high memory pressure due to large file folios, and we are seeing this
> in our fleet as well with xfs.
> 
> I chose 2M cap as that is what most (not all) architectures which currently support this
> hugepage code flow are currently used to, and on archs + base page size combination which
> just evaluate this code to base page, for example 64K base page size, it will provide a
> performance uplift without causing excessive memory pressure.

For what it's worth, memory pressure is still a problem even on 2MB folios. But
yes, I understand.

For what it's worth, I don't think it's such a big issue in this case - you're
asking for THPs, and you're getting them - though maybe not efficiently because
these aren't at the PMD level. But this is also true for e.g x86 where an
an order-3 folio or order-7 folio are completely useless for the hardware, since
there is no mTHP to speak of (and, with that, you also lose PG_active and
PG_reference precision as you only keep one of each for each folio, amongst
other tradeoffs).

> 
> In pagemap.h, we have
> 
> #define PREFERRED_MAX_PAGECACHE_ORDER	HPAGE_PMD_ORDER
> #define MAX_PAGECACHE_ORDER	min(MAX_XAS_ORDER, PREFERRED_MAX_PAGECACHE_ORDER)
> 
> So MAX_PAGECACHE_ORDER <= HPAGE_PMD_ORDER is always true.
> 
> At filesystem/inode setup: mapping_set_folio_order_range() 
> 
> 	if (max > MAX_PAGECACHE_ORDER)
> 		max = MAX_PAGECACHE_ORDER;
> 
> so mapping_max_folio_order() <= MAX_PAGECACHE_ORDER is also always true.

Right, good point. (well, I suppose it will not be true in the future as
someone tries to harness PMD-level cont bits)

> 
> which means mapping_max_folio_order(mapping) <= MAX_PAGECACHE_ORDER <= HPAGE_PMD_ORDER is always
> true, and you dont need the min3(..) in your diff.
> 
> Now the question is if then why not just do:
> 
> 	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && (vm_flags & VM_HUGEPAGE)) {
> 		if (mapping_large_folio_support(mapping)) {
> 			force_thp_readahead = true;
> 			thp_order = min_t(unsigned int,
> 					  mapping_max_folio_order(mapping),
> 					  get_order(SZ_2M));
> 		}
> 	}
> 
> 
> This is because this will regress the 16K ARM case where we already got 32M
> folios. Someone might upgrade the kernel and start getting 2M folios now.

So maybe limit to 32MB? It's still arbitrary but at least you get simpler
logic. If the architecture does not support 32MiB folios, it will clamp
the maximum folio order to HPAGE_PMD_ORDER, and you get the same result.

Does this sound correct?

> 
> So IMHO the current code is correct, both in maintaining what the
> existing behaviour has been, but also trying to provide large folios to
> architectures that currently dont get them but in a controlled way.
> 
> Later on, when we think or evaluate that 2M was too small a cap, it can
> be raised. I think its very easy to raise a cap, but much more difficult
> to make the cap smaller, as we might get reports on kernel upgrades that
> someone was getting a certain size of folio but now its much smaller.

I don't think this is a valid argument. You can also have reports that now
folios are too large and they're regressing workloads because of thrashing/
compaction/possibly TLB thrashing. And indeed we have several reports of that
(https://lore.kernel.org/linux-fsdevel/20260403193535.9970-1-dipiets@amazon.it/
comes to mind, plus all of the anon THP regressions over the years, including
in $current_year).

Bottom line is that changing things will always affect someone :) Particularly
since the logic we have is not too careful at deciding what should or should
not be a THP (both in anon and file cases). And if (once?) we make it smarter,
it will surely also regress someone!

-- 
Pedro