[PATCH 12/12] mm/damon/core: add trace point for damos stat per apply interval

SeongJae Park posted 12 patches 1 month, 3 weeks ago
[PATCH 12/12] mm/damon/core: add trace point for damos stat per apply interval
Posted by SeongJae Park 1 month, 3 weeks ago
DAMON users can read DAMOS stats via DAMON sysfs interface.  It enables
efficient, simple and flexible usages of the stats.  Especially for
systems not having advanced tools like perf or bpftrace, that can be
useful.  But if the advanced tools are available, exposing the stats via
tracepoint can reduce unnecessary reimplementation of the wheels.  Add a
new tracepoint for DAMOS stats, namely damos_stat_after_apply_interval.
The tracepoint is triggered for each scheme's apply interval and exposes
the whole stat values.  If the user needs sub-apply interval information
for any chance, damos_before_apply tracepoint could be used.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/trace/events/damon.h | 41 ++++++++++++++++++++++++++++++++++++
 mm/damon/core.c              | 17 +++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/include/trace/events/damon.h b/include/trace/events/damon.h
index 852d725afea2..24fc402ab3c8 100644
--- a/include/trace/events/damon.h
+++ b/include/trace/events/damon.h
@@ -9,6 +9,47 @@
 #include <linux/types.h>
 #include <linux/tracepoint.h>
 
+TRACE_EVENT(damos_stat_after_apply_interval,
+
+	TP_PROTO(unsigned int context_idx, unsigned int scheme_idx,
+		struct damos_stat *stat),
+
+	TP_ARGS(context_idx, scheme_idx, stat),
+
+	TP_STRUCT__entry(
+		__field(unsigned int, context_idx)
+		__field(unsigned int, scheme_idx)
+		__field(unsigned long, nr_tried)
+		__field(unsigned long, sz_tried)
+		__field(unsigned long, nr_applied)
+		__field(unsigned long, sz_applied)
+		__field(unsigned long, sz_ops_filter_passed)
+		__field(unsigned long, qt_exceeds)
+		__field(unsigned long, nr_snapshots)
+	),
+
+	TP_fast_assign(
+		__entry->context_idx = context_idx;
+		__entry->scheme_idx = scheme_idx;
+		__entry->nr_tried = stat->nr_tried;
+		__entry->sz_tried = stat->sz_tried;
+		__entry->nr_applied = stat->nr_applied;
+		__entry->sz_applied = stat->sz_applied;
+		__entry->sz_ops_filter_passed = stat->sz_ops_filter_passed;
+		__entry->qt_exceeds = stat->qt_exceeds;
+		__entry->nr_snapshots = stat->nr_snapshots;
+	),
+
+	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
+			"nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
+			"qt_exceeds=%lu nr_snapshots=%lu",
+			__entry->context_idx, __entry->scheme_idx,
+			__entry->nr_tried, __entry->sz_tried,
+			__entry->nr_applied, __entry->sz_applied,
+			__entry->sz_ops_filter_passed, __entry->qt_exceeds,
+			__entry->nr_snapshots)
+);
+
 TRACE_EVENT(damos_esz,
 
 	TP_PROTO(unsigned int context_idx, unsigned int scheme_idx,
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 8908aec6670f..68dd2f7acba2 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2256,6 +2256,22 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 	quota->min_score = score;
 }
 
+static void damos_trace_stat(struct damon_ctx *c, struct damos *s)
+{
+	unsigned int cidx = 0, sidx = 0;
+	struct damos *siter;
+
+	if (!trace_damos_stat_after_apply_interval_enabled())
+		return;
+
+	damon_for_each_scheme(siter, c) {
+		if (siter == s)
+			break;
+		sidx++;
+	}
+	trace_damos_stat_after_apply_interval(cidx, sidx, &s->stat);
+}
+
 static void kdamond_apply_schemes(struct damon_ctx *c)
 {
 	struct damon_target *t;
@@ -2297,6 +2313,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
 			(s->apply_interval_us ? s->apply_interval_us :
 			 c->attrs.aggr_interval) / sample_interval;
 		s->last_applied = NULL;
+		damos_trace_stat(c, s);
 	}
 	mutex_unlock(&c->walk_control_lock);
 }
-- 
2.47.3
Re: [PATCH 12/12] mm/damon/core: add trace point for damos stat per apply interval
Posted by Steven Rostedt 1 month, 3 weeks ago
On Tue, 16 Dec 2025 00:01:25 -0800
SeongJae Park <sj@kernel.org> wrote:

> +	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
> +			"nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
> +			"qt_exceeds=%lu nr_snapshots=%lu",

