Hi Steven,
On Wed, 9 Jul 2025 12:21:23 -0400 Steven Rostedt <rost...@goodmis.org> wrote:
> On Fri, 4 Jul 2025 15:14:08 -0700
> SeongJae Park <s...@kernel.org> wrote:
>
> > Aim-oriented DAMOS quota auto-tuning is an important and recommended
> > feature for DAMOS users. Add a trace event for the observability of the
> > tuned quota and tuning itself.
> >
> > Signed-off-by: SeongJae Park <s...@kernel.org>
> > ---
> > include/trace/events/damon.h | 26 ++++++++++++++++++++++++++
> > mm/damon/core.c | 20 +++++++++++++++++++-
> > 2 files changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/trace/events/damon.h b/include/trace/events/damon.h
> > index 32c611076023..36b2cdf47dce 100644
> > --- a/include/trace/events/damon.h
> > +++ b/include/trace/events/damon.h
> > @@ -9,6 +9,32 @@
> > #include <linux/types.h>
> > #include <linux/tracepoint.h>
> >
> > +TRACE_EVENT_CONDITION(damos_esz,
> > +
> > + TP_PROTO(unsigned int context_idx, unsigned int scheme_idx,
> > + unsigned long esz, bool do_trace),
> > +
> > + TP_ARGS(context_idx, scheme_idx, esz, do_trace),
> > +
> > + TP_CONDITION(do_trace),
>
> Please explain to me why you are using a conditional here?
There is no reason to make this conditional. Maybe I was out of my mind.
[...]
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2011,12 +2011,26 @@ static void damos_set_effective_quota(struct
> > damos_quota *quota)
> > quota->esz = esz;
> > }
> >
> > +static void damos_trace_esz(struct damon_ctx *c, struct damos *s,
> > + struct damos_quota *quota)
> > +{
> > + unsigned int cidx = 0, sidx;
> > + struct damos *siter;
> > +
> > + damon_for_each_scheme(siter, c) {
> > + if (siter == s)
> > + break;
> > + sidx++;
> > + }
> > + trace_damos_esz(cidx, sidx, quota->esz, true);
>
> It's set to true, so it's not even a conditional anymore. The compiler
> will likely optimize it out!
You're right.
Andrew, could you please add below fixup patch?
Thanks,
SJ
=================== >8 ======================
>From 13975534cad603410837c5eca299f0a24ad802a1 Mon Sep 17 00:00:00 2001
From: SeongJae Park <s...@kernel.org>
Date: Wed, 9 Jul 2025 11:22:39 -0700
Subject: [PATCH] mm/damon: make damos_esz unconditional trace event
It has no reason to be conditional. Make it unconditional trace event.
Fixes: 9a365a71b830 ("mm/damon: add trace event for effective size quota") #
mm-unstable
Closes: https://lore.kernel.org/20250709122123.779c8...@batman.local.home
Reported-by: Steven Rostedt <rost...@goodmis.org>
Signed-off-by: SeongJae Park <s...@kernel.org>
---
include/trace/events/damon.h | 8 +++-----
mm/damon/core.c | 2 +-
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/include/trace/events/damon.h b/include/trace/events/damon.h
index 2e4d7d281fa7..fc2c61ccc887 100644
--- a/include/trace/events/damon.h
+++ b/include/trace/events/damon.h
@@ -9,14 +9,12 @@
#include <linux/types.h>
#include <linux/tracepoint.h>
-TRACE_EVENT_CONDITION(damos_esz,
+TRACE_EVENT(damos_esz,
TP_PROTO(unsigned int context_idx, unsigned int scheme_idx,
- unsigned long esz, bool do_trace),
+ unsigned long esz),
- TP_ARGS(context_idx, scheme_idx, esz, do_trace),
-
- TP_CONDITION(do_trace),
+ TP_ARGS(context_idx, scheme_idx, esz),
TP_STRUCT__entry(
__field(unsigned int, context_idx)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 0f7d8b37c91d..775e097b6538 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2223,7 +2223,7 @@ static void damos_trace_esz(struct damon_ctx *c, struct
damos *s,
break;
sidx++;
}
- trace_damos_esz(cidx, sidx, quota->esz, true);
+ trace_damos_esz(cidx, sidx, quota->esz);
}
static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
--
2.39.5
