thp: enforcing constraints on file thps

thp: enforcing constraints on file thps

[Zach O'Keefe]

Hey All,

There are currently a number of paths where we can collapse file memory into
THPs:

1a) khugepaged - target of vma being processed
1b) khugepaged - other vma found mapping file, able to lock mmap_lock in
		 retract_page_tables()
1b) khugepaged - other vma found mapping file, deferred pte-mapped THP collapse,
		 processed in collapse_pte_mapped_thp()
2)  page fault finds hugepage in page cache + filemap_map_pages()

In terms of system-enforced THP constraints:

* vma flags + thps sysfs settings

  Checked in 1a. (1b now at least respects "never" THP mode after Yang Shi's
  cleanup series, but still doesn't respect "madvise" THP mode)

* MMF_DISABLE_THP

  Checked in 1a and 1b

I'm wondering if we should align these, and if so, in what direction? I would
argue that a process marked MMF_DISABLE_THP, or a vma marked VM_NOHUGEPAGE,
probably shouldn't be mapping at the pmd level, and that the appropriate checks
should be added in those paths.

Thanks,
Zach

Re: thp: enforcing constraints on file thps

[Yang Shi]

On Wed, Jun 29, 2022 at 7:01 AM Zach O'Keefe <zokeefe@google.com> wrote:
>
> Hey All,
>
> There are currently a number of paths where we can collapse file memory into
> THPs:
>
> 1a) khugepaged - target of vma being processed
> 1b) khugepaged - other vma found mapping file, able to lock mmap_lock in
>                  retract_page_tables()
> 1b) khugepaged - other vma found mapping file, deferred pte-mapped THP collapse,
>                  processed in collapse_pte_mapped_thp()
> 2)  page fault finds hugepage in page cache + filemap_map_pages()
>
> In terms of system-enforced THP constraints:
>
> * vma flags + thps sysfs settings
>
>   Checked in 1a. (1b now at least respects "never" THP mode after Yang Shi's
>   cleanup series, but still doesn't respect "madvise" THP mode)
>
> * MMF_DISABLE_THP
>
>   Checked in 1a and 1b
>
> I'm wondering if we should align these, and if so, in what direction? I would
> argue that a process marked MMF_DISABLE_THP, or a vma marked VM_NOHUGEPAGE,
> probably shouldn't be mapping at the pmd level, and that the appropriate checks
> should be added in those paths.

That depends on how we explain the semantics of MMF_DISABLE_THP and
VM_NOHUGEPAGE IMHO. They definitely prevent from allocating/collapsing
new THPs for that process or vma, but shall they also prevent from
mapping existing THPs to PMD? Maybe not.

>
> Thanks,
> Zach

Re: thp: enforcing constraints on file thps

[Zach O'Keefe]

On Wed, Jun 29, 2022 at 10:26 AM Yang Shi <shy828301@gmail.com> wrote:
>
> On Wed, Jun 29, 2022 at 7:01 AM Zach O'Keefe <zokeefe@google.com> wrote:
> >
> > Hey All,
> >
> > There are currently a number of paths where we can collapse file memory into
> > THPs:
> >
> > 1a) khugepaged - target of vma being processed
> > 1b) khugepaged - other vma found mapping file, able to lock mmap_lock in
> >                  retract_page_tables()
> > 1b) khugepaged - other vma found mapping file, deferred pte-mapped THP collapse,
> >                  processed in collapse_pte_mapped_thp()
> > 2)  page fault finds hugepage in page cache + filemap_map_pages()
> >
> > In terms of system-enforced THP constraints:
> >
> > * vma flags + thps sysfs settings
> >
> >   Checked in 1a. (1b now at least respects "never" THP mode after Yang Shi's
> >   cleanup series, but still doesn't respect "madvise" THP mode)
> >
> > * MMF_DISABLE_THP
> >
> >   Checked in 1a and 1b
> >
> > I'm wondering if we should align these, and if so, in what direction? I would
> > argue that a process marked MMF_DISABLE_THP, or a vma marked VM_NOHUGEPAGE,
> > probably shouldn't be mapping at the pmd level, and that the appropriate checks
> > should be added in those paths.
>
> That depends on how we explain the semantics of MMF_DISABLE_THP and
> VM_NOHUGEPAGE IMHO. They definitely prevent from allocating/collapsing
> new THPs for that process or vma, but shall they also prevent from
> mapping existing THPs to PMD? Maybe not.

Hey Yang, thanks for your thoughts.

IIUC, you're basically saying things are working as intended? I see
the argument that, if the THP is already assembled, we might as well
just map it at the PMD level..

But as you mention, it weakens the API contract of MMF_DISABLE_THP and
sysfs THP settings.

I don't have any real world example where this is an issue, and am
tempted to kick this down the road until someone has a good reason to
change it - but at the same time - the exasperation incurred by all
end users isn't guaranteed to reach linux-mm.

Thanks,
Zach

> >
> > Thanks,
> > Zach