linux-kernel - Re: [PATCH 6/7] mm: folio_may_be_cached() unless folio_test

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: <7113d289-fb8e-4589-7eb5-1f7139965ade@google.com>
Date: Mon, 8 Sep 2025 04:19:56 -0700 (PDT)
From: Hugh Dickins <hughd@...gle.com>
To: David Hildenbrand <david@...hat.com>
cc: Hugh Dickins <hughd@...gle.com>, Andrew Morton <akpm@...ux-foundation.org>, 
    Will Deacon <will@...nel.org>, Shivank Garg <shivankg@....com>, 
    Matthew Wilcox <willy@...radead.org>, 
    Christoph Hellwig <hch@...radead.org>, Keir Fraser <keirf@...gle.com>, 
    Jason Gunthorpe <jgg@...pe.ca>, John Hubbard <jhubbard@...dia.com>, 
    Frederick Mayle <fmayle@...gle.com>, Peter Xu <peterx@...hat.com>, 
    "Aneesh Kumar K.V" <aneesh.kumar@...nel.org>, 
    Johannes Weiner <hannes@...xchg.org>, Vlastimil Babka <vbabka@...e.cz>, 
    Alexander Krabler <Alexander.Krabler@...a.com>, 
    Ge Yang <yangge1116@....com>, Li Zhe <lizhe.67@...edance.com>, 
    Chris Li <chrisl@...nel.org>, Yu Zhao <yuzhao@...gle.com>, 
    Axel Rasmussen <axelrasmussen@...gle.com>, 
    Yuanchu Xie <yuanchu@...gle.com>, Wei Xu <weixugc@...gle.com>, 
    Konstantin Khlebnikov <koct9i@...il.com>, linux-kernel@...r.kernel.org, 
    linux-mm@...ck.org
Subject: Re: [PATCH 6/7] mm: folio_may_be_cached() unless
 folio_test_large()

On Mon, 1 Sep 2025, David Hildenbrand wrote:
> On 31.08.25 11:16, Hugh Dickins wrote:
> > mm/swap.c and mm/mlock.c agree to drain any per-CPU batch as soon as
> > a large folio is added: so collect_longterm_unpinnable_folios() just
> > wastes effort when calling lru_add_drain_all() on a large folio.
> > 
> > But although there is good reason not to batch up PMD-sized folios,
> > we might well benefit from batching a small number of low-order mTHPs
> > (though unclear how that "small number" limitation will be implemented).
> > 
> > So ask if folio_may_be_cached() rather than !folio_test_large(), to
> > insulate those particular checks from future change.  Name preferred
> > to "folio_is_batchable" because large folios can well be put on a batch:
> > it's just the per-CPU LRU caches, drained much later, which need care.
> > 
> > Marked for stable, to counter the increase in lru_add_drain_all()s
> > from "mm/gup: check ref_count instead of lru before migration".
> > 
> > Suggested-by: David Hildenbrand <david@...hat.com>
> > Signed-off-by: Hugh Dickins <hughd@...gle.com>
> > Cc: <stable@...r.kernel.org>
> > ---
> >   include/linux/swap.h | 10 ++++++++++
> >   mm/gup.c             |  5 +++--
> >   mm/mlock.c           |  6 +++---
> >   mm/swap.c            |  2 +-
> >   4 files changed, 17 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/linux/swap.h b/include/linux/swap.h
> > index 2fe6ed2cc3fd..b49a61c32238 100644
> > --- a/include/linux/swap.h
> > +++ b/include/linux/swap.h
> > @@ -385,6 +385,16 @@ void folio_add_lru_vma(struct folio *, struct
> > vm_area_struct *);
> >   void mark_page_accessed(struct page *);
> >   void folio_mark_accessed(struct folio *);
> >   
> 
> Two smaller things:
> 
> (1) We have other "folio_maybe_*" functions, so this one should likely
>     better start with that as well.
> 
> (2) With things like fscache in mind, the function can be a bit
>     misleading.
> 
> So I wonder if (a) we should just add kerneldoc to document it clearly (lru
> cache, mlock cache?) and (b) maybe call it folio_may_be_lru_cached(). Not sure
> if we can find a better abstraction for these two caches.
> 
> Thinking again, "maybe_cached" might be a bit misleading because it implements
> a very very very bad heuristic for small folios.
> 
> Maybe it's more like "supports being cached".
> 
> folio_lru_caching_supported()

folio_may_be_cached() -> folio_may_be_lru_cached(), yes, that's
very much better, thanks.

(Settimg aside that I've never perceived those pagevecs/batches as a
"cache"; but lru_cache_disable() gave us that terminology, and we've
gone with the flow ever since.  lru_add_drain() would be better named
lru_cache_drain() now, I've always got hung up on "adding a drain".)

"may be" rather than "maybe" was intentional: perhaps too subtle,
but to a native speaker it neatly expresses both the "we can do this"
and "might this have been done" cases.

kernel-doc?  I don't think so, this is very much an mm-internal
matter, and I don't care for the way kernel-doc forces us towards
boilerplate ("@folio: The folio.") rather than helpful comment.

Hugh