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

["David Hildenbrand (Arm)" <david@kernel.org>]

On 11/4/26 14:00, Kiryl Shutsemau (Meta) wrote:
> From: Linus Torvalds <torvalds@linux-foundation.org>
> 
> Reading a page cache folio without taking a reference would keep CPUs
> that read the same file from bouncing its refcount.  But without a
> reference the folio can be freed, and its memory reused, while the
> reader copies from it.
> 
> Add PG_rcu_free: the free of a folio with the flag waits for an RCU
> grace period.  A reader that saw the flag under rcu_read_lock(), and
> then still found the folio where it looked, can read it until
> rcu_read_unlock().  A split copies the flag, as such a reader may still
> be reading the new folios.
> 
> The folios wait on per-CPU lists, one grace period in flight per CPU:
> call_rcu() per folio would queue a callback for every folio of a
> truncated file.  A work, which can reschedule, frees each batch.
> Reclaim depends on these frees, so the workqueue is WQ_MEM_RECLAIM.
> 
> The flag is 64-bit only (CONFIG_PAGE_RCU_FREE): page flags are scarce
> on 32-bit, and the contention this avoids is a problem of large
> machines.  It is off on PREEMPT_RT, where expedited grace periods are
> normal ones, so the waiting pages cannot be freed quickly when memory
> runs short.
> 
> pgfree_rcu in /proc/vmstat counts the deferred pages.  Nothing sets the
> flag yet.
> 
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> [kas: rework for flagging at read time, 64-bit only and not PREEMPT_RT,
>  shorter comments and changelog]
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> ---
>  include/linux/page-flags.h     |  21 +++++++
>  include/linux/vm_event_item.h  |   1 +
>  include/trace/events/mmflags.h |   7 +++
>  mm/Kconfig                     |  15 +++++
>  mm/huge_memory.c               |   1 +
>  mm/memory-failure.c            |   4 ++
>  mm/page_alloc.c                | 105 +++++++++++++++++++++++++++++++++
>  mm/vmstat.c                    |   1 +
>  8 files changed, 155 insertions(+)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 7a863572adce..a1e52a1f8a52 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -111,6 +111,9 @@ enum pageflags {
>  	PG_swapbacked,		/* Page is backed by RAM/swap */
>  	PG_unevictable,		/* Page is "unevictable"  */
>  	PG_dropbehind,		/* drop pages on IO completion */
> +#ifdef CONFIG_PAGE_RCU_FREE
> +	PG_rcu_free,		/* free only after an RCU grace period */
> +#endif
>  #ifdef CONFIG_MMU
>  	PG_mlocked,		/* Page is vma mlocked */
>  #endif
> @@ -602,6 +605,24 @@ FOLIO_FLAG(dropbehind, FOLIO_HEAD_PAGE)
>  	FOLIO_TEST_CLEAR_FLAG(dropbehind, FOLIO_HEAD_PAGE)
>  	__FOLIO_SET_FLAG(dropbehind, FOLIO_HEAD_PAGE)
>  
> +/*
> + * A lockless lookup that sees PG_rcu_free and then still finds the folio
> + * where it looked may read it until rcu_read_unlock().  Set under a
> + * reference, cleared only by the free.  Not for hugetlb or
> + * free_pages_nolock().
> + */
> +#ifdef CONFIG_PAGE_RCU_FREE
> +FOLIO_TEST_FLAG(rcu_free, FOLIO_HEAD_PAGE)
> +	FOLIO_TEST_SET_FLAG(rcu_free, FOLIO_HEAD_PAGE)
> +	__FOLIO_CLEAR_FLAG(rcu_free, FOLIO_HEAD_PAGE)
> +#define __PG_RCU_FREE (1UL << PG_rcu_free)
> +#else
> +FOLIO_TEST_FLAG_FALSE(rcu_free)
> +	FOLIO_TEST_SET_FLAG_FALSE(rcu_free)
> +	__FOLIO_CLEAR_FLAG_NOOP(rcu_free)
> +#define __PG_RCU_FREE 0
> +#endif
> +
>  #ifdef CONFIG_HIGHMEM
>  /*
>   * Must use a macro here due to header dependency issues. page_zone() is not
> diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
> index 2628ccda076a..cbe849701653 100644
> --- a/include/linux/vm_event_item.h
> +++ b/include/linux/vm_event_item.h
> @@ -36,6 +36,7 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
>  		FOR_ALL_ZONES(ALLOCSTALL)
>  		FOR_ALL_ZONES(PGSCAN_SKIP)
>  		PGFREE, PGACTIVATE, PGDEACTIVATE, PGLAZYFREE,
> +		PGFREE_RCU,
>  		PGFAULT, PGMAJFAULT,
>  		PGLAZYFREED,
>  		PGREUSE,
> diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
> index 935893e5ea53..47c3411a75e8 100644
> --- a/include/trace/events/mmflags.h
> +++ b/include/trace/events/mmflags.h
> @@ -117,6 +117,12 @@ TRACE_DEFINE_ENUM(___GFP_LAST_BIT);
>  #define IF_HAVE_PG_IDLE(_name)
>  #endif
>  
> +#ifdef CONFIG_PAGE_RCU_FREE
> +#define IF_HAVE_PG_RCU_FREE(_name) ,{1UL << PG_##_name, __stringify(_name)}
> +#else
> +#define IF_HAVE_PG_RCU_FREE(_name)
> +#endif
> +
>  #ifdef CONFIG_ARCH_USES_PG_ARCH_2
>  #define IF_HAVE_PG_ARCH_2(_name) ,{1UL << PG_##_name, __stringify(_name)}
>  #else
> @@ -152,6 +158,7 @@ TRACE_DEFINE_ENUM(___GFP_LAST_BIT);
>  	DEF_PAGEFLAG_NAME(swapbacked),					\
>  	DEF_PAGEFLAG_NAME(unevictable),					\
>  	DEF_PAGEFLAG_NAME(dropbehind)					\
> +IF_HAVE_PG_RCU_FREE(rcu_free)						\
>  IF_HAVE_PG_MLOCK(mlocked)						\
>  IF_HAVE_PG_HWPOISON(hwpoison)						\
>  IF_HAVE_PG_IDLE(idle)							\
> 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 ?

> +	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.

> +
> +	  64-bit only: it takes a page flag, and those are scarce on
> +	  32-bit.  Not on PREEMPT_RT, where expedited grace periods are
> +	  normal ones, so freeing the waiting pages on demand would stall.
> +
>  config PHYS_ADDR_T_64BIT
>  	def_bool 64BIT
>  
> 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.

>  				 LRU_GEN_MASK | LRU_REFS_MASK));
>  
>  		new_folio->mapping = folio->mapping;
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index a8b03e2920ba..7dde7c3c9a0c 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -211,6 +211,10 @@ static bool page_handle_poison(struct page *page, bool hugepage_or_freepage, boo
>  			return false;
>  	}
>  
> +	/* The free of a hwpoisoned page is not deferred: wait for readers */
> +	if (release && folio_test_rcu_free(page_folio(page)))
> +		synchronize_rcu();
> +
>  	SetPageHWPoison(page);
>  	if (release)
>  		put_page(page);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 12fac9084c48..a371e92da273 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1313,6 +1313,96 @@ static inline void pgalloc_tag_sub_pages(struct alloc_tag *tag, unsigned int nr)
>  
>  #endif /* CONFIG_MEM_ALLOC_PROFILING */
>  
> +/*
> + * Each CPU has at most one grace period in flight (@queued), covering
> + * @batch.  Folios freed meanwhile wait on @pending for the next one.
> + */
> +struct rcu_free_pcpu {
> +	spinlock_t lock;
> +	bool queued;
> +	struct list_head pending;
> +	struct list_head batch;
> +	struct rcu_work rwork;
> +};
> +
> +static DEFINE_PER_CPU(struct rcu_free_pcpu, rcu_free_pcpu);
> +
> +static struct workqueue_struct *rcu_free_wq __ro_after_init;
> +
> +static void rcu_free_start(struct rcu_free_pcpu *r)
> +{
> +	lockdep_assert_held(&r->lock);
> +
> +	if (r->queued || list_empty(&r->pending))
> +		return;
> +
> +	list_splice_init(&r->pending, &r->batch);
> +	r->queued = true;
> +	WARN_ON_ONCE(!queue_rcu_work(rcu_free_wq, &r->rwork));
> +}
> +
> +static void rcu_free_pages(struct list_head *list)
> +{
> +	struct folio_batch fbatch;
> +	struct folio *folio, *next;
> +
> +	folio_batch_init(&fbatch);
> +	list_for_each_entry_safe(folio, next, list, lru) {
> +		if (!folio_batch_add(&fbatch, folio)) {
> +			free_unref_folios(&fbatch);
> +			cond_resched_tasks_rcu_qs();
> +		}
> +	}
> +	if (folio_batch_count(&fbatch))
> +		free_unref_folios(&fbatch);
> +}
> +
> +static void rcu_free_workfn(struct work_struct *work)
> +{
> +	struct rcu_free_pcpu *r = container_of(to_rcu_work(work),
> +					       struct rcu_free_pcpu, rwork);
> +	LIST_HEAD(done);
> +
> +	spin_lock_irq(&r->lock);
> +	list_splice_init(&r->batch, &done);
> +	r->queued = false;
> +	rcu_free_start(r);
> +	spin_unlock_irq(&r->lock);
> +
> +	rcu_free_pages(&done);
> +}
> +
> +static void rcu_free_defer(struct folio *folio)
> +{
> +	struct rcu_free_pcpu *r = raw_cpu_ptr(&rcu_free_pcpu);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&r->lock, flags);
> +	list_add_tail(&folio->lru, &r->pending);
> +	rcu_free_start(r);
> +	spin_unlock_irqrestore(&r->lock, flags);
> +}
> +
> +static void __init rcu_free_init(void)
> +{
> +	int cpu;
> +
> +	if (!IS_ENABLED(CONFIG_PAGE_RCU_FREE))
> +		return;
> +
> +	rcu_free_wq = alloc_workqueue("rcu_free",
> +				      WQ_MEM_RECLAIM | WQ_PERCPU, 0);
> +
> +	for_each_possible_cpu(cpu) {
> +		struct rcu_free_pcpu *r = per_cpu_ptr(&rcu_free_pcpu, cpu);
> +
> +		spin_lock_init(&r->lock);
> +		INIT_LIST_HEAD(&r->pending);
> +		INIT_LIST_HEAD(&r->batch);
> +		INIT_RCU_WORK(&r->rwork, rcu_free_workfn);
> +	}
> +}
> +
>  static __always_inline bool __free_pages_prepare(struct page *page,
>  		unsigned int order, fpi_t fpi_flags)
>  {
> @@ -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).

So maybe this handling could be placed into mm/folio.c instead, just a quick
thought ... reading about folio batches above I think that might be the right
direction, we are mixing too much page vs. folio things here.

-- 
Cheers,

David