From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>, Ingo Molnar <mingo@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
LKML <linux-kernel@vger.kernel.org>,
Ian Rogers <irogers@google.com>,
linux-perf-users@vger.kernel.org, Will Deacon <will@kernel.org>,
Waiman Long <longman@redhat.com>,
Boqun Feng <boqun.feng@gmail.com>,
Davidlohr Bueso <dave@stgolabs.net>,
Song Liu <songliubraving@fb.com>,
Blake Jones <blakejones@google.com>
Subject: Re: [PATCH v2 1/3] perf lock: Introduce struct lock_contention
Date: Tue, 2 Aug 2022 18:04:29 -0300 [thread overview]
Message-ID: <YumRXcxc5XIUwlBO@kernel.org> (raw)
In-Reply-To: <20220802191004.347740-1-namhyung@kernel.org>
Em Tue, Aug 02, 2022 at 12:10:02PM -0700, Namhyung Kim escreveu:
> The lock_contention struct is to carry related fields together and to
> minimize the change when we add new config options.
Thanks, applied. Forgot the cover letter? :-)
- Arnaldo
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-lock.c | 23 ++++++++++++++---------
> tools/perf/util/bpf_lock_contention.c | 9 ++++++---
> tools/perf/util/lock-contention.h | 17 +++++++++++------
> 3 files changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> index 7897a33fec1b..eef778b7d33d 100644
> --- a/tools/perf/builtin-lock.c
> +++ b/tools/perf/builtin-lock.c
> @@ -1594,7 +1594,10 @@ static int __cmd_contention(int argc, const char **argv)
> .mode = PERF_DATA_MODE_READ,
> .force = force,
> };
> - struct evlist *evlist = NULL;
> + struct lock_contention con = {
> + .target = &target,
> + .result = &lockhash_table[0],
> + };
>
> session = perf_session__new(use_bpf ? NULL : &data, &eops);
> if (IS_ERR(session)) {
> @@ -1620,24 +1623,26 @@ static int __cmd_contention(int argc, const char **argv)
> signal(SIGCHLD, sighandler);
> signal(SIGTERM, sighandler);
>
> - evlist = evlist__new();
> - if (evlist == NULL) {
> + con.machine = &session->machines.host;
> +
> + con.evlist = evlist__new();
> + if (con.evlist == NULL) {
> err = -ENOMEM;
> goto out_delete;
> }
>
> - err = evlist__create_maps(evlist, &target);
> + err = evlist__create_maps(con.evlist, &target);
> if (err < 0)
> goto out_delete;
>
> if (argc) {
> - err = evlist__prepare_workload(evlist, &target,
> + err = evlist__prepare_workload(con.evlist, &target,
> argv, false, NULL);
> if (err < 0)
> goto out_delete;
> }
>
> - if (lock_contention_prepare(evlist, &target) < 0) {
> + if (lock_contention_prepare(&con) < 0) {
> pr_err("lock contention BPF setup failed\n");
> goto out_delete;
> }
> @@ -1672,13 +1677,13 @@ static int __cmd_contention(int argc, const char **argv)
> if (use_bpf) {
> lock_contention_start();
> if (argc)
> - evlist__start_workload(evlist);
> + evlist__start_workload(con.evlist);
>
> /* wait for signal */
> pause();
>
> lock_contention_stop();
> - lock_contention_read(&session->machines.host, &lockhash_table[0]);
> + lock_contention_read(&con);
> } else {
> err = perf_session__process_events(session);
> if (err)
> @@ -1691,7 +1696,7 @@ static int __cmd_contention(int argc, const char **argv)
> print_contention_result();
>
> out_delete:
> - evlist__delete(evlist);
> + evlist__delete(con.evlist);
> lock_contention_finish();
> perf_session__delete(session);
> return err;
> diff --git a/tools/perf/util/bpf_lock_contention.c b/tools/perf/util/bpf_lock_contention.c
> index 16b7451b4b09..f5e2b4f19a72 100644
> --- a/tools/perf/util/bpf_lock_contention.c
> +++ b/tools/perf/util/bpf_lock_contention.c
> @@ -27,10 +27,12 @@ struct lock_contention_data {
> u32 flags;
> };
>
> -int lock_contention_prepare(struct evlist *evlist, struct target *target)
> +int lock_contention_prepare(struct lock_contention *con)
> {
> int i, fd;
> int ncpus = 1, ntasks = 1;
> + struct evlist *evlist = con->evlist;
> + struct target *target = con->target;
>
> skel = lock_contention_bpf__open();
> if (!skel) {
> @@ -102,12 +104,13 @@ int lock_contention_stop(void)
> return 0;
> }
>
> -int lock_contention_read(struct machine *machine, struct hlist_head *head)
> +int lock_contention_read(struct lock_contention *con)
> {
> int fd, stack;
> u32 prev_key, key;
> struct lock_contention_data data;
> struct lock_stat *st;
> + struct machine *machine = con->machine;
> u64 stack_trace[CONTENTION_STACK_DEPTH];
>
> fd = bpf_map__fd(skel->maps.lock_stat);
> @@ -163,7 +166,7 @@ int lock_contention_read(struct machine *machine, struct hlist_head *head)
> return -1;
> }
>
> - hlist_add_head(&st->hash_entry, head);
> + hlist_add_head(&st->hash_entry, con->result);
> prev_key = key;
> }
>
> diff --git a/tools/perf/util/lock-contention.h b/tools/perf/util/lock-contention.h
> index 092c84441f9f..a0df5308cca4 100644
> --- a/tools/perf/util/lock-contention.h
> +++ b/tools/perf/util/lock-contention.h
> @@ -107,18 +107,24 @@ struct evlist;
> struct machine;
> struct target;
>
> +struct lock_contention {
> + struct evlist *evlist;
> + struct target *target;
> + struct machine *machine;
> + struct hlist_head *result;
> +};
> +
> #ifdef HAVE_BPF_SKEL
>
> -int lock_contention_prepare(struct evlist *evlist, struct target *target);
> +int lock_contention_prepare(struct lock_contention *con);
> int lock_contention_start(void);
> int lock_contention_stop(void);
> -int lock_contention_read(struct machine *machine, struct hlist_head *head);
> +int lock_contention_read(struct lock_contention *con);
> int lock_contention_finish(void);
>
> #else /* !HAVE_BPF_SKEL */
>
> -static inline int lock_contention_prepare(struct evlist *evlist __maybe_unused,
> - struct target *target __maybe_unused)
> +static inline int lock_contention_prepare(struct lock_contention *con __maybe_unused)
> {
> return 0;
> }
> @@ -127,8 +133,7 @@ static inline int lock_contention_start(void) { return 0; }
> static inline int lock_contention_stop(void) { return 0; }
> static inline int lock_contention_finish(void) { return 0; }
>
> -static inline int lock_contention_read(struct machine *machine __maybe_unused,
> - struct hlist_head *head __maybe_unused)
> +static inline int lock_contention_read(struct lock_contention *con __maybe_unused)
> {
> return 0;
> }
> --
> 2.37.1.455.g008518b4e5-goog
--
- Arnaldo
next prev parent reply other threads:[~2022-08-02 21:04 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-02 19:10 [PATCH v2 1/3] perf lock: Introduce struct lock_contention Namhyung Kim
2022-08-02 19:10 ` [PATCH v2 2/3] perf lock: Add --map-nr-entries option Namhyung Kim
2022-08-02 19:10 ` [PATCH v2 3/3] perf lock: Print the number of lost entries for BPF Namhyung Kim
2022-09-26 8:05 ` Jiri Slaby
2022-09-26 21:42 ` Namhyung Kim
2022-08-02 21:04 ` Arnaldo Carvalho de Melo [this message]
2022-08-02 23:47 ` [PATCH v2 1/3] perf lock: Introduce struct lock_contention Namhyung Kim
2022-08-03 15:12 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YumRXcxc5XIUwlBO@kernel.org \
--to=acme@kernel.org \
--cc=blakejones@google.com \
--cc=boqun.feng@gmail.com \
--cc=dave@stgolabs.net \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=longman@redhat.com \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=songliubraving@fb.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.