[PATCH v7 01/10] perf record --off-cpu: Add --off-cpu-thresh option

Howard Chu posted 10 patches 2 weeks, 1 day ago
There is a newer version of this series
[PATCH v7 01/10] perf record --off-cpu: Add --off-cpu-thresh option
Posted by Howard Chu 2 weeks, 1 day ago
Specify the threshold for dumping offcpu samples with --off-cpu-thresh,
the unit is us (microsecond). Default value is 500000us (500ms, 0.5s).

Suggested-by: Ian Rogers <irogers@google.com>
Signed-off-by: Howard Chu <howardchu95@gmail.com>
---
 tools/perf/builtin-record.c | 26 ++++++++++++++++++++++++++
 tools/perf/util/off_cpu.h   |  1 +
 tools/perf/util/record.h    |  1 +
 3 files changed, 28 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f83252472921..ee04fdd7f2ca 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -3149,6 +3149,28 @@ static int record__parse_mmap_pages(const struct option *opt,
 	return ret;
 }
 
+static int record__parse_off_cpu_thresh(const struct option *opt,
+					const char *str,
+					int unset __maybe_unused)
+{
+	struct record_opts *opts = opt->value;
+	char *endptr;
+	u64 off_cpu_thresh;
+
+	if (!str)
+		return -EINVAL;
+
+	off_cpu_thresh = strtoull(str, &endptr, 10);
+
+	/* threshold isn't string "0", yet strtoull() returns 0, parsing failed */
+	if (*endptr || (off_cpu_thresh == 0 && strcmp(str, "0")))
+		return -EINVAL;
+	else
+		opts->off_cpu_thresh = off_cpu_thresh;
+
+	return 0;
+}
+
 void __weak arch__add_leaf_frame_record_opts(struct record_opts *opts __maybe_unused)
 {
 }
@@ -3342,6 +3364,7 @@ static struct record record = {
 		.ctl_fd              = -1,
 		.ctl_fd_ack          = -1,
 		.synth               = PERF_SYNTH_ALL,
+		.off_cpu_thresh      = OFFCPU_THRESH,
 	},
 };
 
@@ -3564,6 +3587,9 @@ static struct option __record_options[] = {
 	OPT_BOOLEAN(0, "off-cpu", &record.off_cpu, "Enable off-cpu analysis"),
 	OPT_STRING(0, "setup-filter", &record.filter_action, "pin|unpin",
 		   "BPF filter action"),
+	OPT_CALLBACK(0, "off-cpu-thresh", &record.opts, "us",
+		     "Dump off-cpu samples if off-cpu time reaches this threshold. The unit is microsecond (default: 500000)",
+		     record__parse_off_cpu_thresh),
 	OPT_END()
 };
 
diff --git a/tools/perf/util/off_cpu.h b/tools/perf/util/off_cpu.h
index 2dd67c60f211..c6edc0f7c40d 100644
--- a/tools/perf/util/off_cpu.h
+++ b/tools/perf/util/off_cpu.h
@@ -16,6 +16,7 @@ struct record_opts;
 			      PERF_SAMPLE_PERIOD | PERF_SAMPLE_CALLCHAIN | \
 			      PERF_SAMPLE_CGROUP)
 
