linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Zach O'Keefe" <zokeefe@google.com>
To: Yang Shi <shy828301@gmail.com>
Cc: Song Liu <songliubraving@fb.com>,
	Matthew Wilcox <willy@infradead.org>,
	 Miaohe Lin <linmiaohe@huawei.com>, Linux MM <linux-mm@kvack.org>
Subject: Re: thp: enforcing constraints on file thps
Date: Wed, 29 Jun 2022 13:43:01 -0700	[thread overview]
Message-ID: <CAAa6QmSe46L_H7t8pX8KLgW9-oDpmAb-ECO67c9b_iLRSMT03g@mail.gmail.com> (raw)
In-Reply-To: <CAHbLzkrDL4Rcs9jYBd6rD=8oXF+81YfaCNb8VqMBqJi-XJBGpA@mail.gmail.com>

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


      reply	other threads:[~2022-06-29 20:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-29 14:01 thp: enforcing constraints on file thps Zach O'Keefe
2022-06-29 17:25 ` Yang Shi
2022-06-29 20:43   ` Zach O'Keefe [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAAa6QmSe46L_H7t8pX8KLgW9-oDpmAb-ECO67c9b_iLRSMT03g@mail.gmail.com \
    --to=zokeefe@google.com \
    --cc=linmiaohe@huawei.com \
    --cc=linux-mm@kvack.org \
    --cc=shy828301@gmail.com \
    --cc=songliubraving@fb.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).