From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
Cc: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
Jiri Olsa <olsajiri@gmail.com>, Ian Rogers <irogers@google.com>,
linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v2] libperf: Add API for allocating new thread map
Date: Mon, 21 Mar 2022 18:10:55 -0300 [thread overview]
Message-ID: <Yjjp37EkI4sbNCx/@kernel.org> (raw)
In-Reply-To: <CAPpZLN4Nov2XGE1FQ4FAU7JKSmUfDK0+VdZcnCO-WkTAE-LoBA@mail.gmail.com>
Em Mon, Mar 21, 2022 at 01:10:49PM +0200, Tzvetomir Stoyanov escreveu:
> On Wed, Feb 23, 2022 at 5:30 PM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Wed, Feb 23, 2022 at 02:31:01PM +0100, Jiri Olsa escreveu:
> > > On Tue, Feb 22, 2022 at 09:22:06PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Mon, Feb 21, 2022 at 08:46:49PM +0100, Jiri Olsa escreveu:
> > > > > On Mon, Feb 21, 2022 at 12:26:28PM +0200, Tzvetomir Stoyanov (VMware) wrote:
> > > > > > The existing API perf_thread_map__new_dummy() allocates new thread map
> > > > > > for one thread. I couldn't find a way to reallocate the map with more
> > > > > > threads, or to allocate a new map for more than one thread. Having
> > > > > > multiple threads in a thread map is essential for some use cases.
> > > > > > That's why a new API is proposed, which allocates a new thread map
> > > > > > for given number of threads:
> > > > > > perf_thread_map__new_array()
> > > > > >
> > > > > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> > > > > > ---
> > > > > > v2 changes:
> > > > > > - Changed the new API as Arnaldo suggested - get number of threads in the
> > > > > > map and array with initial values (optional).
> > > > >
> > > > > looks good, would you also find useful in your use case the comm support
> > > >
> > > > Is that an Acked-by? :-)
> > >
> > > yes, I wasn't sure about the previous args name changes,
> > > this one is fine
> > >
> > > Acked-by: Jiri Olsa <jolsa@kernel.org>
> >
> > Ok, applying this one after applying the arg name changes one, please
> > confirm if you agree with that one.
> >
> > - Arnaldo
>
> ping
> Please, can you merge this one, If there are no problems with the
> code. We would like to use the new API perf_thread_map__new_array().
> Thanks!
Its there already, was merged one month ago:
commit 56dce868198cd01b023e05d72bbbba7c87cc384d
Author: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
Date: Mon Feb 21 12:26:28 2022 +0200
libperf: Add API for allocating new thread map array
Its in perf/core, will go to Linus this week.
- Arnaldo
> > > thanks,
> > > jirka
> > >
> > > >
> > > > - Arnaldo
> > > >
> > > > > in thread_map? the thread_map__read_comms function? we could move it from
> > > > > perf to libperf
> > > > >
> > > > > thanks,
> > > > > jirka
> > > > >
> > > > > >
> > > > > > tools/lib/perf/Documentation/libperf.txt | 1 +
> > > > > > tools/lib/perf/include/perf/threadmap.h | 1 +
> > > > > > tools/lib/perf/libperf.map | 1 +
> > > > > > tools/lib/perf/tests/test-threadmap.c | 41 ++++++++++++++++++++++++
> > > > > > tools/lib/perf/threadmap.c | 24 ++++++++++----
> > > > > > 5 files changed, 61 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/tools/lib/perf/Documentation/libperf.txt b/tools/lib/perf/Documentation/libperf.txt
> > > > > > index a21f733b95b1..a8f1a237931b 100644
> > > > > > --- a/tools/lib/perf/Documentation/libperf.txt
> > > > > > +++ b/tools/lib/perf/Documentation/libperf.txt
> > > > > > @@ -62,6 +62,7 @@ SYNOPSIS
> > > > > > struct perf_thread_map;
> > > > > >
> > > > > > struct perf_thread_map *perf_thread_map__new_dummy(void);
> > > > > > + struct perf_thread_map *perf_thread_map__new_array(int nr_threads, pid_t *array);
> > > > > >
> > > > > > void perf_thread_map__set_pid(struct perf_thread_map *map, int idx, pid_t pid);
> > > > > > char *perf_thread_map__comm(struct perf_thread_map *map, int idx);
> > > > > > diff --git a/tools/lib/perf/include/perf/threadmap.h b/tools/lib/perf/include/perf/threadmap.h
> > > > > > index 58f7fbdce446..8b40e7777cea 100644
> > > > > > --- a/tools/lib/perf/include/perf/threadmap.h
> > > > > > +++ b/tools/lib/perf/include/perf/threadmap.h
> > > > > > @@ -8,6 +8,7 @@
> > > > > > struct perf_thread_map;
> > > > > >
> > > > > > LIBPERF_API struct perf_thread_map *perf_thread_map__new_dummy(void);
> > > > > > +LIBPERF_API struct perf_thread_map *perf_thread_map__new_array(int nr_threads, pid_t *array);
> > > > > >
> > > > > > LIBPERF_API void perf_thread_map__set_pid(struct perf_thread_map *map, int idx, pid_t pid);
> > > > > > LIBPERF_API char *perf_thread_map__comm(struct perf_thread_map *map, int idx);
> > > > > > diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
> > > > > > index 6fa0d651576b..190b56ae923a 100644
> > > > > > --- a/tools/lib/perf/libperf.map
> > > > > > +++ b/tools/lib/perf/libperf.map
> > > > > > @@ -12,6 +12,7 @@ LIBPERF_0.0.1 {
> > > > > > perf_cpu_map__empty;
> > > > > > perf_cpu_map__max;
> > > > > > perf_cpu_map__has;
> > > > > > + perf_thread_map__new_array;
> > > > > > perf_thread_map__new_dummy;
> > > > > > perf_thread_map__set_pid;
> > > > > > perf_thread_map__comm;
> > > > > > diff --git a/tools/lib/perf/tests/test-threadmap.c b/tools/lib/perf/tests/test-threadmap.c
> > > > > > index 5e2a0291e94c..f728ad7002bb 100644
> > > > > > --- a/tools/lib/perf/tests/test-threadmap.c
> > > > > > +++ b/tools/lib/perf/tests/test-threadmap.c
> > > > > > @@ -11,9 +11,43 @@ static int libperf_print(enum libperf_print_level level,
> > > > > > return vfprintf(stderr, fmt, ap);
> > > > > > }
> > > > > >
> > > > > > +static int test_threadmap_array(int nr, pid_t *array)
> > > > > > +{
> > > > > > + struct perf_thread_map *threads;
> > > > > > + int i;
> > > > > > +
> > > > > > + threads = perf_thread_map__new_array(nr, array);
> > > > > > + __T("Failed to allocate new thread map", threads);
> > > > > > +
> > > > > > + __T("Unexpected number of threads", perf_thread_map__nr(threads) == nr);
> > > > > > +
> > > > > > + for (i = 0; i < nr; i++) {
> > > > > > + __T("Unexpected initial value of thread",
> > > > > > + perf_thread_map__pid(threads, i) == (array ? array[i] : -1));
> > > > > > + }
> > > > > > +
> > > > > > + for (i = 1; i < nr; i++)
> > > > > > + perf_thread_map__set_pid(threads, i, i * 100);
> > > > > > +
> > > > > > + __T("Unexpected value of thread 0",
> > > > > > + perf_thread_map__pid(threads, 0) == (array ? array[0] : -1));
> > > > > > +
> > > > > > + for (i = 1; i < nr; i++) {
> > > > > > + __T("Unexpected thread value",
> > > > > > + perf_thread_map__pid(threads, i) == i * 100);
> > > > > > + }
> > > > > > +
> > > > > > + perf_thread_map__put(threads);
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > +#define THREADS_NR 10
> > > > > > int test_threadmap(int argc, char **argv)
> > > > > > {
> > > > > > struct perf_thread_map *threads;
> > > > > > + pid_t thr_array[THREADS_NR];
> > > > > > + int i;
> > > > > >
> > > > > > __T_START;
> > > > > >
> > > > > > @@ -27,6 +61,13 @@ int test_threadmap(int argc, char **argv)
> > > > > > perf_thread_map__put(threads);
> > > > > > perf_thread_map__put(threads);
> > > > > >
> > > > > > + test_threadmap_array(THREADS_NR, NULL);
> > > > > > +
> > > > > > + for (i = 0; i < THREADS_NR; i++)
> > > > > > + thr_array[i] = i + 100;
> > > > > > +
> > > > > > + test_threadmap_array(THREADS_NR, thr_array);
> > > > > > +
> > > > > > __T_END;
> > > > > > return tests_failed == 0 ? 0 : -1;
> > > > > > }
> > > > > > diff --git a/tools/lib/perf/threadmap.c b/tools/lib/perf/threadmap.c
> > > > > > index 84fa07c79d00..07968f3ea093 100644
> > > > > > --- a/tools/lib/perf/threadmap.c
> > > > > > +++ b/tools/lib/perf/threadmap.c
> > > > > > @@ -42,18 +42,28 @@ char *perf_thread_map__comm(struct perf_thread_map *map, int idx)
> > > > > > return map->map[idx].comm;
> > > > > > }
> > > > > >
> > > > > > -struct perf_thread_map *perf_thread_map__new_dummy(void)
> > > > > > +struct perf_thread_map *perf_thread_map__new_array(int nr_threads, pid_t *array)
> > > > > > {
> > > > > > - struct perf_thread_map *threads = thread_map__alloc(1);
> > > > > > + struct perf_thread_map *threads = thread_map__alloc(nr_threads);
> > > > > > + int i;
> > > > > > +
> > > > > > + if (!threads)
> > > > > > + return NULL;
> > > > > > +
> > > > > > + for (i = 0; i < nr_threads; i++)
> > > > > > + perf_thread_map__set_pid(threads, i, array ? array[i] : -1);
> > > > > > +
> > > > > > + threads->nr = nr_threads;
> > > > > > + refcount_set(&threads->refcnt, 1);
> > > > > >
> > > > > > - if (threads != NULL) {
> > > > > > - perf_thread_map__set_pid(threads, 0, -1);
> > > > > > - threads->nr = 1;
> > > > > > - refcount_set(&threads->refcnt, 1);
> > > > > > - }
> > > > > > return threads;
> > > > > > }
> > > > > >
> > > > > > +struct perf_thread_map *perf_thread_map__new_dummy(void)
> > > > > > +{
> > > > > > + return perf_thread_map__new_array(1, NULL);
> > > > > > +}
> > > > > > +
> > > > > > static void perf_thread_map__delete(struct perf_thread_map *threads)
> > > > > > {
> > > > > > if (threads) {
> > > > > > --
> > > > > > 2.34.1
> > > > > >
> > > >
> > > > --
> > > >
> > > > - Arnaldo
> >
> > --
> >
> > - Arnaldo
>
>
>
> --
> Tzvetomir (Ceco) Stoyanov
> VMware Open Source Technology Center
--
- Arnaldo
prev parent reply other threads:[~2022-03-21 21:11 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-21 10:26 [PATCH v2] libperf: Add API for allocating new thread map Tzvetomir Stoyanov (VMware)
2022-02-21 19:46 ` Jiri Olsa
2022-02-21 20:07 ` Arnaldo Carvalho de Melo
2022-02-22 2:32 ` Tzvetomir Stoyanov
2022-02-23 0:21 ` Arnaldo Carvalho de Melo
2022-02-23 9:08 ` Tzvetomir Stoyanov
2022-02-23 13:02 ` Jiri Olsa
2022-02-23 15:47 ` Tzvetomir Stoyanov
2022-02-23 18:06 ` Jiri Olsa
2022-02-22 2:21 ` Tzvetomir Stoyanov
2022-02-23 0:22 ` Arnaldo Carvalho de Melo
2022-02-23 13:31 ` Jiri Olsa
2022-02-23 15:30 ` Arnaldo Carvalho de Melo
2022-03-21 11:10 ` Tzvetomir Stoyanov
2022-03-21 21:10 ` Arnaldo Carvalho de Melo [this message]
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=Yjjp37EkI4sbNCx/@kernel.org \
--to=acme@kernel.org \
--cc=arnaldo.melo@gmail.com \
--cc=irogers@google.com \
--cc=linux-perf-users@vger.kernel.org \
--cc=olsajiri@gmail.com \
--cc=tz.stoyanov@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).