Re: [PATCH v3] mm: vmscan: rework lru_shrink and write_folio tracepoints

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The `mm_vmscan_lru_shrink_inactive` tracepoint prints `nr_ref_keep` twice and omits the newly added `nr_ref_activate` field.
- [Medium] Missing TRACE_DEFINE_ENUM exports for PGSTEAL_* enum values used in trace_reclaim_reason_ops.
- [Medium] The patch breaks the in-tree `trace-vmscan-postprocess.pl` script by altering tracepoint arguments without updating the script.
--

commit 415a9b2c51b867b9381070883253c78fbde68aee
Author: qiwu.chen <qiwu.chen@transsion.com>

mm: vmscan: rework lru_shrink and write_folio tracepoints

This commit reworks the lru_shrink and write_folio tracepoints for better
correlation and analysis. It traces each folio LRU type and reclaim reason,
adds activation statistics, and removes some unused reclaim flags.

> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index 8a872990b4bee..db6c6d2d777af 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
[ ... ]
> -#define trace_reclaim_flags(file) ( \
> -	(file ? RECLAIM_WB_FILE : RECLAIM_WB_ANON) | \
> -	(RECLAIM_WB_ASYNC) \
> -	)
> +#define trace_reclaim_reason_ops		\
> +	{PGSTEAL_KSWAPD,	"KSWAPD"},	\
> +	{PGSTEAL_DIRECT,	"DIRECT"},	\
> +	{PGSTEAL_KHUGEPAGED,	"KHUGEPAGED"}, \
> +	{PGSTEAL_PROACTIVE,	"PROACTIVE"}

[Severity: Medium]
Since this macro maps enums to strings for use with __print_symbolic(), do
these enum values need corresponding TRACE_DEFINE_ENUM() exports?

Without them, userspace tools like perf or trace-cmd will not be able to
evaluate the enum values from the trace format file and will fail to parse
the reason field.

[ ... ]
> @@ -495,29 +480,32 @@ TRACE_EVENT(mm_vmscan_lru_shrink_inactive,
>  		__entry->nr_activate0 = stat->nr_activate[0];
>  		__entry->nr_activate1 = stat->nr_activate[1];
>  		__entry->nr_ref_keep = stat->nr_ref_keep;
> +		__entry->nr_ref_activate = stat->nr_ref_activate;
>  		__entry->nr_unmap_fail = stat->nr_unmap_fail;
>  		__entry->priority = priority;
> -		__entry->reclaim_flags = trace_reclaim_flags(file);
> +		__entry->lru = lru;
> +		__entry->reason = reason;
>  	),
>  
> -	TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d nr_activate_file=%d nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
> +	TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d nr_activate_file=%d nr_ref_keep=%ld nr_ref_activate=%ld nr_unmap_fail=%ld priority=%d lru=%s reason=%s",
>  		__entry->nid,
>  		__entry->nr_scanned, __entry->nr_reclaimed,
>  		__entry->nr_dirty, __entry->nr_writeback,
>  		__entry->nr_congested, __entry->nr_immediate,
>  		__entry->nr_activate0, __entry->nr_activate1,
> -		__entry->nr_ref_keep, __entry->nr_unmap_fail,
> -		__entry->priority,
> -		show_reclaim_flags(__entry->reclaim_flags))
> +		__entry->nr_ref_keep, __entry->nr_ref_keep,

[Severity: Medium]
Is the second __entry->nr_ref_keep here a typo?

It looks like it should be __entry->nr_ref_activate to match the format
string. The duplicated nr_ref_keep argument causes the correct
nr_ref_activate field to be ignored in the trace output.

[Severity: Medium]
Does the Documentation/trace/postprocess/trace-vmscan-postprocess.pl
script also need to be updated to handle this new format?

The script contains a hardcoded regular expression for this tracepoint:

$regex_lru_shrink_inactive = generate_traceevent_regex(
		"vmscan/mm_vmscan_lru_shrink_inactive",
		$regex_lru_shrink_inactive_default,
		"nid", "nr_scanned", "nr_reclaimed", "nr_dirty", "nr_writeback",
		"nr_congested", "nr_immediate", "nr_activate_anon",
		"nr_activate_file", "nr_ref_keep",
		"nr_unmap_fail", "priority", "flags");

Since flags was renamed to lru, and reason and nr_ref_activate were
added, the parsing logic in the script will fail on traces using this new
format.

> +		__entry->nr_unmap_fail, __entry->priority,
> +		__print_symbolic(__entry->lru, LRU_NAMES),
> +		__print_symbolic(__entry->reason, trace_reclaim_reason_ops))
>  );

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914072433.39636-1-qiwu.chen@transsion.com?part=1