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

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

Example:

  perf record --off-cpu --off-cpu-thresh 400000

The example above collects off-cpu samples whose off-cpu time is longer
than 400,000us

Suggested-by: Ian Rogers <irogers@google.com>
Suggested-by: Namhyung Kim <namhyung@kernel.org>
Suggested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Howard Chu <howardchu95@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Clark <james.clark@linaro.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: https://lore.kernel.org/r/20241108204137.2444151-2-howardchu95@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-record.txt |  9 ++++++++
 tools/perf/builtin-record.c              | 26 ++++++++++++++++++++++++
 tools/perf/util/off_cpu.h                |  1 +
 tools/perf/util/record.h                 |  1 +
 4 files changed, 37 insertions(+)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 242223240a08..05c8977983de 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -829,6 +829,15 @@ filtered through the mask provided by -C option.
 	only, as of now.  So the applications built without the frame
 	pointer might see bogus addresses.
 
+	off-cpu profiling consists two types of samples: direct samples, which
+	share the same behavior as regular samples, and the accumulated
+	samples, stored in BPF stack trace map, presented after all the regular
+	samples.
+
+--off-cpu-thresh::
+	Once a task's off-cpu time reaches this threshold, it generates a
+	direct off-cpu sample.
+
 --setup-filter=<action>::
 	Prepare BPF filter to be used by regular users.  The action should be
 	either "pin" or "unpin".  The filter can be used after it's pinned.
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f83252472921..c069000efe5c 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_us;
+
+	if (!str)
+		return -EINVAL;
+
+	off_cpu_thresh_us = strtoull(str, &endptr, 10);
+
+	/* threshold isn't string "0", yet strtoull() returns 0, parsing failed */
+	if (*endptr || (off_cpu_thresh_us == 0 && strcmp(str, "0")))
+		return -EINVAL;
+	else
+		opts->off_cpu_thresh_us = off_cpu_thresh_us;
+
+	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_us   = 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..2ca74add26c0 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_us;
 };
 
 extern const char * const *record_usage;
-- 
2.43.0
Re: [PATCH v9 01/10] perf record --off-cpu: Add --off-cpu-thresh option
Posted by Namhyung Kim 1 year, 2 months ago
Hi Howard,

Sorry for the late review.

On Thu, Nov 21, 2024 at 08:38:31PM -0800, Howard Chu wrote:
> Specify the threshold for dumping offcpu samples with --off-cpu-thresh,
> the unit is us (microsecond). Default value is 500,000us (500ms, 0.5s).

I guess we mostly care about more than milli-seconds of off-cpu times.
Can we change the unit to msec?

Also I think this commit can be moved to later in this series - like
after implementing direct offcpu samples.  If I see the commit in
history later, I'd think it works.  But it's not at this point.

Otherwise, looks all good to me. :)

Thanks,
Namhyung

> 
> Example:
> 
>   perf record --off-cpu --off-cpu-thresh 400000
> 
> The example above collects off-cpu samples whose off-cpu time is longer
> than 400,000us
> 
> Suggested-by: Ian Rogers <irogers@google.com>
> Suggested-by: Namhyung Kim <namhyung@kernel.org>
> Suggested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Reviewed-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Howard Chu <howardchu95@gmail.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: James Clark <james.clark@linaro.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kan Liang <kan.liang@linux.intel.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Link: https://lore.kernel.org/r/20241108204137.2444151-2-howardchu95@gmail.com
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/Documentation/perf-record.txt |  9 ++++++++
>  tools/perf/builtin-record.c              | 26 ++++++++++++++++++++++++
>  tools/perf/util/off_cpu.h                |  1 +
>  tools/perf/util/record.h                 |  1 +
>  4 files changed, 37 insertions(+)
> 
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 242223240a08..05c8977983de 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -829,6 +829,15 @@ filtered through the mask provided by -C option.
>  	only, as of now.  So the applications built without the frame
>  	pointer might see bogus addresses.
>  
> +	off-cpu profiling consists two types of samples: direct samples, which
> +	share the same behavior as regular samples, and the accumulated
> +	samples, stored in BPF stack trace map, presented after all the regular
> +	samples.
> +
> +--off-cpu-thresh::
> +	Once a task's off-cpu time reaches this threshold, it generates a
> +	direct off-cpu sample.
> +
>  --setup-filter=<action>::
>  	Prepare BPF filter to be used by regular users.  The action should be
>  	either "pin" or "unpin".  The filter can be used after it's pinned.
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index f83252472921..c069000efe5c 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_us;
> +
> +	if (!str)
> +		return -EINVAL;
> +
> +	off_cpu_thresh_us = strtoull(str, &endptr, 10);
> +
> +	/* threshold isn't string "0", yet strtoull() returns 0, parsing failed */
> +	if (*endptr || (off_cpu_thresh_us == 0 && strcmp(str, "0")))
> +		return -EINVAL;
> +	else
> +		opts->off_cpu_thresh_us = off_cpu_thresh_us;
> +
> +	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_us   = 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..2ca74add26c0 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_us;
>  };
>  
>  extern const char * const *record_usage;
> -- 
> 2.43.0
>
Re: [PATCH v9 01/10] perf record --off-cpu: Add --off-cpu-thresh option
Posted by Ian Rogers 1 year, 2 months ago
On Mon, Dec 9, 2024 at 4:09 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Howard,
>
> Sorry for the late review.
>
> On Thu, Nov 21, 2024 at 08:38:31PM -0800, Howard Chu wrote:
> > Specify the threshold for dumping offcpu samples with --off-cpu-thresh,
> > the unit is us (microsecond). Default value is 500,000us (500ms, 0.5s).
>
> I guess we mostly care about more than milli-seconds of off-cpu times.
> Can we change the unit to msec?

