linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] perf ftrace latency: Do not read trace files when BPF is used
@ 2026-08-31  6:39 Namhyung Kim
  2026-08-31  6:47 ` sashiko-bot
  2026-09-04 17:36 ` Ian Rogers
  0 siblings, 2 replies; 4+ messages in thread
From: Namhyung Kim @ 2026-08-31  6:39 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users, Steven Rostedt,
	Masami Hiramatsu

I've realized that it didn't set up the tracing files when BPF is used
so poll() just returns immediately.  It ends up with calling poll()
unnecessarily in a loop.

BPF still needs the loop to wait for the target process exiting or a
signal from users.  Let's use a semaphore instead.

Suggested-by: Ian Rogers <irogers@google.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-ftrace.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 4f881a40c311ae52..6017493ff179758b 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -6,23 +6,23 @@
  * Copyright (c) 2020  Changbin Du <changbin.du@gmail.com>, significant enhancement.
  */
 
-#include "builtin.h"
-
+#include <ctype.h>
 #include <errno.h>
-#include <unistd.h>
-#include <signal.h>
-#include <stdlib.h>
 #include <fcntl.h>
 #include <inttypes.h>
 #include <math.h>
 #include <poll.h>
-#include <ctype.h>
+#include <semaphore.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
 #include <linux/capability.h>
 #include <linux/err.h>
 #include <linux/string.h>
 #include <linux/zalloc.h>
 #include <sys/stat.h>
 
+#include "builtin.h"
 #include "debug.h"
 #include <subcmd/pager.h>
 #include <subcmd/parse-options.h>
@@ -45,6 +45,7 @@
 
 static volatile sig_atomic_t workload_exec_errno;
 static volatile sig_atomic_t done;
+static sem_t sig_sem;
 
 static struct stats latency_stats;  /* for tracepoints */
 
