Re: [PATCH v4 1/5] mm/rmap: add tracepoint for rmap_walk

Re: [PATCH v4 1/5] mm/rmap: add tracepoint for rmap_walk

[David Hildenbrand (Arm)]

On 5/3/26 14:39, xu.xin16@zte.com.cn wrote:
> From: xu xin <xu.xin16@zte.com.cn>
> 
> Add a new tracepoint rmap_walk in mm/rmap.c to monitor reverse mapping
> traversal. The tracepoint records the duration (in nanoseconds), the
> type of the folio (KSM, anonymous, or file-backed), and the addresses
> of the folio and rmap_walk_control structures. The type determination
> is performed inside the tracepoint to keep the function itself
> lightweight.
> 
> '# cat /sys/kernel/tracing/trace
> '# tracer: nop
> '#
> '# entries-in-buffer/entries-written: 408/408   #P:4
> '#
> '#                     _-----=> irqs-off/BH-disabled
> '#                    / _----=> need-resched
> '#                   | / _---=> hardirq/softirq
> '#                   || / _--=> preempt-depth
> '#                   ||| / _-=> migrate-disable
> '#                   |||| /     delay
> '#TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
> '#   | |         |   |||||     |         |
> rmap-215     [001] .....   692.237079: rmap_walk: folio=000000000029ddcb rwc=00000000dac4cda0 duration_ns=828682 page_type=ksm locked=false
> rmap-215     [001] .....   692.239480: rmap_walk: folio=0000000092a21fd3 rwc=00000000986376ff duration_ns=905966 page_type=ksm locked=false
> rmap-230     [003] .....   692.583619: rmap_walk: folio=0000000037a237b6 rwc=0000000080fbbb0a duration_ns=107892 page_type=ksm locked=false
> rmap-230     [003] .....   692.584104: rmap_walk: folio=0000000031bdbf8b rwc=00000000b39c973a duration_ns=330886 page_type=ksm locked=false
> rmap-244     [003] .....   692.708706: rmap_walk: folio=000000009105fa6b rwc=0000000037d46cd7 duration_ns=987826 page_type=file locked=false
> rmap-244     [003] .....   692.709198: rmap_walk: folio=000000009105fa6b rwc=0000000093942e2c duration_ns=161733 page_type=file locked=false
> rmap-244     [003] .....   692.709606: rmap_walk: folio=000000009105fa6b rwc=0000000037d46cd7 duration_ns=54428 page_type=file locked=false
> rmap-244     [003] .....   692.709658: rmap_walk: folio=000000009105fa6b rwc=0000000093942e2c duration_ns=27192 page_type=file locked=false
> 
> Signed-off-by: xu xin <xu.xin16@zte.com.cn>
> ---
>  include/trace/events/rmap.h | 49 +++++++++++++++++++++++++++++++++++++
>  mm/rmap.c                   | 14 +++++++++++
>  2 files changed, 63 insertions(+)
>  create mode 100644 include/trace/events/rmap.h
> 
> diff --git a/include/trace/events/rmap.h b/include/trace/events/rmap.h
> new file mode 100644
> index 000000000000..987fa204d65d
> --- /dev/null
> +++ b/include/trace/events/rmap.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM rmap
> +
> +#if !defined(_TRACE_RMAP_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_RMAP_H
> +
> +#include <linux/tracepoint.h>
> +#include <linux/rmap.h>
> +
> +#define GET_RMAP_PAGE_TYPE(folio) (folio_test_ksm(folio) ? "ksm" : \
> +		(folio_test_anon(folio) ? "anon" : "file"))
> +
> +TRACE_EVENT(rmap_walk,
> +
> +	TP_PROTO(struct folio *folio, struct rmap_walk_control *rwc, u64 duration_ns, bool locked),
> +
> +	TP_ARGS(folio, rwc, duration_ns, locked),
> +
> +	TP_STRUCT__entry(
> +		__field(unsigned long, folio_addr)
> +		__field(unsigned long, rwc_addr)
> +		__field(u64, duration_ns)
> +		__string(page_type, GET_RMAP_PAGE_TYPE(folio))
> +		__field(bool, locked)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->folio_addr = (unsigned long)folio;
> +		__entry->rwc_addr = (unsigned long)rwc;
> +		__entry->duration_ns = duration_ns;
> +		__assign_str(page_type);
> +		__entry->locked = locked;
> +	),
> +
> +	TP_printk("folio=%p rwc=%p duration_ns=%llu page_type=%s locked=%s",
> +		(void *)(unsigned long)__entry->folio_addr,
> +		(void *)(unsigned long)__entry->rwc_addr,
> +		__entry->duration_ns,
> +		__get_str(page_type),
> +		__entry->locked ? "true" : "false")
> +);
> +
> +
> +
> +#endif /* _TRACE_RMAP_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 78b7fb5f367c..14bf8483f38b 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -75,11 +75,13 @@
>  #include <linux/userfaultfd_k.h>
>  #include <linux/mm_inline.h>
>  #include <linux/oom.h>
> +#include <linux/sched/clock.h>
> 
>  #include <asm/tlb.h>
> 
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/migrate.h>
> +#include <trace/events/rmap.h>
> 
>  #include "internal.h"
>  #include "swap.h"
> @@ -3098,23 +3100,35 @@ static void rmap_walk_file(struct folio *folio,
> 
>  void rmap_walk(struct folio *folio, struct rmap_walk_control *rwc)
>  {
> +	u64 ts_start, delta_ns;
> +	ts_start = local_clock();
> +
>  	if (unlikely(folio_test_ksm(folio)))
>  		rmap_walk_ksm(folio, rwc);
>  	else if (folio_test_anon(folio))
>  		rmap_walk_anon(folio, rwc, false);
>  	else
>  		rmap_walk_file(folio, rwc, false);
> +
> +	delta_ns = local_clock() - ts_start;
> +	trace_rmap_walk(folio, rwc, delta_ns, false);
>  }
> 
>  /* Like rmap_walk, but caller holds relevant rmap lock */
>  void rmap_walk_locked(struct folio *folio, struct rmap_walk_control *rwc)
>  {
> +	u64 ts_start, delta_ns;
> +	ts_start = local_clock();
> +
>  	/* no ksm support for now */
>  	VM_BUG_ON_FOLIO(folio_test_ksm(folio), folio);
>  	if (folio_test_anon(folio))
>  		rmap_walk_anon(folio, rwc, true);
>  	else
>  		rmap_walk_file(folio, rwc, true);
> +
> +	delta_ns = local_clock() - ts_start;
> +	trace_rmap_walk(folio, rwc, delta_ns, true);
>  }
> 
>  #ifdef CONFIG_HUGETLB_PAGE

Doing the clock stuff manually is nasty.

We could have a trace_rmap_walk_start() + trace_rmap_walk_stop().

But I wonder right now, whether we really need that information in the tree?

For local debugging (what you did) having this OOT patch is probably fine.

-- 
Cheers,

David