| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fUm6kWmX3FzFYJ_xoBgfgs0zPigyMKLy7VrXvNt1sC+iQ@mail.gmail.com>
Date: Mon, 9 Dec 2024 18:24:21 -0800
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Howard Chu <howardchu95@...il.com>, peterz@...radead.org, mingo@...hat.com,
acme@...nel.org, mark.rutland@....com, alexander.shishkin@...ux.intel.com,
jolsa@...nel.org, adrian.hunter@...el.com, kan.liang@...ux.intel.com,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
Arnaldo Carvalho de Melo <acme@...hat.com>, James Clark <james.clark@...aro.org>
Subject: Re: [PATCH v9 01/10] perf record --off-cpu: Add --off-cpu-thresh option
On Mon, Dec 9, 2024 at 4:09 PM Namhyung Kim <namhyung@...nel.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@...gle.com>
> > Suggested-by: Namhyung Kim <namhyung@...nel.org>
> > Suggested-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> > Reviewed-by: Ian Rogers <irogers@...gle.com>
> > Signed-off-by: Howard Chu <howardchu95@...il.com>
> > Cc: Adrian Hunter <adrian.hunter@...el.com>
> > Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> > Cc: Ingo Molnar <mingo@...hat.com>
> > Cc: James Clark <james.clark@...aro.org>
> > Cc: Jiri Olsa <jolsa@...nel.org>
> > Cc: Kan Liang <kan.liang@...ux.intel.com>
> > Cc: Mark Rutland <mark.rutland@....com>
> > Cc: Namhyung Kim <namhyung@...nel.org>
> > Cc: Peter Zijlstra <peterz@...radead.org>
> > Link: https://lore.kernel.org/r/20241108204137.2444151-2-howardchu95@gmail.com
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.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
> >
Powered by blists - more mailing lists