Re: [PATCH v1 00/10] Remove READ_ONLY_THP_FOR_FS Kconfig

[Nico Pache <npache@redhat.com>]

On Sun, Apr 5, 2026 at 7:59 PM Zi Yan <ziy@nvidia.com> wrote:
>
> On 5 Apr 2026, at 13:38, Nico Pache wrote:
>
> > On Thu, Mar 26, 2026 at 7:43 PM Zi Yan <ziy@nvidia.com> wrote:
> >>
> >> Hi all,
> >>
> >> This patchset removes READ_ONLY_THP_FOR_FS Kconfig and enables creating
> >> read-only THPs for FSes with large folio support (the supported orders
> >> need to include PMD_ORDER) by default.
> >
> > Hi Zi,
> >
> > Thank you for tackling this :) Ill try to review the next version as
> > I'm a little behind on this thread.
>
> Sure. Thanks.
>
> >
> > Should we guard collapsing READ_ONLY_THPs with a sysctl? My fear is
> > workloads that convert READ_ONLY THPs into writable pages (assuming
> > this is common/possible; my understanding of FS is rather low),
> > leading to storms of thp splitting. Do you think this is a real
> > concern? I guess this is also true of read-only-->writable fs-THPS
> > even without khugepaged, correct?
>
> Why would a read-only THP need to be split when it becomes writable?
> After this patchset, a read-only THP can only be created on a FS that
> supports large folios (to be precise PMD THP). That means any write
> to that read-only THP would just change it to a writable THP.

Ah, okay. I was misremembering some stuff.

The concern I spotted earlier when investigating read-only THPs for
khugepaged was this:

For frequent yet short-lived writes on read-only pages (e.g., package
updates, log updates)
Wouldn't we get destructive cycles of cache invalidations and refault storms?
Imagine such pages are shared (library, execs, etc) across many processes.

When these files are marked for writing we must invalidate all of
their mappings, destroying their Page Tables and PageCache. Now all
processes must refault these mappings.

Once the write is complete, they are eligible for read-only promotion again.

The part I didn't understand (thanks Claude) is that this truncation
path in do_dentry_open is only taken for mappings/Filesystems that do
not support large folios, as only those filesystems track
mapping->nr_thps. Furthermore, with FS that natively support large
folios, khugepaged does not need to re-collapse these pages, as even
if this was the case they would be refaulted as THPs.

TLDR: My concern is not a real concern.

Cheers,
-- Nico

>
> Let me know if I miss anything.
>
> >
> > Cheers,
> > -- Nico
> >
> >>
> >> The changes are:
> >> 1. collapse_file() from mm/khugepaged.c, instead of checking
> >>    CONFIG_READ_ONLY_THP_FOR_FS, makes sure the mapping_max_folio_order()
> >>    of struct address_space of the file is at least PMD_ORDER.
> >> 2. file_thp_enabled() also checks mapping_max_folio_order() instead.
> >> 3. truncate_inode_partial_folio() calls folio_split() directly instead
> >>    of the removed try_folio_split_to_order(), since large folios can
> >>    only show up on a FS with large folio support.
> >> 4. nr_thps is removed from struct address_space, since it is no longer
> >>    needed to drop all read-only THPs from a FS without large folio
> >>    support when the fd becomes writable. Its related filemap_nr_thps*()
> >>    are removed too.
> >> 5. folio_check_splittable() no longer checks READ_ONLY_THP_FOR_FS.
> >> 6. Updated comments in various places.
> >>
> >> Changelog
> >> ===
> >> From RFC[1]:
> >> 1. instead of removing READ_ONLY_THP_FOR_FS function entirely, turn it
> >>    on by default for all FSes with large folio support and the supported
> >>    orders includes PMD_ORDER.
> >>
> >> Suggestions and comments are welcome.
> >>
> >> Link: https://lore.kernel.org/all/20260323190644.1714379-1-ziy@nvidia.com/ [1]
> >>
> >> Zi Yan (10):
> >>   mm: remove READ_ONLY_THP_FOR_FS Kconfig option
> >>   mm/khugepaged: remove READ_ONLY_THP_FOR_FS check
> >>   mm: fs: remove filemap_nr_thps*() functions and their users
> >>   fs: remove nr_thps from struct address_space
> >>   mm/huge_memory: remove READ_ONLY_THP_FOR_FS from file_thp_enabled()
> >>   mm/huge_memory: remove folio split check for READ_ONLY_THP_FOR_FS
> >>   mm/truncate: use folio_split() in truncate_inode_partial_folio()
> >>   fs/btrfs: remove a comment referring to READ_ONLY_THP_FOR_FS
> >>   selftests/mm: remove READ_ONLY_THP_FOR_FS in khugepaged
> >>   selftests/mm: remove READ_ONLY_THP_FOR_FS from comments in
> >>     guard-regions
> >>
> >>  fs/btrfs/defrag.c                          |  3 --
> >>  fs/inode.c                                 |  3 --
> >>  fs/open.c                                  | 27 ----------------
> >>  include/linux/fs.h                         |  5 ---
> >>  include/linux/huge_mm.h                    | 25 ++-------------
> >>  include/linux/pagemap.h                    | 29 -----------------
> >>  mm/Kconfig                                 | 11 -------
> >>  mm/filemap.c                               |  1 -
> >>  mm/huge_memory.c                           | 29 ++---------------
> >>  mm/khugepaged.c                            | 36 +++++-----------------
> >>  mm/truncate.c                              |  8 ++---
> >>  tools/testing/selftests/mm/guard-regions.c |  9 +++---
> >>  tools/testing/selftests/mm/khugepaged.c    |  4 +--
> >>  13 files changed, 23 insertions(+), 167 deletions(-)
> >>
> >> --
> >> 2.43.0
> >>
>
>
> --
> Best Regards,
> Yan, Zi
>