Nit, but it's been stated that strings should not be broken up because of
the column limit.

> +			__entry->context_idx, __entry->scheme_idx,
> +			__entry->nr_tried, __entry->sz_tried,
> +			__entry->nr_applied, __entry->sz_applied,
> +			__entry->sz_ops_filter_passed, __entry->qt_exceeds,
> +			__entry->nr_snapshots)
> +);
> +
>  TRACE_EVENT(damos_esz,
>  
>  	TP_PROTO(unsigned int context_idx, unsigned int scheme_idx,
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 8908aec6670f..68dd2f7acba2 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2256,6 +2256,22 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
>  	quota->min_score = score;
>  }
>  
> +static void damos_trace_stat(struct damon_ctx *c, struct damos *s)
> +{
> +	unsigned int cidx = 0, sidx = 0;
> +	struct damos *siter;
> +
> +	if (!trace_damos_stat_after_apply_interval_enabled())
> +		return;

Other than that, from a tracing POV:

Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve

> +
> +	damon_for_each_scheme(siter, c) {
> +		if (siter == s)
> +			break;
> +		sidx++;
> +	}
> +	trace_damos_stat_after_apply_interval(cidx, sidx, &s->stat);
> +}
> +
>  static void kdamond_apply_schemes(struct damon_ctx *c)
>  {
>  	struct damon_target *t;
> @@ -2297,6 +2313,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
>  			(s->apply_interval_us ? s->apply_interval_us :
>  			 c->attrs.aggr_interval) / sample_interval;
>  		s->last_applied = NULL;
> +		damos_trace_stat(c, s);
>  	}
>  	mutex_unlock(&c->walk_control_lock);
>  }
Re: [PATCH 12/12] mm/damon/core: add trace point for damos stat per apply interval
Posted by SeongJae Park 1 month, 3 weeks ago
On Wed, 17 Dec 2025 17:48:51 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 16 Dec 2025 00:01:25 -0800
> SeongJae Park <sj@kernel.org> wrote:
> 
> > +	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
> > +			"nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
> > +			"qt_exceeds=%lu nr_snapshots=%lu",
> 
> Nit, but it's been stated that strings should not be broken up because of
> the column limit.

I also found checkpatch.pl shows warning for that.  I was ignoring that since I
was thinking that's just a recommendation.  But as I got more than one warning,
now I'd like to fix this.

Andrew, could you please add below attaching fixup patch?

