On Wed, 9 Oct 2024 17:46:22 -0700 Yosry Ahmed <yosryahmed@xxxxxxxxxx> 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
