Re: [PATCH] mm/huge_memory: Avoid PMD-size page cache if needed

[David Hildenbrand <david@redhat.com>]

On 11.07.24 23:03, David Hildenbrand wrote:
> On 11.07.24 22:46, Matthew Wilcox wrote:
>> On Thu, Jul 11, 2024 at 08:48:40PM +1000, Gavin Shan wrote:
>>> +++ b/mm/huge_memory.c
>>> @@ -136,7 +136,8 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
>>>    
>>>    		while (orders) {
>>>    			addr = vma->vm_end - (PAGE_SIZE << order);
>>> -			if (thp_vma_suitable_order(vma, addr, order))
>>> +			if (!(vma->vm_file && order > MAX_PAGECACHE_ORDER) &&
>>> +			    thp_vma_suitable_order(vma, addr, order))
>>>    				break;
>>
>> Why does 'orders' even contain potential orders that are larger than
>> MAX_PAGECACHE_ORDER?
>>
>> We do this at the top:
>>
>>           orders &= vma_is_anonymous(vma) ?
>>                           THP_ORDERS_ALL_ANON : THP_ORDERS_ALL_FILE;
>>
>> include/linux/huge_mm.h:#define THP_ORDERS_ALL_FILE     (BIT(PMD_ORDER) | BIT(PUD_ORDER))
>>
>> ... and that seems very wrong.  We support all kinds of orders for
>> files, not just PMD order.  We don't support PUD order at all.
>>
>> What the hell is going on here?
> 
> yes, that's just absolutely confusing. I mentioned it to Ryan lately
> that we should clean that up (I wanted to look into that, but am happy
> if someone else can help).
> 
> There should likely be different defines for
> 
> DAX (PMD|PUD)
> 
> SHMEM (PMD) -- but soon more. Not sure if we want separate ANON_SHMEM
> for the time being. Hm. But shmem is already handles separately, so
> maybe we can just ignore shmem here.

Correction: of course <= MAX_PAGECACHE_ORDER

But yeah, this needs cleanups

-- 
Cheers,

David / dhildenb