All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Vlastimil Babka (SUSE)" <vbabka@kernel.org>
To: Shakeel Butt <shakeel.butt@linux.dev>,
	"David Hildenbrand (Arm)" <david@kernel.org>
Cc: JP Kobryn <jp.kobryn@linux.dev>,
	linux-mm@kvack.org, willy@infradead.org, usama.arif@linux.dev,
	akpm@linux-foundation.org, mhocko@suse.com, rostedt@goodmis.org,
	mhiramat@kernel.org, mathieu.desnoyers@efficios.com,
	kasong@tencent.com, qi.zheng@linux.dev, baohua@kernel.org,
	axelrasmussen@google.com, yuanchu@google.com, weixugc@google.com,
	chrisl@kernel.org, shikemeng@huaweicloud.com, nphamcs@gmail.com,
	baoquan.he@linux.dev, youngjun.park@lge.com,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v3] mm/lruvec: trace LRU add drains and drain-all requests
Date: Wed, 17 Jun 2026 20:18:57 +0200	[thread overview]
Message-ID: <1136baf3-3967-4202-9eaa-5fd667c235cf@kernel.org> (raw)
In-Reply-To: <ajK1YsIJmD2ImbAk@linux.dev>

On 6/17/26 17:03, Shakeel Butt wrote:
> 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.

Yeah and I don't recall ever that a change to a mm tracepoint would ever
break someone who'd complain and we'd have to revert it. These are niche
enough. So I think the risk is low.

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


      reply	other threads:[~2026-06-17 18:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-10 19:52 [PATCH v3] mm/lruvec: trace LRU add drains and drain-all requests JP Kobryn
2026-06-10 21:03 ` Barry Song
2026-06-10 21:13 ` Shakeel Butt
2026-06-17 11:11 ` David Hildenbrand (Arm)
2026-06-17 15:03   ` Shakeel Butt
2026-06-17 18:18     ` Vlastimil Babka (SUSE) [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1136baf3-3967-4202-9eaa-5fd667c235cf@kernel.org \
    --to=vbabka@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=baohua@kernel.org \
    --cc=baoquan.he@linux.dev \
    --cc=chrisl@kernel.org \
    --cc=david@kernel.org \
    --cc=jp.kobryn@linux.dev \
    --cc=kasong@tencent.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=mhocko@suse.com \
    --cc=nphamcs@gmail.com \
    --cc=qi.zheng@linux.dev \
    --cc=rostedt@goodmis.org \
    --cc=shakeel.butt@linux.dev \
    --cc=shikemeng@huaweicloud.com \
    --cc=usama.arif@linux.dev \
    --cc=weixugc@google.com \
    --cc=willy@infradead.org \
    --cc=youngjun.park@lge.com \
    --cc=yuanchu@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.