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

[Ridong Chen]

On 9/28/2026 7:58 PM, sashiko-...@kernel.org wrote:
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 <chenrid...@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?


You're right, that wording is misleading and I'll fix it in next version.

I just mixed it up with other xxx_begin tracepoints.

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?


Thanks, "No functional change" is inaccurate here.

Kernel behaviour and the tracefs format are unchanged (TP_STRUCT__entry /
TP_printk are untouched), so perf / trace-cmd / libbpf CO-RE consumers are
unaffected. But for BPF raw tracepoints the arg layout does change: a program
reading the old args[3] (nr_reclaimed) now has to read it out of sc. I
shouldn't have hidden that under "no functional change".

Raw tracepoints aren't a stable ABI though 
(Documentation/bpf/bpf_design_QA.rst), so the change itself is fine — I'll just 
fix the commit message in next version.

--
Best regards
Ridong