On Wed, 13 May 2026 14:07:32 -0400 Steven Rostedt <[email protected]> wrote:
> On Tue, 12 May 2026 07:36:33 -0700 > SeongJae Park <[email protected]> wrote: > > > Introduce a new tracepoint for exposing the per-region per-probe > > positive sample count via tracefs. > > > > Signed-off-by: SeongJae Park <[email protected]> > > --- > > include/trace/events/damon.h | 36 ++++++++++++++++++++++++++++++++++++ > > mm/damon/core.c | 7 +++++++ > > 2 files changed, 43 insertions(+) > > > > diff --git a/include/trace/events/damon.h b/include/trace/events/damon.h > > index 7e25f4469b81b..d7b94c7640217 100644 > > --- a/include/trace/events/damon.h > > +++ b/include/trace/events/damon.h > > @@ -130,6 +130,42 @@ TRACE_EVENT(damon_monitor_intervals_tune, > > TP_printk("sample_us=%lu", __entry->sample_us) > > ); > > > > +TRACE_EVENT(damon_aggregated_v2, > > + > > + TP_PROTO(unsigned int target_id, struct damon_region *r, > > + unsigned int nr_regions, unsigned int nr_probes), > > + > > + TP_ARGS(target_id, r, nr_regions, nr_probes), > > + > > + TP_STRUCT__entry( > > + __field(unsigned long, target_id) > > + __field(unsigned long, start) > > + __field(unsigned long, end) > > + __field(unsigned int, nr_regions) > > + __field(unsigned int, nr_accesses) > > + __field(unsigned int, age) > > + __dynamic_array(unsigned char, probe_hits, nr_probes) > > + ), > > + > > + TP_fast_assign( > > + __entry->target_id = target_id; > > + __entry->start = r->ar.start; > > + __entry->end = r->ar.end; > > + __entry->nr_regions = nr_regions; > > + __entry->nr_accesses = r->nr_accesses; > > + __entry->age = r->age; > > + memcpy(__get_dynamic_array(probe_hits), r->probe_hits, > > + sizeof(*r->probe_hits) * nr_probes); > > + ), > > + > > + TP_printk("target_id=%lu nr_regions=%u %lu-%lu: %u %u probe_hits=%s", > > + __entry->target_id, __entry->nr_regions, > > + __entry->start, __entry->end, > > + __entry->nr_accesses, __entry->age, > > + __print_hex(__get_dynamic_array(probe_hits), > > + __get_dynamic_array_len(probe_hits))) > > +); > > + > > TRACE_EVENT(damon_aggregated, > > > > TP_PROTO(unsigned int target_id, struct damon_region *r, > > diff --git a/mm/damon/core.c b/mm/damon/core.c > > index fe6c789f2cecb..14b15c9876516 100644 > > --- a/mm/damon/core.c > > +++ b/mm/damon/core.c > > @@ -1905,6 +1905,11 @@ static void kdamond_reset_aggregated(struct > > damon_ctx *c) > > { > > struct damon_target *t; > > unsigned int ti = 0; /* target's index */ > > + unsigned int nr_probes = 0; > > + struct damon_probe *probe; > > + > > + damon_for_each_probe(probe, c) > > + nr_probes++; > > Is the above logic needed when the tracepoint isn't enabled? If not, then you > could add: > > if (trace_damon_aggregated_v2_enabled()) { > damon_for_each_probe(probe, c) > nr_probes++; > } > > And change the tracepoint to be a conditional tracepoint: > > TRACE_EVENT_CONDITION(damon_aggregated_v2, > > TP_PROTO(..), > > TP_ARGS(..), > > TP_CONDITION(nr_probes > 0), > > [..] > > And then the tracepoint is only triggered if nr_probes is greater than zero > (to handle races between the tracepoint being enabled in between the above > check and where it triggers). It is not needed when the tracepoint isn't enabled. I will follow your suggestion in the next revision. Thank you for the nice suggestion, Steven! Btw, if you don't mind, may I ask your opinion about the name having '_v2' suffix? I chose that as an RFC phase temporal name that doesn't break the compatibility, planning to give it a better name later. But I start feeling just extending the original one might be another option because tracepoints are not strict stable ABI to my understanding, and the change of the TP_prink format should be simple enough (append the probe_hits= part) that the user space could reasonably deal with. Thanks, SJ [...]
