linux-kernel - Re: [EXTERNAL] [PATCH] mm/thp: fix "mm: thp: kill __transhuge_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZN5um2yIXFcxiFjS@casper.infradead.org>
Date:   Thu, 17 Aug 2023 20:01:47 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Zach O'Keefe <zokeefe@...gle.com>
Cc:     Saurabh Singh Sengar <ssengar@...rosoft.com>,
        Dan Williams <dan.j.williams@...el.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Yang Shi <shy828301@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [EXTERNAL] [PATCH] mm/thp: fix "mm: thp: kill
 __transhuge_page_enabled()"

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.

> > > 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 hugepage
> > > when THP="never". Also, if THP="always", we might as well skip the
> > > VM_HUGEPAGE check, and try the final pmd install (and save khugepaged
> > > 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 based
> > on history what to do.  This is far superior to having a sysadmin tell
> > 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.