@@ -53,6 +54,7 @@ static char tracing_instance[PATH_MAX];	/* Trace instance directory */
 static void sig_handler(int sig __maybe_unused)
 {
 	done = true;
+	sem_post(&sig_sem);
 }
 
 /*
@@ -68,6 +70,7 @@ static void ftrace__workload_exec_failed_signal(int signo __maybe_unused,
 {
 	workload_exec_errno = info->si_value.sival_int;
 	done = true;
+	sem_post(&sig_sem);
 }
 
 static bool check_ftrace_capable(void)
@@ -1146,6 +1149,11 @@ static int __cmd_latency(struct perf_ftrace *ftrace)
 
 	line[0] = '\0';
 	while (!done) {
+		if (ftrace->target.use_bpf) {
+			sem_wait(&sig_sem);
+			break;
+		}
+
 		if (poll(&pollfd, 1, -1) < 0)
 			break;
 
@@ -1852,6 +1860,8 @@ int cmd_ftrace(int argc, const char **argv)
 	INIT_LIST_HEAD(&ftrace.nograph_funcs);
 	INIT_LIST_HEAD(&ftrace.event_pair);
 
+	sem_init(&sig_sem, 0, 0);
+
 	signal(SIGINT, sig_handler);
 	signal(SIGUSR1, sig_handler);
 	signal(SIGCHLD, sig_handler);
@@ -2015,5 +2025,11 @@ int cmd_ftrace(int argc, const char **argv)
 	delete_filter_func(&ftrace.nograph_funcs);
 	delete_filter_func(&ftrace.event_pair);
 
+	signal(SIGINT, SIG_DFL);
+	signal(SIGUSR1, SIG_DFL);
+	signal(SIGCHLD, SIG_DFL);
+	signal(SIGPIPE, SIG_DFL);
+
+	sem_destroy(&sig_sem);
 	return ret;
 }
-- 
2.55.0.897.gb25b4bd76c-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] perf ftrace latency: Do not read trace files when BPF is used
  2026-08-31  6:39 [PATCH v2] perf ftrace latency: Do not read trace files when BPF is used Namhyung Kim
@ 2026-08-31  6:47 ` sashiko-bot
  2026-09-04 17:36 ` Ian Rogers
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-31  6:47 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-perf-users

> I've realized that it didn't set up the tracing files when BPF is used
> so poll() just returns immediately.  It ends up with calling poll()
> unnecessarily in a loop.
> 
> BPF still needs the loop to wait for the target process exiting or a
> signal from users.  Let's use a semaphore instead.
> 
> Suggested-by: Ian Rogers <irogers@google.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831063938.756610-1-namhyung@kernel.org?part=1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] perf ftrace latency: Do not read trace files when BPF is used
  2026-08-31  6:39 [PATCH v2] perf ftrace latency: Do not read trace files when BPF is used Namhyung Kim
  2026-08-31  6:47 ` sashiko-bot
@ 2026-09-04 17:36 ` Ian Rogers
  2026-09-08 21:04   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2026-09-04 17:36 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, James Clark,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users,
	Steven Rostedt, Masami Hiramatsu

On Sun, Aug 30, 2026 at 11:39 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> I've realized that it didn't set up the tracing files when BPF is used
> so poll() just returns immediately.  It ends up with calling poll()
> unnecessarily in a loop.
>
> BPF still needs the loop to wait for the target process exiting or a
> signal from users.  Let's use a semaphore instead.
>
> Suggested-by: Ian Rogers <irogers@google.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/builtin-ftrace.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 4f881a40c311ae52..6017493ff179758b 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -6,23 +6,23 @@
>   * Copyright (c) 2020  Changbin Du <changbin.du@gmail.com>, significant enhancement.
>   */
>
> -#include "builtin.h"
> -
> +#include <ctype.h>
>  #include <errno.h>
> -#include <unistd.h>
> -#include <signal.h>
> -#include <stdlib.h>
>  #include <fcntl.h>
>  #include <inttypes.h>
>  #include <math.h>
>  #include <poll.h>
> -#include <ctype.h>
> +#include <semaphore.h>
> +#include <signal.h>
> +#include <stdlib.h>
> +#include <unistd.h>
>  #include <linux/capability.h>
>  #include <linux/err.h>
>  #include <linux/string.h>
>  #include <linux/zalloc.h>
>  #include <sys/stat.h>
>
> +#include "builtin.h"
>  #include "debug.h"
>  #include <subcmd/pager.h>
>  #include <subcmd/parse-options.h>
> @@ -45,6 +45,7 @@
>
>  static volatile sig_atomic_t workload_exec_errno;
>  static volatile sig_atomic_t done;
> +static sem_t sig_sem;
>
>  static struct stats latency_stats;  /* for tracepoints */
>
> @@ -53,6 +54,7 @@ static char tracing_instance[PATH_MAX];       /* Trace instance directory */
>  static void sig_handler(int sig __maybe_unused)
>  {
>         done = true;
> +       sem_post(&sig_sem);
>  }
>
>  /*
> @@ -68,6 +70,7 @@ static void ftrace__workload_exec_failed_signal(int signo __maybe_unused,
>  {
>         workload_exec_errno = info->si_value.sival_int;
>         done = true;
> +       sem_post(&sig_sem);
>  }
>
>  static bool check_ftrace_capable(void)
> @@ -1146,6 +1149,11 @@ static int __cmd_latency(struct perf_ftrace *ftrace)
>
>         line[0] = '\0';
>         while (!done) {
> +               if (ftrace->target.use_bpf) {
> +                       sem_wait(&sig_sem);
> +                       break;
> +               }
> +
>                 if (poll(&pollfd, 1, -1) < 0)
>                         break;
>
> @@ -1852,6 +1860,8 @@ int cmd_ftrace(int argc, const char **argv)
>         INIT_LIST_HEAD(&ftrace.nograph_funcs);
>         INIT_LIST_HEAD(&ftrace.event_pair);
>
> +       sem_init(&sig_sem, 0, 0);
> +
>         signal(SIGINT, sig_handler);
>         signal(SIGUSR1, sig_handler);
>         signal(SIGCHLD, sig_handler);
> @@ -2015,5 +2025,11 @@ int cmd_ftrace(int argc, const char **argv)
>         delete_filter_func(&ftrace.nograph_funcs);
>         delete_filter_func(&ftrace.event_pair);
>
> +       signal(SIGINT, SIG_DFL);
> +       signal(SIGUSR1, SIG_DFL);
> +       signal(SIGCHLD, SIG_DFL);
> +       signal(SIGPIPE, SIG_DFL);
> +
> +       sem_destroy(&sig_sem);
>         return ret;
>  }
> --
> 2.55.0.897.gb25b4bd76c-goog
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] perf ftrace latency: Do not read trace files when BPF is used
  2026-09-04 17:36 ` Ian Rogers
@ 2026-09-08 21:04   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-09-08 21:04 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Namhyung Kim, Jiri Olsa, Adrian Hunter, James Clark,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users,
	Steven Rostedt, Masami Hiramatsu

On Fri, Sep 04, 2026 at 10:36:08AM -0700, Ian Rogers wrote:
> On Sun, Aug 30, 2026 at 11:39 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > I've realized that it didn't set up the tracing files when BPF is used
> > so poll() just returns immediately.  It ends up with calling poll()
> > unnecessarily in a loop.
> >
> > BPF still needs the loop to wait for the target process exiting or a
> > signal from users.  Let's use a semaphore instead.
> >
> > Suggested-by: Ian Rogers <irogers@google.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks, applied to perf-tools-next, for v7.4.

- Arnaldo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-08 21:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31  6:39 [PATCH v2] perf ftrace latency: Do not read trace files when BPF is used Namhyung Kim
2026-08-31  6:47 ` sashiko-bot
2026-09-04 17:36 ` Ian Rogers
2026-09-08 21:04   ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).