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

[Usama Arif]

On 30/05/2026 16:16, Jan Kara wrote:
> 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?
> 

I think the logic isn't that complicated for 1, but I am happy with option 3.

>>> 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).


Thanks! Yes, let me send this. 

Andrew I will send this as a new and hopefully last revision as I am not sure
if you would like another fixup on top of this series! Thanks!


> 
> 								Honza