Re: [RFC PATCH 3/3] mm: enable filesystem page-cache large folios without THP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] mm: PG_has_hwpoisoned stays gated on THP when
  CONFIG_LARGE_FOLIO=y
- [Medium] mm: MAX_PAGECACHE_ORDER is not clamped to MAX_PAGE_ORDER
  without THP
- [Medium] mm: LARGE_FOLIO does not select MM_ID for
  folio_maybe_mapped_shared()
--

commit f8230ba10447d9debf48d1c6aebdac69481344a4
Author: Fang Xieyan <fangxy@xiaopeng.com>

mm: enable filesystem page-cache large folios without THP

This commit gates the page-cache folio order accessors on CONFIG_LARGE_FOLIO
in addition to CONFIG_TRANSPARENT_HUGEPAGE so filesystems that opt in to
large folios can use them without THP enabled, while keeping shmem gated on
CONFIG_TRANSPARENT_HUGEPAGE.

> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 73af18a373670..2102733c109c3 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -393,7 +393,8 @@ static inline void mapping_set_gfp_mask(struct address_space *m, gfp_t mask)
>   */
>  static inline size_t mapping_max_folio_size_supported(void)
>  {
> -	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> +	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) ||
> +	    IS_ENABLED(CONFIG_LARGE_FOLIO))
>  		return 1U << (PAGE_SHIFT + MAX_PAGECACHE_ORDER);
>  	return PAGE_SIZE;
>  }

[Severity: Medium]
Can MAX_PAGECACHE_ORDER exceed MAX_PAGE_ORDER when CONFIG_LARGE_FOLIO is
enabled without CONFIG_TRANSPARENT_HUGEPAGE?

Without THP, PREFERRED_MAX_PAGECACHE_ORDER is 8 and MAX_PAGECACHE_ORDER is
only clamped to MAX_XAS_ORDER.  On configurations where MAX_PAGE_ORDER < 8
(such as powerpc or sh with 64K pages and a smaller ARCH_FORCE_MAX_ORDER),
mapping_set_large_folios() sets mapping_max_folio_order() to 8.  Once
sequential readahead in page_cache_ra_order() scales ra->order above
MAX_PAGE_ORDER, ra_alloc_folio() -> filemap_alloc_folio() is rejected by
alloc_order_allowed() without retrying smaller orders, aborting large-folio
readahead and falling back to order-0 allocations in do_page_cache_ra().

> @@ -418,7 +419,8 @@ static inline void mapping_set_folio_order_range(struct address_space *mapping,
>  						 unsigned int min,
>  						 unsigned int max)
>  {
> -	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> +	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
> +	    !IS_ENABLED(CONFIG_LARGE_FOLIO))
>  		return;
>  
>  	if (min > MAX_PAGECACHE_ORDER)
> @@ -459,7 +461,8 @@ static inline void mapping_set_large_folios(struct address_space *mapping)
>  static inline unsigned int
>  mapping_max_folio_order(const struct address_space *mapping)
>  {
> -	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> +	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
> +	    !IS_ENABLED(CONFIG_LARGE_FOLIO))
>  		return 0;
>  	return (mapping->flags & AS_FOLIO_ORDER_MAX_MASK) >> AS_FOLIO_ORDER_MAX;
>  }
> @@ -467,7 +470,8 @@ mapping_max_folio_order(const struct address_space *mapping)
>  static inline unsigned int
>  mapping_min_folio_order(const struct address_space *mapping)
>  {
> -	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
> +	if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) &&
> +	    !IS_ENABLED(CONFIG_LARGE_FOLIO))
>  		return 0;
>  	return (mapping->flags & AS_FOLIO_ORDER_MIN_MASK) >> AS_FOLIO_ORDER_MIN;
>  }
> @@ -500,8 +504,9 @@ static inline pgoff_t mapping_align_index(const struct address_space *mapping,
>  }
>  
>  /*
> - * Large folio support currently depends on THP.  These dependencies are
> - * being worked on but are not yet fixed.
> + * Large folio support is opt-in per mapping and requires CONFIG_LARGE_FOLIO.
> + * It does not depend on THP; CONFIG_LARGE_FOLIO defaults to y when
> + * CONFIG_TRANSPARENT_HUGEPAGE is enabled.
>   */
>  static inline bool mapping_large_folio_support(const struct address_space *mapping)
>  {

[Severity: High]
Should FOLIO_FLAG(has_hwpoisoned, FOLIO_SECOND_PAGE) in
include/linux/page-flags.h also be enabled for CONFIG_LARGE_FOLIO?

include/linux/page-flags.h:
    #if defined(CONFIG_MEMORY_FAILURE) && defined(CONFIG_TRANSPARENT_HUGEPAGE)
    FOLIO_FLAG(has_hwpoisoned, FOLIO_SECOND_PAGE)
    #else
    FOLIO_FLAG_FALSE(has_hwpoisoned)
    #endif

With CONFIG_LARGE_FOLIO=y, CONFIG_TRANSPARENT_HUGEPAGE=n, and
CONFIG_MEMORY_FAILURE=y, folio_set_has_hwpoisoned() is a no-op and
folio_test_has_hwpoisoned() always returns false.  When memory_failure()
poisons a tail page of a large page-cache folio and the folio remains large
after split_huge_page_to_order(p, min_order_for_split(folio)) (for example
when mapping_min_folio_order() > 0 or splitting fails),
folio_contain_hwpoisoned_page() returns false.  shrink_folio_list() then
reclaims and frees the poisoned large folio instead of skipping it, and
__free_pages_prepare() (which only intercepts order-0 PageHWPoison pages)
either triggers bad_page() or returns the poisoned tail page to the buddy
allocator.

> diff --git a/mm/Kconfig b/mm/Kconfig
> index 9f97ad6982bc8..7bbed71af85b4 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -831,13 +831,20 @@ config LARGE_FOLIO
>  	bool "Large folio support for the page cache (without THP)" if !TRANSPARENT_HUGEPAGE
>  	depends on MMU
>  	select XARRAY_MULTI
> +	# Keep LARGE_FOLIO enabled whenever THP is enabled:
> +	# huge_memory.o is built for TRANSPARENT_HUGEPAGE and calls into
> +	# folio_split.o, which is built for LARGE_FOLIO.
>  	default y if TRANSPARENT_HUGEPAGE
>  	default n

[Severity: Medium]
Should LARGE_FOLIO also select MM_ID (or MM_ID if !PREEMPT_RT) like
TRANSPARENT_HUGEPAGE does?

include/linux/mm.h:folio_maybe_mapped_shared() {
    ...
	if (!IS_ENABLED(CONFIG_MM_ID))
		return true;

	if (mapcount <= 1)
		return false;
    ...
}

When CONFIG_LARGE_FOLIO=y and CONFIG_TRANSPARENT_HUGEPAGE=n, CONFIG_MM_ID is
not selected, so folio_maybe_mapped_shared() unconditionally returns true for
every large page-cache folio even when mapped by a single process with
mapcount <= 1.  As a result, unprivileged move_pages() fails with -EACCES in
__add_folio_for_migration(), mbind(MPOL_MF_MOVE) and migrate_pages() skip
those folios in migrate_folio_add(), and MADV_COLD / MADV_PAGEOUT refuse to
split sub-ranges in madvise_cold_or_pageout_pte_range().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110035205.3036-1-fangxy@xiaopeng.com?part=3