+#define OFFCPU_THRESH 500000ull
 
 #ifdef HAVE_BPF_SKEL
 int off_cpu_prepare(struct evlist *evlist, struct target *target,
diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
index a6566134e09e..3c11416e6627 100644
--- a/tools/perf/util/record.h
+++ b/tools/perf/util/record.h
@@ -79,6 +79,7 @@ struct record_opts {
 	int	      synth;
 	int	      threads_spec;
 	const char    *threads_user_spec;
+	u64	      off_cpu_thresh;
 };
 
 extern const char * const *record_usage;
-- 
2.43.0
Re: [PATCH v7 01/10] perf record --off-cpu: Add --off-cpu-thresh option
Posted by Ian Rogers 1 week, 5 days ago
On Fri, Nov 8, 2024 at 12:41 PM Howard Chu <howardchu95@gmail.com> wrote:
>
> Specify the threshold for dumping offcpu samples with --off-cpu-thresh,
> the unit is us (microsecond). Default value is 500000us (500ms, 0.5s).
>
> Suggested-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Howard Chu <howardchu95@gmail.com>
> ---
>  tools/perf/builtin-record.c | 26 ++++++++++++++++++++++++++
>  tools/perf/util/off_cpu.h   |  1 +
>  tools/perf/util/record.h    |  1 +
>  3 files changed, 28 insertions(+)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index f83252472921..ee04fdd7f2ca 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -3149,6 +3149,28 @@ static int record__parse_mmap_pages(const struct option *opt,
>         return ret;
>  }
>
> +static int record__parse_off_cpu_thresh(const struct option *opt,
> +                                       const char *str,
> +                                       int unset __maybe_unused)
> +{
> +       struct record_opts *opts = opt->value;
> +       char *endptr;
> +       u64 off_cpu_thresh;
> +
> +       if (!str)
> +               return -EINVAL;
> +
> +       off_cpu_thresh = strtoull(str, &endptr, 10);
> +
> +       /* threshold isn't string "0", yet strtoull() returns 0, parsing failed */
> +       if (*endptr || (off_cpu_thresh == 0 && strcmp(str, "0")))
> +               return -EINVAL;
> +       else
> +               opts->off_cpu_thresh = off_cpu_thresh;
> +
> +       return 0;
> +}
> +
>  void __weak arch__add_leaf_frame_record_opts(struct record_opts *opts __maybe_unused)
>  {
>  }
> @@ -3342,6 +3364,7 @@ static struct record record = {
>                 .ctl_fd              = -1,
>                 .ctl_fd_ack          = -1,
>                 .synth               = PERF_SYNTH_ALL,
> +               .off_cpu_thresh      = OFFCPU_THRESH,
>         },
>  };
>
> @@ -3564,6 +3587,9 @@ static struct option __record_options[] = {
>         OPT_BOOLEAN(0, "off-cpu", &record.off_cpu, "Enable off-cpu analysis"),
>         OPT_STRING(0, "setup-filter", &record.filter_action, "pin|unpin",
>                    "BPF filter action"),
> +       OPT_CALLBACK(0, "off-cpu-thresh", &record.opts, "us",
> +                    "Dump off-cpu samples if off-cpu time reaches this threshold. The unit is microsecond (default: 500000)",
> +                    record__parse_off_cpu_thresh),
>         OPT_END()
>  };
>
> diff --git a/tools/perf/util/off_cpu.h b/tools/perf/util/off_cpu.h
> index 2dd67c60f211..c6edc0f7c40d 100644
> --- a/tools/perf/util/off_cpu.h
> +++ b/tools/perf/util/off_cpu.h
> @@ -16,6 +16,7 @@ struct record_opts;
>                               PERF_SAMPLE_PERIOD | PERF_SAMPLE_CALLCHAIN | \
>                               PERF_SAMPLE_CGROUP)
>
> +#define OFFCPU_THRESH 500000ull
>
>  #ifdef HAVE_BPF_SKEL
>  int off_cpu_prepare(struct evlist *evlist, struct target *target,
> diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
> index a6566134e09e..3c11416e6627 100644
> --- a/tools/perf/util/record.h
> +++ b/tools/perf/util/record.h
> @@ -79,6 +79,7 @@ struct record_opts {
>         int           synth;
>         int           threads_spec;
>         const char    *threads_user_spec;
> +       u64           off_cpu_thresh;

nit: I know existing style doesn't do this but it would be nice if
this were off_cpu_thresh_us (ie the unit is at the end of the variable
name). Most programming languages have gone for having explicit
time/duration types to remove ambiguity. We're peasants so it'd be
nice if the variable name were hinting me in the write direction.

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

Thanks,
Ian

>  };
>
>  extern const char * const *record_usage;
> --
> 2.43.0
>