I wonder if there should be two values, one for the command line and
one in the code. I think microseconds make most sense in the code, 1ms
being roughly 1000 context switches. On the command line millisecond
would align with perf stat's -I option, and as you point out, users
aren't usually working at the granularity of a microsecond. Having the
code be microsecond oriented allows the option to have a more precise
command line option in the future, autotuning, etc.

Thanks,
Ian

> Also I think this commit can be moved to later in this series - like
> after implementing direct offcpu samples.  If I see the commit in
> history later, I'd think it works.  But it's not at this point.
>
> Otherwise, looks all good to me. :)
>
> Thanks,
> Namhyung
>
> >
> > Example:
> >
> >   perf record --off-cpu --off-cpu-thresh 400000
> >
> > The example above collects off-cpu samples whose off-cpu time is longer
> > than 400,000us
> >
> > Suggested-by: Ian Rogers <irogers@google.com>
> > Suggested-by: Namhyung Kim <namhyung@kernel.org>
> > Suggested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Reviewed-by: Ian Rogers <irogers@google.com>
> > Signed-off-by: Howard Chu <howardchu95@gmail.com>
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: James Clark <james.clark@linaro.org>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Kan Liang <kan.liang@linux.intel.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Link: https://lore.kernel.org/r/20241108204137.2444151-2-howardchu95@gmail.com
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/Documentation/perf-record.txt |  9 ++++++++
> >  tools/perf/builtin-record.c              | 26 ++++++++++++++++++++++++
> >  tools/perf/util/off_cpu.h                |  1 +
> >  tools/perf/util/record.h                 |  1 +
> >  4 files changed, 37 insertions(+)
> >
> > diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> > index 242223240a08..05c8977983de 100644
> > --- a/tools/perf/Documentation/perf-record.txt
> > +++ b/tools/perf/Documentation/perf-record.txt
> > @@ -829,6 +829,15 @@ filtered through the mask provided by -C option.
> >       only, as of now.  So the applications built without the frame
> >       pointer might see bogus addresses.
> >
> > +     off-cpu profiling consists two types of samples: direct samples, which
> > +     share the same behavior as regular samples, and the accumulated
> > +     samples, stored in BPF stack trace map, presented after all the regular
> > +     samples.
> > +
> > +--off-cpu-thresh::
> > +     Once a task's off-cpu time reaches this threshold, it generates a
> > +     direct off-cpu sample.
> > +
> >  --setup-filter=<action>::
> >       Prepare BPF filter to be used by regular users.  The action should be
> >       either "pin" or "unpin".  The filter can be used after it's pinned.
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index f83252472921..c069000efe5c 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_us;
> > +
> > +     if (!str)
> > +             return -EINVAL;
> > +
> > +     off_cpu_thresh_us = strtoull(str, &endptr, 10);
> > +
> > +     /* threshold isn't string "0", yet strtoull() returns 0, parsing failed */
> > +     if (*endptr || (off_cpu_thresh_us == 0 && strcmp(str, "0")))
> > +             return -EINVAL;
> > +     else
> > +             opts->off_cpu_thresh_us = off_cpu_thresh_us;
> > +
> > +     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_us   = 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..2ca74add26c0 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_us;
> >  };
> >
> >  extern const char * const *record_usage;
> > --
> > 2.43.0
> >
Re: [PATCH v9 01/10] perf record --off-cpu: Add --off-cpu-thresh option
Posted by Namhyung Kim 1 year, 2 months ago
On Mon, Dec 09, 2024 at 06:24:21PM -0800, Ian Rogers wrote:
> On Mon, Dec 9, 2024 at 4:09 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hi Howard,
> >
> > Sorry for the late review.
> >
> > On Thu, Nov 21, 2024 at 08:38:31PM -0800, Howard Chu wrote:
> > > Specify the threshold for dumping offcpu samples with --off-cpu-thresh,
> > > the unit is us (microsecond). Default value is 500,000us (500ms, 0.5s).
> >
> > I guess we mostly care about more than milli-seconds of off-cpu times.
> > Can we change the unit to msec?
> 
> I wonder if there should be two values, one for the command line and
> one in the code. I think microseconds make most sense in the code, 1ms
> being roughly 1000 context switches. On the command line millisecond
> would align with perf stat's -I option, and as you point out, users
> aren't usually working at the granularity of a microsecond. Having the
> code be microsecond oriented allows the option to have a more precise
> command line option in the future, autotuning, etc.

I'm fine as long as it has a proper suffix for the unit. :)

Thanks,
Namhyung