Re: [PATCH] mm: thp: Deny THP for guest_memfd and secretmem in file_thp_enabled()

[David Hildenbrand (Arm)]

> > BUT, something just occurred to me.
> >
> > We added the mc-handling in
> >
> > commit 98c76c9f1ef7599b39bfd4bd99b8a760d4a8cd3b
> > Author: Jiaqi Yan <jiaqiyan@xxxxxxxxxx>
> > Date:   Wed Mar 29 08:11:19 2023 -0700
> >
> >       mm/khugepaged: recover from poisoned anonymous memory
> >
> > ..
> >
> > So I assume kernels before that would crash when collapsing?
> >
> > Looking at 5.15.199, it does not contain 98c76c9f1e [1].
> >
> > So I suspect we need a fix+stable backport.
> >
> > Who volunteers to try a secretmem reproducer on a stable kernel? :)
>
> I could give this a shot. 5.15.199 doesn't have AS_INACCESSIBLE. Should
> we backport AS_INACCESSIBLE there or could the fix for 5.15.199 just be
> special-casing secretmem like you suggested below?

Yes. If there is no guest_memfd we wouldn't need it.

> > The following is a bit nasty as well but should do the trick until we rip
> > out the CONFIG_READ_ONLY_THP_FOR_FS stuff.
> >
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 03886d4ccecc..4ac1cb36b861 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -40,6 +40,7 @@
> >    #include <linux/pgalloc.h>
> >    #include <linux/pgalloc_tag.h>
> >    #include <linux/pagewalk.h>
> > +#include <linux/secretmem.h>
> >
> >    #include <asm/tlb.h>
> >    #include "internal.h"
> > @@ -94,6 +95,10 @@ static inline bool file_thp_enabled(struct vm_area_struct *vma)
> >
> >           inode = file_inode(vma->vm_file);
> >
> > +       if (mapping_inaccessible(inode->i_mapping) ||
> > +           secretmem_mapping(inode->i_mapping))
> > +               return false;
> > +
>
> Regarding the degradation of filesystems that don't support large folios
> yet: Do you mean having the collapse function respect AS_FOLIO_ORDER_MAX
> would disable collapsing for filesystems that actually want pages to be
> collapsed, but don't update max folio order and hence appear to not
> support large folios yet?
>
> What about a check like this instead
>
> 	if (!mapping_large_folio_support())
>          	return false;

That would essentially disable CONFIG_READ_ONLY_THP_FOR_FS (support for 
THP before filesystems started supporting large folios officially), no?

--
Cheers,

David