[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAM9d7chu8UFNGBk-1rEBY9+_Q+8dj_4CQ4VJ1BZzs+Dq3T=Shg@mail.gmail.com>
Date: Tue, 2 Aug 2022 11:08:09 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Jiri Olsa <jolsa@...nel.org>, Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
LKML <linux-kernel@...r.kernel.org>,
Ian Rogers <irogers@...gle.com>,
linux-perf-users <linux-perf-users@...r.kernel.org>,
Will Deacon <will@...nel.org>,
Waiman Long <longman@...hat.com>,
Boqun Feng <boqun.feng@...il.com>,
Davidlohr Bueso <dave@...olabs.net>,
Song Liu <songliubraving@...com>,
Blake Jones <blakejones@...gle.com>
Subject: Re: [PATCH 3/3] perf lock: Print lost entries at the end
On Tue, Aug 2, 2022 at 10:45 AM Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
> Em Tue, Aug 02, 2022 at 12:35:11AM -0700, Namhyung Kim escreveu:
> > Like the normal perf lock report output, it'd print bad stats at the end
> > if exists or -v option is passed. Currently it uses BROKEN_CONTENDED
> stat for the lost count (due to full stack maps).
>
> "Print the number of lost entries in verbose mode"?
Do you want me to change the commit description?
Note that it can print the numbers not in verbose mode.
It used to print unconditionally but I changed it to do
that only if there's a bad number. The -v option can
help to keep the original behavior of printing it always.
Thanks,
Namhyung
>
> > $ sudo perf lock con -a -b -m 128 sleep 5
> > ...
> > === output for debug===
> >
> > bad: 43, total: 14903
> > bad rate: 0.29 %
> > histogram of events caused bad sequence
> > acquire: 0
> > acquired: 0
> > contended: 43
> > release: 0
> >
> > Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> > ---
> > tools/perf/builtin-lock.c | 8 +++++++-
> > tools/perf/util/bpf_lock_contention.c | 6 ++++--
> > tools/perf/util/bpf_skel/lock_contention.bpf.c | 9 +++++++--
> > tools/perf/util/lock-contention.h | 1 +
> > 4 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> > index e32fdcd497e0..8065f0268e55 100644
> > --- a/tools/perf/builtin-lock.c
> > +++ b/tools/perf/builtin-lock.c
> > @@ -1471,8 +1471,11 @@ static void print_contention_result(void)
> > pr_info(" %10s %s\n\n", "type", "caller");
> >
> > bad = total = 0;
> > + if (use_bpf)
> > + bad = bad_hist[BROKEN_CONTENDED];
> > +
> > while ((st = pop_from_result())) {
> > - total++;
> > + total += use_bpf ? st->nr_contended : 1;
> > if (st->broken)
> > bad++;
> >
> > @@ -1686,6 +1689,9 @@ static int __cmd_contention(int argc, const char **argv)
> >
> > lock_contention_stop();
> > lock_contention_read(&con);
> > +
> > + /* abuse bad hist stats for lost entries */
> > + bad_hist[BROKEN_CONTENDED] = con.lost;
> > } else {
> > err = perf_session__process_events(session);
> > if (err)
> > diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
> > index 26128e5bb659..65f51cc25236 100644
> > --- a/tools/perf/util/bpf_lock_contention.c
> > +++ b/tools/perf/util/bpf_lock_contention.c
> > @@ -16,7 +16,7 @@ static struct lock_contention_bpf *skel;
> >
> > /* should be same as bpf_skel/lock_contention.bpf.c */
> > struct lock_contention_key {
> > - u32 stack_id;
> > + s32 stack_id;
> > };
> >
> > struct lock_contention_data {
> > @@ -110,7 +110,7 @@ int lock_contention_stop(void)
> > int lock_contention_read(struct lock_contention *con)
> > {
> > int fd, stack;
> > - u32 prev_key, key;
> > + s32 prev_key, key;
> > struct lock_contention_data data;
> > struct lock_stat *st;
> > struct machine *machine = con->machine;
> > @@ -119,6 +119,8 @@ int lock_contention_read(struct lock_contention *con)
> > fd = bpf_map__fd(skel->maps.lock_stat);
> > stack = bpf_map__fd(skel->maps.stacks);
> >
> > + con->lost = skel->bss->lost;
> > +
> > prev_key = 0;
> > while (!bpf_map_get_next_key(fd, &prev_key, &key)) {
> > struct map *kmap;
> > diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> > index 67d46533e518..9e8b94eb6320 100644
> > --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> > +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> > @@ -12,7 +12,7 @@
> > #define MAX_ENTRIES 10240
> >
> > struct contention_key {
> > - __u32 stack_id;
> > + __s32 stack_id;
> > };
> >
> > struct contention_data {
> > @@ -27,7 +27,7 @@ struct tstamp_data {
> > __u64 timestamp;
> > __u64 lock;
> > __u32 flags;
> > - __u32 stack_id;
> > + __s32 stack_id;
> > };
> >
> > /* callstack storage */
> > @@ -73,6 +73,9 @@ int enabled;
> > int has_cpu;
> > int has_task;
> >
> > +/* error stat */
> > +unsigned long lost;
> > +
> > static inline int can_record(void)
> > {
> > if (has_cpu) {
> > @@ -116,6 +119,8 @@ int contention_begin(u64 *ctx)
> > pelem->flags = (__u32)ctx[1];
> > pelem->stack_id = bpf_get_stackid(ctx, &stacks, BPF_F_FAST_STACK_CMP);
> >
> > + if (pelem->stack_id < 0)
> > + lost++;
> > return 0;
> > }
> >
> > diff --git a/tools/perf/util/lock-contention.h b/tools/perf/util/lock-contention.h
> > index b09fd6eb978a..d9fc5f076567 100644
> > --- a/tools/perf/util/lock-contention.h
> > +++ b/tools/perf/util/lock-contention.h
> > @@ -113,6 +113,7 @@ struct lock_contention {
> > struct machine *machine;
> > struct hlist_head *result;
> > unsigned long map_len;
> > + unsigned long lost;
> > };
> >
> > #ifdef HAVE_BPF_SKEL
> > --
> > 2.37.1.455.g008518b4e5-goog
>
> --
>
> - Arnaldo
Powered by blists - more mailing lists