Re: [PATCH v2] trace/trace_event_perf: remove duplicate samples on the first tracepoint event

Hello,

On Tue, Oct 01, 2024 at 08:48:45AM +0000, Yeo Reum Yun wrote:

Gentle ping in case of forgotten.

________________________________________
From: Steven Rostedt <rostedt@goodmis.org>
Sent: 26 September 2024 13:51
To: Yeo Reum Yun
Cc: mhiramat@kernel.org; mathieu.desnoyers@efficios.com; a.p.zijlstra@chello.nl; mingo@elte.hu; Mark Rutland; james.clark@linaro.org; nd; linux-kernel@vger.kernel.org; linux-trace-kernel@vger.kernel.org; Peter Zijlstra; Frederic Weisbecker; Arnaldo Carvalho de Melo; Namhyung Kim
Subject: Re: [PATCH v2] trace/trace_event_perf: remove duplicate samples on the first tracepoint event

Can one of the perf folks give me an Ack, and I'll take this through my tree.

I'm not sure if we want to support freq mode or period other than 1 for
tracepoint events.  But it looks broken apparently, so

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

On Fri, 13 Sep 2024 03:13:47 +0100
Levi Yun [off-list ref] wrote:

quoted

When a tracepoint event is created with attr.freq = 1,
'hwc->period_left' is not initialized correctly. As a result,
in the perf_swevent_overflow() function, when the first time the event occurs,
it calculates the event overflow and the perf_swevent_set_period() returns 3,
this leads to the event are recorded for three duplicate times.

Step to reproduce:
    1. Enable the tracepoint event & starting tracing
         $ echo 1 > /sys/kernel/tracing/events/module/module_free
         $ echo 1 > /sys/kernel/tracing/tracing_on

    2. Record with perf
         $ perf record -a --strict-freq -F 1 -e "module:module_free"

    3. Trigger module_free event.
         $ modprobe -i sunrpc
         $ modprobe -r sunrpc

Result:
     - Trace pipe result:
         $ cat trace_pipe
         modprobe-174509  [003] .....  6504.868896: module_free: sunrpc

     - perf sample:
         modprobe  174509 [003]  6504.868980: module:module_free: sunrpc
         modprobe  174509 [003]  6504.868980: module:module_free: sunrpc
         modprobe  174509 [003]  6504.868980: module:module_free: sunrpc

By setting period_left via perf_swevent_set_period() as other sw_event did,
This problem could be solved.

After patch:
     - Trace pipe result:
         $ cat trace_pipe
         modprobe 1153096 [068] 613468.867774: module:module_free: xfs

     - perf sample
         modprobe 1153096 [068] 613468.867794: module:module_free: xfs

Fixes: bd2b5b12849a ("perf_counter: More aggressive frequency adjustment")
Signed-off-by: Levi Yun <redacted>
---
Changes in v2:
  - Fix build error.
---
 kernel/trace/trace_event_perf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 05e791241812..3ff9caa4a71b 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c

@@ -352,10 +352,16 @@ void perf_uprobe_destroy(struct perf_event *p_event)
 int perf_trace_add(struct perf_event *p_event, int flags)
 {
      struct trace_event_call *tp_event = p_event->tp_event;
+     struct hw_perf_event *hwc = &p_event->hw;

      if (!(flags & PERF_EF_START))
              p_event->hw.state = PERF_HES_STOPPED;

+     if (is_sampling_event(p_event)) {
+             hwc->last_period = hwc->sample_period;
+             perf_swevent_set_period(p_event);
+     }
+
      /*
       * If TRACE_REG_PERF_ADD returns false; no custom action was performed
       * and we need to take the default action of enqueueing our event on
--

LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}