> 
> > +			__entry->context_idx, __entry->scheme_idx,
> > +			__entry->nr_tried, __entry->sz_tried,
> > +			__entry->nr_applied, __entry->sz_applied,
> > +			__entry->sz_ops_filter_passed, __entry->qt_exceeds,
> > +			__entry->nr_snapshots)
> > +);
> > +
> >  TRACE_EVENT(damos_esz,
> >  
> >  	TP_PROTO(unsigned int context_idx, unsigned int scheme_idx,
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 8908aec6670f..68dd2f7acba2 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2256,6 +2256,22 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
> >  	quota->min_score = score;
> >  }
> >  
> > +static void damos_trace_stat(struct damon_ctx *c, struct damos *s)
> > +{
> > +	unsigned int cidx = 0, sidx = 0;
> > +	struct damos *siter;
> > +
> > +	if (!trace_damos_stat_after_apply_interval_enabled())
> > +		return;
> 
> Other than that, from a tracing POV:
> 
> Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Thank you! :)


Thanks,
SJ

[...]

=== >8 ===
From 67a7762ee1ade258c53913ea4ecf9bafd4746ea9 Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj@kernel.org>
Date: Wed, 17 Dec 2025 15:42:06 -0800
Subject: [PATCH] mm/damon: do not break the string for damos_stat tracepoint

The format string for damos_stat tracepoint is broken up to multiple
lines.  Strings shouldn't be broken up due to the column limit, though.
Update the string to be put on a single line.

Suggested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/trace/events/damon.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/trace/events/damon.h b/include/trace/events/damon.h
index 24fc402ab3c8..bf25ee07cb48 100644
--- a/include/trace/events/damon.h
+++ b/include/trace/events/damon.h
@@ -40,9 +40,7 @@ TRACE_EVENT(damos_stat_after_apply_interval,
 		__entry->nr_snapshots = stat->nr_snapshots;
 	),
 
-	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
-			"nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
-			"qt_exceeds=%lu nr_snapshots=%lu",
+	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu qt_exceeds=%lu nr_snapshots=%lu",
 			__entry->context_idx, __entry->scheme_idx,
 			__entry->nr_tried, __entry->sz_tried,
 			__entry->nr_applied, __entry->sz_applied,
-- 
2.47.3
Re: [PATCH 12/12] mm/damon/core: add trace point for damos stat per apply interval
Posted by Andrew Morton 1 month, 3 weeks ago
On Wed, 17 Dec 2025 15:52:18 -0800 SeongJae Park <sj@kernel.org> wrote:

> On Wed, 17 Dec 2025 17:48:51 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Tue, 16 Dec 2025 00:01:25 -0800
> > SeongJae Park <sj@kernel.org> wrote:
> > 
> > > +	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
> > > +			"nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
> > > +			"qt_exceeds=%lu nr_snapshots=%lu",
> > 
> > Nit, but it's been stated that strings should not be broken up because of
> > the column limit.

screw the rules

> --- a/include/trace/events/damon.h
> +++ b/include/trace/events/damon.h
> @@ -40,9 +40,7 @@ TRACE_EVENT(damos_stat_after_apply_interval,
>  		__entry->nr_snapshots = stat->nr_snapshots;
>  	),
>  
> -	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
> -			"nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
> -			"qt_exceeds=%lu nr_snapshots=%lu",
> +	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu qt_exceeds=%lu nr_snapshots=%lu",
>  			__entry->context_idx, __entry->scheme_idx,
>  			__entry->nr_tried, __entry->sz_tried,
>  			__entry->nr_applied, __entry->sz_applied,

because that's just crazy.  Let's use some judgment here!
Re: [PATCH 12/12] mm/damon/core: add trace point for damos stat per apply interval
Posted by Steven Rostedt 1 month, 3 weeks ago
On Wed, 17 Dec 2025 18:29:15 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> > > Nit, but it's been stated that strings should not be broken up because of
> > > the column limit.  
> 
> screw the rules

Heh.

> 
> > --- a/include/trace/events/damon.h
> > +++ b/include/trace/events/damon.h
> > @@ -40,9 +40,7 @@ TRACE_EVENT(damos_stat_after_apply_interval,
> >  		__entry->nr_snapshots = stat->nr_snapshots;
> >  	),
> >  
> > -	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
> > -			"nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
> > -			"qt_exceeds=%lu nr_snapshots=%lu",
> > +	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu qt_exceeds=%lu nr_snapshots=%lu",
> >  			__entry->context_idx, __entry->scheme_idx,
> >  			__entry->nr_tried, __entry->sz_tried,
> >  			__entry->nr_applied, __entry->sz_applied,  
> 
> because that's just crazy.  Let's use some judgment here!

Does it really matter? Actually, I prefer this way because it better shows
where the format ends and the parameters start. I care more about the
parameters than the format string, except to look for each "%*" value.

-- Steve
Re: [PATCH 12/12] mm/damon/core: add trace point for damos stat per apply interval
Posted by SeongJae Park 1 month, 3 weeks ago
On Wed, 17 Dec 2025 18:29:15 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 17 Dec 2025 15:52:18 -0800 SeongJae Park <sj@kernel.org> wrote:
> 
> > On Wed, 17 Dec 2025 17:48:51 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > On Tue, 16 Dec 2025 00:01:25 -0800
> > > SeongJae Park <sj@kernel.org> wrote:
> > > 
> > > > +	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
> > > > +			"nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
> > > > +			"qt_exceeds=%lu nr_snapshots=%lu",
> > > 
> > > Nit, but it's been stated that strings should not be broken up because of
> > > the column limit.
> 
> screw the rules
> 
> > --- a/include/trace/events/damon.h
> > +++ b/include/trace/events/damon.h
> > @@ -40,9 +40,7 @@ TRACE_EVENT(damos_stat_after_apply_interval,
> >  		__entry->nr_snapshots = stat->nr_snapshots;
> >  	),
> >  
> > -	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu "
> > -			"nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu "
> > -			"qt_exceeds=%lu nr_snapshots=%lu",
> > +	TP_printk("ctx_idx=%u scheme_idx=%u nr_tried=%lu sz_tried=%lu nr_applied=%lu sz_tried=%lu sz_ops_filter_passed=%lu qt_exceeds=%lu nr_snapshots=%lu",
> >  			__entry->context_idx, __entry->scheme_idx,
> >  			__entry->nr_tried, __entry->sz_tried,
> >  			__entry->nr_applied, __entry->sz_applied,
> 
> because that's just crazy.  Let's use some judgment here!

I'm fine with either direction.  So I understand you want to just keep the
original patch without this fixup, and therefore no action is needed from my
side?  Let me know if I'm getting anything wrong.


Thanks,
SJ

[...]