Re: [RFC PATCH v2 01/18] mm: free PG_rcu_free folios only after an RCU grace period

[Kiryl Shutsemau <kas@kernel.org>]

On Thu, Nov 05, 2026 at 09:37:31PM +0100, David Hildenbrand (Arm) wrote:
> > diff --git a/mm/Kconfig b/mm/Kconfig
> > index 604c58199acb..6e5341b0bf64 100644
> > --- a/mm/Kconfig
> > +++ b/mm/Kconfig
> > @@ -718,6 +718,21 @@ config PCP_BATCH_SCALE_MAX
> >  	  latency.  This option sets the upper limit of scale factor to limit
> >  	  the maximum latency.
> >  
> > +config PAGE_RCU_FREE
> 
> I think we really want to focus this only on folios (like the current code does).
> 
> FOLIO_RCU_FREE ?

Sure.

> > +	def_bool y
> > +	depends on 64BIT
> > +	depends on !PREEMPT_RT
> > +	help
> > +	  Reads from the page cache can copy from folios that are read
> > +	  again and again without taking a reference on them, so small
> > +	  reads of one file from many CPUs do not contend on the folio's
> > +	  reference count.  The cost is that memory freed from such
> > +	  folios becomes reusable only after an RCU grace period.
> 
> While that description is helpful, that's not really what this patch, or rather
> the basic infrastructure in page_alloc, does.
> 
> I'd suggest to have a dedicated FOLIO_RCU_FREE config and maybe another one
> (that selects/depends on) FOLIO_RCU_FREE that actually enables the pagecache use
> and documents the details.

I will make the help describe what this patch does.  The page cache is
the only user, and both symbols would be def_bool without a prompt, so
I would rather add the second one with a second user.

Does that work for you?

> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 1e5d68acf62a..25082949a4ca 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -3743,6 +3743,7 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
> >  #endif
> >  				 (1L << PG_dirty) |
> >  				 (1L << PG_dropbehind) |
> > +				 __PG_RCU_FREE |
> 
> That looks a bit odd at first, as it's very different to the other once. But no
> easy way around assuming that the flag is not always around.
> 
> Maybe we should just have this be a const mask constructed somewhere using BIT()
> and __PG_RCU_FREE etc, instead of having this growing list in this user here.

Makes sense.  Something like the diff below, on top of this patch.
Introducing split_copy_flags would be a preparatory patch, and this one
would only add __PG_RCU_FREE to it.

> > @@ -1327,6 +1417,19 @@ static __always_inline bool __free_pages_prepare(struct page *page,
> >  
> >  	VM_BUG_ON_PAGE(PageTail(page), page);
> >  
> > +	/*
> > +	 * Not for a hwpoisoned order-0 page: it never reaches the allocator,
> > +	 * and page_handle_poison() takes a reference right after this free.
> > +	 */
> > +	if (unlikely(folio_test_rcu_free(folio)) &&
> > +	    !(PageHWPoison(page) && !order)) {
> > +		VM_WARN_ON_ONCE(fpi_flags & FPI_NOLOCK);
> > +		__folio_clear_rcu_free(folio);
> > +		count_vm_events(PGFREE_RCU, 1 << order);
> > +		rcu_free_defer(folio);
> > +		return false;
> > +	}
> I'm wondering whether we want to intercept on the folio freeing path instead?
> That would be __folio_put(), before we go into free_frozen_pages() used by other
> machinery that does not apply (slab etc).

Not all page cache folios are freed through __folio_put().  Reclaim
frees them with free_unref_folios() directly (shrink_folio_list(),
move_folios_to_lru()), and so do folios_put_refs() and the LRU batches.
__free_pages_prepare() is the one place they all pass.

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index a1e52a1f8a52..6d4e534c9ccc 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -1185,6 +1185,18 @@ static __always_inline void __ClearPageAnonExclusive(struct page *page)
 #define __PG_MLOCKED		0
 #endif
 
+#ifdef CONFIG_ARCH_USES_PG_ARCH_2
+#define __PG_ARCH_2		BIT(PG_arch_2)
+#else
+#define __PG_ARCH_2		0
+#endif
+
+#ifdef CONFIG_ARCH_USES_PG_ARCH_3
+#define __PG_ARCH_3		BIT(PG_arch_3)
+#else
+#define __PG_ARCH_3		0
+#endif
+
 /*
  * Flags checked when a page is freed.  Pages being freed should not have
  * these flags set.  If they are, there is a problem.
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 25082949a4ca..44a1aaa3f834 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3689,6 +3689,23 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
 	const bool handle_hwpoison = folio_test_has_hwpoisoned(folio) && new_order;
 	long new_nr_pages = 1 << new_order;
 	long nr_pages = 1 << old_order;
+	const unsigned long split_copy_flags =
+		BIT(PG_referenced) |
+		BIT(PG_swapbacked) |
+		BIT(PG_swapcache) |
+		__PG_MLOCKED |
+		BIT(PG_uptodate) |
+		BIT(PG_active) |
+		BIT(PG_workingset) |
+		BIT(PG_locked) |
+		BIT(PG_unevictable) |
+		__PG_ARCH_2 |
+		__PG_ARCH_3 |
+		BIT(PG_dirty) |
+		BIT(PG_dropbehind) |
+		__PG_RCU_FREE |
+		LRU_GEN_MASK |
+		LRU_REFS_MASK;
 	long i;
 
 	folio_clear_has_hwpoisoned(folio);
@@ -3725,26 +3742,7 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
 		 * PG_anon_exclusive (-> PG_mappedtodisk) for these here.
 		 */
 		new_folio->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
-		new_folio->flags.f |= (folio->flags.f &
-				((1L << PG_referenced) |
-				 (1L << PG_swapbacked) |
-				 (1L << PG_swapcache) |
-				 (1L << PG_mlocked) |
-				 (1L << PG_uptodate) |
-				 (1L << PG_active) |
-				 (1L << PG_workingset) |
-				 (1L << PG_locked) |
-				 (1L << PG_unevictable) |
-#ifdef CONFIG_ARCH_USES_PG_ARCH_2
-				 (1L << PG_arch_2) |
-#endif
-#ifdef CONFIG_ARCH_USES_PG_ARCH_3
-				 (1L << PG_arch_3) |
-#endif
-				 (1L << PG_dirty) |
-				 (1L << PG_dropbehind) |
-				 __PG_RCU_FREE |
-				 LRU_GEN_MASK | LRU_REFS_MASK));
+		new_folio->flags.f |= folio->flags.f & split_copy_flags;
 
 		new_folio->mapping = folio->mapping;
 		new_folio->index = folio->index + i;
-- 
  Kiryl Shutsemau / Kirill A. Shutemov