linux-kernel - Re: [PATCH v13 09/10] perf record --off-cpu: Add --off-cpu-thresh option

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAH0uvohK2sc7Em0+Ka=oEQcmsP5KQj-x=VYTUYYt1Vr3qD-d9w@mail.gmail.com>
Date: Thu, 12 Dec 2024 18:33:58 -0800
From: Howard Chu <howardchu95@...il.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: acme@...nel.org, mark.rutland@....com, alexander.shishkin@...ux.intel.com, 
	jolsa@...nel.org, irogers@...gle.com, 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>, 
	Ingo Molnar <mingo@...hat.com>, James Clark <james.clark@...aro.org>, 
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v13 09/10] perf record --off-cpu: Add --off-cpu-thresh option

Hello Namhyung,

On Thu, Dec 12, 2024 at 5:58 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> On Thu, Dec 12, 2024 at 03:27:16PM -0800, Howard Chu wrote:
> > Specify the threshold for dumping offcpu samples with --off-cpu-thresh,
> > the unit is milliseconds. Default value is 500ms.
> >
> > Example:
> >
> >   perf record --off-cpu --off-cpu-thresh 824
> >
> > The example above collects off-cpu samples where the off-cpu time is
> > longer than 824ms
> >
> > 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/bpf_off_cpu.c            |  3 +++
> >  tools/perf/util/bpf_skel/off_cpu.bpf.c   |  2 +-
> >  tools/perf/util/off_cpu.h                |  1 +
> >  tools/perf/util/record.h                 |  1 +
> >  6 files changed, 41 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> > index 242223240a08..f3ac4c739d5f 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 (in milliseconds), it
> > +     generates a direct off-cpu sample. The default is 500ms.
> > +
> >  --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 0b637cea4850..62183a6857f2 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -3147,6 +3147,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_ms; // converted to us for potential future improvements
>
> I'm not sure about the future, but we now have the option in msec and
> convert it to usec, and finally BPF uses nsec.  Is the usec conversion
> really needed?  Maybe we can just use nsec internally.

Actually got the same idea lol. I will change it to nsec.

Thanks,
Howard