Re: [PATCH] memcg: add tracing for memcg stat updates

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 9 Oct 2024 17:46:22 -0700
Yosry Ahmed <yosryahmed@google.com> wrote:

> > +++ b/mm/memcontrol.c
> > @@ -71,6 +71,10 @@
> >
> >  #include <linux/uaccess.h>
> >
> > +#define CREATE_TRACE_POINTS
> > +#include <trace/events/memcg.h>
> > +#undef CREATE_TRACE_POINTS
> > +
> >  #include <trace/events/vmscan.h>
> >
> >  struct cgroup_subsys memory_cgrp_subsys __read_mostly;
> > @@ -682,7 +686,9 @@ void __mod_memcg_state(struct mem_cgroup *memcg, enum memcg_stat_item idx,
> >                 return;
> >
> >         __this_cpu_add(memcg->vmstats_percpu->state[i], val);
> > -       memcg_rstat_updated(memcg, memcg_state_val_in_pages(idx, val));
> > +       val = memcg_state_val_in_pages(idx, val);
> > +       memcg_rstat_updated(memcg, val);
> > +       trace_mod_memcg_state(memcg, idx, val);  
> 
> Is it too unreasonable to include the stat name?
> 
> The index has to be correlated with the kernel config and perhaps even
> version. It's not a big deal, but if performance is not a concern when
> tracing is enabled anyway, maybe we can lookup the name here (or in
> TP_fast_assign()).

What name? Is it looked up from idx? If so, you can do it on the reading of
the trace event where performance is not an issue. See the __print_symbolic()
and friends in samples/trace_events/trace-events-sample.h

-- Steve