Re: [PATCH RFC v2 7/7] mm/vmscan: pass scan_control to mm_vmscan_balance_pgdat_end

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: BPF raw tracepoint ABI breakage and commit message mismatch in mm_vmscan_balance_pgdat_end
--

commit eb8bc6091aab714af4b68f316fecfc08d43c39dc
Author: Ridong Chen <chenridong@xiaomi.com>

mm/vmscan: pass scan_control to mm_vmscan_balance_pgdat_end

This commit updates the mm_vmscan_balance_pgdat_end tracepoint to receive
a struct scan_control pointer rather than passing individual struct fields.
It updates the TP_PROTO and TP_ARGS while leaving the exported event format
unchanged.

>     balance_pgdat() passes sc.order and sc.nr_reclaimed to
>     mm_vmscan_balance_pgdat_end as bare arguments. Pass the enclosing
>     struct scan_control instead and read the fields out in TP_fast_assign,
>     mirroring mm_vmscan_balance_pgdat_begin. highest_zoneidx stays a

[Severity: Medium]
The commit message states this change is "mirroring
mm_vmscan_balance_pgdat_begin." Is this description accurate?

Looking at mm_vmscan_balance_pgdat_begin in mm/trace_vmscan.h, it still takes
individual arguments and was not updated to take struct scan_control *sc:

TRACE_EVENT(mm_vmscan_balance_pgdat_begin,
        TP_PROTO(int nid, int order, int highest_zoneidx),

Could the commit message be more precise about the rationale?

> diff --git a/mm/trace_vmscan.h b/mm/trace_vmscan.h
> index 04f285e776596..c27bc7cc60630 100644
> --- a/mm/trace_vmscan.h
> +++ b/mm/trace_vmscan.h
[ ... ]
> @@ -124,10 +124,9 @@ TRACE_EVENT(mm_vmscan_balance_pgdat_begin,
>  
>  TRACE_EVENT(mm_vmscan_balance_pgdat_end,
>  
> -	TP_PROTO(int nid, int order, int highest_zoneidx,
> -		 unsigned long nr_reclaimed),
> +	TP_PROTO(int nid, struct scan_control *sc, int highest_zoneidx),
>  
> -	TP_ARGS(nid, order, highest_zoneidx, nr_reclaimed),
> +	TP_ARGS(nid, sc, highest_zoneidx),

[Severity: Medium]
The commit message mentions there is "No functional change", but could this
modification break existing BPF programs hooked to this tracepoint?

While standard trace events parse the formatted output, BPF raw tracepoints
consume the TP_PROTO arguments directly via ctx->args. Changing the number
and type of arguments alters the BPF raw tracepoint ABI for this event.

Does this inadvertently break user space tools that attach to this raw
tracepoint and expect the 4 original arguments?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928114625.3609130-1-ridong.chen@linux.dev?part=7