Re: [PATCH v3] mm/lruvec: trace LRU add drains and drain-all requests

[Shakeel Butt <shakeel.butt@linux.dev>]

On Wed, Jun 17, 2026 at 01:11:16PM +0200, David Hildenbrand (Arm) wrote:
> On 6/10/26 21:52, JP Kobryn wrote:
> > LRU add batches can be drained before they reach capacity. This can be a
> > source of LRU lock contention, but it is not currently possible to
> > attribute these drains to callers with existing tracepoints.
> > 
> > Add mm_lru_add_drain to report the CPU and lru_add batch count when an
> > lru_add batch is drained. This allows tracing to distinguish full drains
> > from partial drains and attribute them to the calling stack.
> > 
> > Add mm_lru_add_drain_all to capture callers of __lru_add_drain_all and
> > whether they set the force flag for all CPUs. The tracepoint resembles
> > the signature of the enclosing function, but is needed because of
> > potential inlining.
> > 
> > Signed-off-by: JP Kobryn <jp.kobryn@linux.dev>
> > ---
> >  include/trace/events/pagemap.h | 37 ++++++++++++++++++++++++++++++++++
> >  mm/swap.c                      |  7 ++++++-
> >  2 files changed, 43 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/trace/events/pagemap.h b/include/trace/events/pagemap.h
> > index 171524d3526d..ff3da07ccb40 100644
> > --- a/include/trace/events/pagemap.h
> > +++ b/include/trace/events/pagemap.h
> > @@ -77,6 +77,43 @@ TRACE_EVENT(mm_lru_activate,
> >  	TP_printk("folio=%p pfn=0x%lx", __entry->folio, __entry->pfn)
> >  );
> >  
> > +TRACE_EVENT(mm_lru_add_drain,
> > +
> > +	TP_PROTO(int cpu, unsigned int nr),
> > +
> > +	TP_ARGS(cpu, nr),
> > +
> > +	TP_STRUCT__entry(
> > +		__field(int,		cpu	)
> > +		__field(unsigned int,	nr	)
> > +	),
> > +
> > +	TP_fast_assign(
> > +		__entry->cpu	= cpu;
> > +		__entry->nr	= nr;
> > +	),
> > +
> > +	TP_printk("cpu=%d nr=%u", __entry->cpu, __entry->nr)
> > +);
> > +
> > +TRACE_EVENT(mm_lru_add_drain_all,
> > +
> > +	TP_PROTO(bool force_all_cpus),
> > +
> > +	TP_ARGS(force_all_cpus),
> > +
> > +	TP_STRUCT__entry(
> > +		__field(bool,	force_all_cpus	)
> > +	),
> > +
> > +	TP_fast_assign(
> > +		__entry->force_all_cpus	= force_all_cpus;
> > +	),
> > +
> > +	TP_printk("force_all_cpus=%s",
> > +		__entry->force_all_cpus ? "true" : "false")
> > +);
> > +
> >  #endif /* _TRACE_PAGEMAP_H */
> >  
> >  /* This part must be outside protection */
> > diff --git a/mm/swap.c b/mm/swap.c
> > index 588f50d8f1a8..e14b7612f896 100644
> > --- a/mm/swap.c
> > +++ b/mm/swap.c
> > @@ -694,9 +694,12 @@ void lru_add_drain_cpu(int cpu)
> >  {
> >  	struct cpu_fbatches *fbatches = &per_cpu(cpu_fbatches, cpu);
> >  	struct folio_batch *fbatch = &fbatches->lru_add;
> > +	unsigned int nr_folios_add = folio_batch_count(fbatch);
> >  
> > -	if (folio_batch_count(fbatch))
> > +	if (nr_folios_add) {
> >  		folio_batch_move_lru(fbatch, lru_add);
> > +		trace_mm_lru_add_drain(cpu, nr_folios_add);
> > +	}
> >  
> >  	fbatch = &fbatches->lru_move_tail;
> >  	/* Disabling interrupts below acts as a compiler barrier. */
> > @@ -869,6 +872,8 @@ static inline void __lru_add_drain_all(bool force_all_cpus)
> >  	if (WARN_ON(!mm_percpu_wq))
> >  		return;
> >  
> > +	trace_mm_lru_add_drain_all(force_all_cpus);
> > +
> >  	/*
> >  	 * Guarantee folio_batch counter stores visible by this CPU
> >  	 * are visible to other CPUs before loading the current drain
> 
> Given that trace events can quickly become stable ABI [1], are we really sure we
> want to add this?

Yes, I think so as this is useful to get insights into lru cache draining.
Trace events being stable or not is secondary IMHO. If in future we rearchitect
the lru page handling where there is no cache draining anymore, we can make
these a noops.

> 
> [1] https://lore.kernel.org/r/20260603130006.7d2c4a62@gandalf.local.home
> 
> -- 
> Cheers,
> 
> David