diff mbox series

[2/2,v3] memcg: add flush tracepoint

Message ID 20241029021106.25587-3-inwardvessel@gmail.com (mailing list archive)
State New
Headers show
Series memcg: tracepoint for flushing stats | expand

Commit Message

JP Kobryn Oct. 29, 2024, 2:11 a.m. UTC
This tracepoint gives visibility on how often the flushing of memcg stats
occurs and contains info on whether it was forced, skipped, and the value of
stats updated. It can help with understanding how readers are affected by
having to perform the flush, and the effectiveness of the flush by inspecting
the number of stats updated. Paired with the recently added tracepoints for
tracing rstat updates, it can also help show correlation where stats exceed
thresholds frequently.

Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
---
 include/trace/events/memcg.h | 25 +++++++++++++++++++++++++
 mm/memcontrol.c              |  7 ++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

Comments

Yosry Ahmed Oct. 29, 2024, 2:44 a.m. UTC | #1
On Mon, Oct 28, 2024 at 7:11 PM JP Kobryn <inwardvessel@gmail.com> wrote:
>
> This tracepoint gives visibility on how often the flushing of memcg stats
> occurs and contains info on whether it was forced, skipped, and the value of
> stats updated. It can help with understanding how readers are affected by
> having to perform the flush, and the effectiveness of the flush by inspecting
> the number of stats updated. Paired with the recently added tracepoints for
> tracing rstat updates, it can also help show correlation where stats exceed
> thresholds frequently.
>
> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>

Reviewed-by: Yosry Ahmed <yosryahmed@google.com>

> ---
>  include/trace/events/memcg.h | 25 +++++++++++++++++++++++++
>  mm/memcontrol.c              |  7 ++++++-
>  2 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/include/trace/events/memcg.h b/include/trace/events/memcg.h
> index 8667e57816d2..dfe2f51019b4 100644
> --- a/include/trace/events/memcg.h
> +++ b/include/trace/events/memcg.h
> @@ -74,6 +74,31 @@ DEFINE_EVENT(memcg_rstat_events, count_memcg_events,
>         TP_ARGS(memcg, item, val)
>  );
>
> +TRACE_EVENT(memcg_flush_stats,
> +
> +       TP_PROTO(struct mem_cgroup *memcg, s64 stats_updates,
> +               bool force, bool needs_flush),
> +
> +       TP_ARGS(memcg, stats_updates, force, needs_flush),
> +
> +       TP_STRUCT__entry(
> +               __field(u64, id)
> +               __field(s64, stats_updates)
> +               __field(bool, force)
> +               __field(bool, needs_flush)
> +       ),
> +
> +       TP_fast_assign(
> +               __entry->id = cgroup_id(memcg->css.cgroup);
> +               __entry->stats_updates = stats_updates;
> +               __entry->force = force;
> +               __entry->needs_flush = needs_flush;
> +       ),
> +
> +       TP_printk("memcg_id=%llu stats_updates=%lld force=%d needs_flush=%d",
> +               __entry->id, __entry->stats_updates,
> +               __entry->force, __entry->needs_flush)
> +);
>
>  #endif /* _TRACE_MEMCG_H */
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 59f6f247fc13..c3d6163aaa1c 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -590,7 +590,12 @@ static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val)
>
>  static void __mem_cgroup_flush_stats(struct mem_cgroup *memcg, bool force)
>  {
> -       if (!force && !memcg_vmstats_needs_flush(memcg->vmstats))
> +       bool needs_flush = memcg_vmstats_needs_flush(memcg->vmstats);
> +
> +       trace_memcg_flush_stats(memcg, atomic64_read(&memcg->vmstats->stats_updates),
> +               force, needs_flush);
> +
> +       if (!force && !needs_flush)
>                 return;
>
>         if (mem_cgroup_is_root(memcg))
> --
> 2.47.0
>
Shakeel Butt Oct. 29, 2024, 5:27 a.m. UTC | #2
On Mon, Oct 28, 2024 at 07:11:06PM GMT, JP Kobryn wrote:
> This tracepoint gives visibility on how often the flushing of memcg stats
> occurs and contains info on whether it was forced, skipped, and the value of
> stats updated. It can help with understanding how readers are affected by
> having to perform the flush, and the effectiveness of the flush by inspecting
> the number of stats updated. Paired with the recently added tracepoints for
> tracing rstat updates, it can also help show correlation where stats exceed
> thresholds frequently.
> 
> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
diff mbox series

Patch

diff --git a/include/trace/events/memcg.h b/include/trace/events/memcg.h
index 8667e57816d2..dfe2f51019b4 100644
--- a/include/trace/events/memcg.h
+++ b/include/trace/events/memcg.h
@@ -74,6 +74,31 @@  DEFINE_EVENT(memcg_rstat_events, count_memcg_events,
 	TP_ARGS(memcg, item, val)
 );
 
+TRACE_EVENT(memcg_flush_stats,
+
+	TP_PROTO(struct mem_cgroup *memcg, s64 stats_updates,
+		bool force, bool needs_flush),
+
+	TP_ARGS(memcg, stats_updates, force, needs_flush),
+
+	TP_STRUCT__entry(
+		__field(u64, id)
+		__field(s64, stats_updates)
+		__field(bool, force)
+		__field(bool, needs_flush)
+	),
+
+	TP_fast_assign(
+		__entry->id = cgroup_id(memcg->css.cgroup);
+		__entry->stats_updates = stats_updates;
+		__entry->force = force;
+		__entry->needs_flush = needs_flush;
+	),
+
+	TP_printk("memcg_id=%llu stats_updates=%lld force=%d needs_flush=%d",
+		__entry->id, __entry->stats_updates,
+		__entry->force, __entry->needs_flush)
+);
 
 #endif /* _TRACE_MEMCG_H */
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 59f6f247fc13..c3d6163aaa1c 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -590,7 +590,12 @@  static inline void memcg_rstat_updated(struct mem_cgroup *memcg, int val)
 
 static void __mem_cgroup_flush_stats(struct mem_cgroup *memcg, bool force)
 {
-	if (!force && !memcg_vmstats_needs_flush(memcg->vmstats))
+	bool needs_flush = memcg_vmstats_needs_flush(memcg->vmstats);
+
+	trace_memcg_flush_stats(memcg, atomic64_read(&memcg->vmstats->stats_updates),
+		force, needs_flush);
+
+	if (!force && !needs_flush)
 		return;
 
 	if (mem_cgroup_is_root(memcg))