Re: [PATCH 2/2] mm/damon: add trace event for effective size quota

[SeongJae Park]

Hi Steven,

On Wed, 9 Jul 2025 12:21:23 -0400 Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> On Fri,  4 Jul 2025 15:14:08 -0700
> SeongJae Park <sj@xxxxxxxxxx> 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 <sj@xxxxxxxxxx>
> > ---
> >  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 ======================