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

[Jan Kara]

On Fri 29-05-26 15:11:54, Usama Arif wrote:
> On 29/05/2026 14:40, Pedro Falcato wrote:
> > On Fri, May 29, 2026 at 01:19:03PM +0100, Usama Arif wrote:
> >>
> >> 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?
> > 
> 
> Yes, so if we replace it with SZ_32M, it sounds correct. I just think
> the 32M size is too large. But as you pointed out, even 2M can be too large...

So AFAIU the practical discussion is about two options:

1) limiting at 2MB with a slighly more complicated logic to keep mapping at
PMD order for 16k pagesize on ARM but use 2MB pages for 64k pagesize on ARM

or

2) limit at 32MB with simple logic which results in larger (32MB) folios
with 16k and 64k pagesize on ARM and thus larger memory overhead.

I'd like to maybe offer option 3): limit at 2MB with simple logic. This
will reduce folio size on 16k pagesize ARM compared to 1) but do we really
care? I.e., is there big enough practical performance impact with conpte
and other tricks ARM is playing?

> > 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!
> 
> Yes completely agree on this as well.
> 
> So personally I do have a preference of keeping the cap at 2M atleast initially
> while we currently try and solve the issues we see with 2M alone. As we are already
> seeing reports of thrashing and compaction with just 2M, I dont think the logic
> in this patch with just an if else is that complicated.
> 
> Matthew, Jan, do you have any thoughts or strong preferences on cap size?

Frankly, no strong opinion. I'd think 3) is worth trying for its simplicity
and seeing whether somebody complains, otherwise I can live with both 1)
and 2).

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR