Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20230812210053.2325091-1-zokeefe@google.com> <PUZP153MB06358FF02518EF3B279F5DD4BE16A@PUZP153MB0635.APCP153.PROD.OUTLOOK.COM>
 <CAAa6QmSrwe2m4MjS9mGO+DeGNGSv=B2uZ72EAxnZk2jsDh39rQ@mail.gmail.com>
 <ZNp7JDaPhT3Se4de@casper.infradead.org> <CAAa6QmSN4NhaDL0DQsRd-F8HTnCCjq1ULRNk88LAA9gVbDXE4g@mail.gmail.com>
 <ZNrh6w9ICu4rMrhV@casper.infradead.org> <CAAa6QmTA8aADSYbpxXU8kne0KqyeY7fCw5_QYSj0T7bCtPKmfA@mail.gmail.com>
 <ZN4QFNZlx8mK9pQm@casper.infradead.org> <CAAa6QmTi99exY+NtNDZFi74cs7AB2xpaZ_kej3pSaZVdAtbrKA@mail.gmail.com>
 <ZN5um2yIXFcxiFjS@casper.infradead.org>
In-Reply-To: <ZN5um2yIXFcxiFjS@casper.infradead.org>
From:   "Zach O'Keefe" <zokeefe@google.com>
Date:   Thu, 17 Aug 2023 14:12:54 -0700
Message-ID: <CAAa6QmSGnXS3t-=i7RdBvqD9XXHhSg1Y9bZU4Dzg903UjZspUw@mail.gmail.com>
Subject: Re: [EXTERNAL] [PATCH] mm/thp: fix "mm: thp: kill __transhuge_page_enabled()"
To:     Matthew Wilcox <willy@infradead.org>
Cc:     Saurabh Singh Sengar <ssengar@microsoft.com>,
        Dan Williams <dan.j.williams@intel.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        Yang Shi <shy828301@gmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Aug 17, 2023 at 12:01=E2=80=AFPM Matthew Wilcox <willy@infradead.or=
g> wrote:
>
> On Thu, Aug 17, 2023 at 11:13:36AM -0700, Zach O'Keefe wrote:
> > > > IIUC then, there is a bug in smaps THPeligible code when
> > > > CONFIG_READ_ONLY_THP_FOR_FS is not set. Not obvious, but apparently
> > > > this config is (according to it's Kconfig desc) khugepaged-only, so=
 it
> > > > should be fine for it to be disabled, yet allow
> > > > do_sync_mmap_readahead() to install a pmd for file-backed memory.
> > > > hugepage_vma_check() will need to be patched to fix this.
> > >
> > > I guess so ...
> >
> > The easiest and most satisfying way to handle this -- and I think we
> > talked about this before -- is relaxing that complicated
> > file_thp_enabled() check when the file's mapping supports large
> > folios. I think that makes sense to me, though I don't know all the
> > details fs-side. Will we need any hook to give fs the chance to update
> > any internal state on collapse?
>
> If the filesystem has per-folio metadata, we need to give the filesystem
> the chance to set that up.  I've vaguaely been wondering about using the
> ->migrate_folio callback for it.  At the moment, I think it just refuses
> to work if the folio isn't order-0.

Ok, no free lunch here then. I'll give myself a reminder to come back
here then and dig a little deeper. Thanks Matthew

> > > > But I have a larger question for you: should we care about
> > > > /sys/kernel/mm/transparent_hugepage/enabled for file-fault? We
> > > > currently don't. Seems weird that we can transparently get a hugepa=
ge
> > > > when THP=3D"never". Also, if THP=3D"always", we might as well skip =
the
> > > > VM_HUGEPAGE check, and try the final pmd install (and save khugepag=
ed
> > > > the trouble of attempting it later).
> > >
> > > I deliberately ignored the humungous complexity of the THP options.
> > > They're overgrown and make my brain hurt. [..]
> >
> > Same
> >
> > > [..] Instead, large folios are
> > > adaptive; they observe the behaviour of the user program and choose b=
ased
> > > on history what to do.  This is far superior to having a sysadmin tel=
l
> > > us what to do!
> >
> > I had written a bunch on this, but I arrived to the conclusion that
> > (a) pmd-mapping here is ~ a free win, and (b) I'm not the best  person
> > to argue for these knobs, given MADV_COLLAPSE ignores them entirely :P
> >
> > ..But (sorry) what about MMF_DISABLE_THP?
>
> Yeah, we ignore that too.  My rationale is -- as you said -- using the
> PMDs is actually free, and it's really none of the app's business how
> the page cache chooses to cache things.

What should be done to be consistent with the collapse side here, for
file/shmem if at all? Answering the question, "can this memory be
backed by THPs" is becoming really complex, and that THPelligble smaps
field is becoming increasingly more difficult to use.