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

[David Hildenbrand]

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