All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] libperf: Add API for allocating new thread map
@ 2022-02-21 10:26 Tzvetomir Stoyanov (VMware)
  2022-02-21 19:46 ` Jiri Olsa
  0 siblings, 1 reply; 15+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2022-02-21 10:26 UTC (permalink / raw)
  To: acme, olsajiri, irogers; +Cc: linux-perf-users

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).

 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  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
                     ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Jiri Olsa @ 2022-02-21 19:46 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: acme, irogers, linux-perf-users

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
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
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  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-22  2:21   ` Tzvetomir Stoyanov
  2022-02-23  0:22   ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-02-21 20:07 UTC (permalink / raw)
  To: Jiri Olsa, Tzvetomir Stoyanov (VMware); +Cc: acme, irogers, linux-perf-users



On February 21, 2022 4:46:49 PM GMT-03:00, Jiri Olsa <olsajiri@gmail.com> wrote:
>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
>in thread_map? the thread_map__read_comms function? we could move it from
>perf to libperf

IOW: we're happy that you're working on libperf, so feel encouraged to move things from tools/perf/util/ to tools/lib/perf/ if you find a supporting use case, brownie points if you also add an entry to tools/perf/tests/, AKA 'perf test'.

Thanks!

- Arnaldo

>
>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
>> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  2022-02-21 19:46 ` Jiri Olsa
  2022-02-21 20:07   ` Arnaldo Carvalho de Melo
@ 2022-02-22  2:21   ` Tzvetomir Stoyanov
  2022-02-23  0:22   ` Arnaldo Carvalho de Melo
  2 siblings, 0 replies; 15+ messages in thread
From: Tzvetomir Stoyanov @ 2022-02-22  2:21 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Arnaldo Carvalho de Melo, Ian Rogers, linux-perf-users

On Mon, Feb 21, 2022 at 9:46 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> 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
> in thread_map? the thread_map__read_comms function? we could move it from
> perf to libperf
>

Yes, we definitely need that functionality. I implemented that in our
code, but if there is support in libperf - I prefer to use it.
Thanks!

> 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
> >



-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  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
  0 siblings, 1 reply; 15+ messages in thread
From: Tzvetomir Stoyanov @ 2022-02-22  2:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Ian Rogers, linux-perf-users

On Mon, Feb 21, 2022 at 10:21 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
>
>
> On February 21, 2022 4:46:49 PM GMT-03:00, Jiri Olsa <olsajiri@gmail.com> wrote:
> >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
> >in thread_map? the thread_map__read_comms function? we could move it from
> >perf to libperf
>
> IOW: we're happy that you're working on libperf, so feel encouraged to move things from tools/perf/util/ to tools/lib/perf/ if you find a supporting use case, brownie points if you also add an entry to tools/perf/tests/, AKA 'perf test'.
>

Thanks, I see that there is a lot of functionality that could be moved
from perf to libperf and which will be useful for the library users.
We are going to use libperf in trace-cruncher,
https://github.com/vmware/trace-cruncher, as an interface to perf.

> Thanks!
>
> - Arnaldo
>
> >
> >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
> >>



-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  2022-02-22  2:32     ` Tzvetomir Stoyanov
@ 2022-02-23  0:21       ` Arnaldo Carvalho de Melo
  2022-02-23  9:08         ` Tzvetomir Stoyanov
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-02-23  0:21 UTC (permalink / raw)
  To: Tzvetomir Stoyanov
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ian Rogers, linux-perf-users

Em Tue, Feb 22, 2022 at 04:32:05AM +0200, Tzvetomir Stoyanov escreveu:
> On Mon, Feb 21, 2022 at 10:21 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > On February 21, 2022 4:46:49 PM GMT-03:00, Jiri Olsa <olsajiri@gmail.com> wrote:
> > >On Mon, Feb 21, 2022 at 12:26:28PM +0200, Tzvetomir Stoyanov (VMware) wrote:
> > >looks good, would you also find useful in your use case the comm support
> > >in thread_map? the thread_map__read_comms function? we could move it from
> > >perf to libperf

> > IOW: we're happy that you're working on libperf, so feel encouraged
> > to move things from tools/perf/util/ to tools/lib/perf/ if you find
> > a supporting use case, brownie points if you also add an entry to
> > tools/perf/tests/, AKA 'perf test'.

> Thanks, I see that there is a lot of functionality that could be moved
> from perf to libperf and which will be useful for the library users.
> We are going to use libperf in trace-cruncher,
> https://github.com/vmware/trace-cruncher, as an interface to perf.

Cool, so as you go on adding functionality to trace cruncher and notice
that something that is in tools/perf/util/ that is usable, please submit
patches to move things to tools/lib/perf/, adding tests to 'perf test'
as you go.

Thanks a lot!

- Arnaldo

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  2022-02-21 19:46 ` Jiri Olsa
  2022-02-21 20:07   ` Arnaldo Carvalho de Melo
  2022-02-22  2:21   ` Tzvetomir Stoyanov
@ 2022-02-23  0:22   ` Arnaldo Carvalho de Melo
  2022-02-23 13:31     ` Jiri Olsa
  2 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-02-23  0:22 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Tzvetomir Stoyanov (VMware), irogers, linux-perf-users

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? :-)

- 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  2022-02-23  0:21       ` Arnaldo Carvalho de Melo
@ 2022-02-23  9:08         ` Tzvetomir Stoyanov
  2022-02-23 13:02           ` Jiri Olsa
  0 siblings, 1 reply; 15+ messages in thread
From: Tzvetomir Stoyanov @ 2022-02-23  9:08 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, Ian Rogers, linux-perf-users

On Wed, Feb 23, 2022 at 2:21 AM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Tue, Feb 22, 2022 at 04:32:05AM +0200, Tzvetomir Stoyanov escreveu:
> > On Mon, Feb 21, 2022 at 10:21 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > > On February 21, 2022 4:46:49 PM GMT-03:00, Jiri Olsa <olsajiri@gmail.com> wrote:
> > > >On Mon, Feb 21, 2022 at 12:26:28PM +0200, Tzvetomir Stoyanov (VMware) wrote:
> > > >looks good, would you also find useful in your use case the comm support
> > > >in thread_map? the thread_map__read_comms function? we could move it from
> > > >perf to libperf
>
> > > IOW: we're happy that you're working on libperf, so feel encouraged
> > > to move things from tools/perf/util/ to tools/lib/perf/ if you find
> > > a supporting use case, brownie points if you also add an entry to
> > > tools/perf/tests/, AKA 'perf test'.
>
> > Thanks, I see that there is a lot of functionality that could be moved
> > from perf to libperf and which will be useful for the library users.
> > We are going to use libperf in trace-cruncher,
> > https://github.com/vmware/trace-cruncher, as an interface to perf.
>
> Cool, so as you go on adding functionality to trace cruncher and notice
> that something that is in tools/perf/util/ that is usable, please submit
> patches to move things to tools/lib/perf/, adding tests to 'perf test'
> as you go.
>
> Thanks a lot!
>
> - Arnaldo

Sure, I'll do it. I have one more question - do you plan to release
the next version of libperf soon, so we can use this new functionality
in trace-cruncher ?

-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  2022-02-23  9:08         ` Tzvetomir Stoyanov
@ 2022-02-23 13:02           ` Jiri Olsa
  2022-02-23 15:47             ` Tzvetomir Stoyanov
  0 siblings, 1 reply; 15+ messages in thread
From: Jiri Olsa @ 2022-02-23 13:02 UTC (permalink / raw)
  To: Tzvetomir Stoyanov; +Cc: Arnaldo Carvalho de Melo, Ian Rogers, linux-perf-users

On Wed, Feb 23, 2022 at 11:08:24AM +0200, Tzvetomir Stoyanov wrote:
> On Wed, Feb 23, 2022 at 2:21 AM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Tue, Feb 22, 2022 at 04:32:05AM +0200, Tzvetomir Stoyanov escreveu:
> > > On Mon, Feb 21, 2022 at 10:21 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > > > On February 21, 2022 4:46:49 PM GMT-03:00, Jiri Olsa <olsajiri@gmail.com> wrote:
> > > > >On Mon, Feb 21, 2022 at 12:26:28PM +0200, Tzvetomir Stoyanov (VMware) wrote:
> > > > >looks good, would you also find useful in your use case the comm support
> > > > >in thread_map? the thread_map__read_comms function? we could move it from
> > > > >perf to libperf
> >
> > > > IOW: we're happy that you're working on libperf, so feel encouraged
> > > > to move things from tools/perf/util/ to tools/lib/perf/ if you find
> > > > a supporting use case, brownie points if you also add an entry to
> > > > tools/perf/tests/, AKA 'perf test'.
> >
> > > Thanks, I see that there is a lot of functionality that could be moved
> > > from perf to libperf and which will be useful for the library users.
> > > We are going to use libperf in trace-cruncher,
> > > https://github.com/vmware/trace-cruncher, as an interface to perf.
> >
> > Cool, so as you go on adding functionality to trace cruncher and notice
> > that something that is in tools/perf/util/ that is usable, please submit
> > patches to move things to tools/lib/perf/, adding tests to 'perf test'
> > as you go.
> >
> > Thanks a lot!
> >
> > - Arnaldo
> 
> Sure, I'll do it. I have one more question - do you plan to release
> the next version of libperf soon, so we can use this new functionality
> in trace-cruncher ?

we can discuss that, can be fast ;-)

btw I took a look on that and already put some change together,
please take a look and feel free to use it

I just moved thread_map__read_comms, but I wonder we want it
to change to return status if it's in libperf

jirka


---
 tools/lib/perf/Build                    |  7 ++-
 tools/lib/perf/Makefile                 |  1 +
 tools/lib/perf/include/perf/threadmap.h |  2 +
 tools/lib/perf/libperf.map              |  5 +++
 tools/lib/perf/threadmap.c              | 57 +++++++++++++++++++++++++
 tools/perf/builtin-stat.c               |  2 +-
 tools/perf/tests/thread-map.c           |  6 +--
 tools/perf/util/Build                   |  6 ---
 tools/perf/util/python-ext-sources      |  1 -
 tools/perf/util/thread_map.c            | 53 -----------------------
 tools/perf/util/thread_map.h            |  1 -
 11 files changed, 75 insertions(+), 66 deletions(-)

diff --git a/tools/lib/perf/Build b/tools/lib/perf/Build
index e8f5b7fb9973..6d3f63793a51 100644
--- a/tools/lib/perf/Build
+++ b/tools/lib/perf/Build
@@ -7,8 +7,13 @@ libperf-y += mmap.o
 libperf-y += zalloc.o
 libperf-y += xyarray.o
 libperf-y += lib.o
+libperf-y += string.o
 
-$(OUTPUT)zalloc.o: ../../lib/zalloc.c FORCE
+$(OUTPUT)zalloc.o: ../zalloc.c FORCE
+	$(call rule_mkdir)
+	$(call if_changed_dep,cc_o_c)
+
+$(OUTPUT)string.o: ../string.c FORCE
 	$(call rule_mkdir)
 	$(call if_changed_dep,cc_o_c)
 
diff --git a/tools/lib/perf/Makefile b/tools/lib/perf/Makefile
index 08fe6e3c4089..81b495af3cf1 100644
--- a/tools/lib/perf/Makefile
+++ b/tools/lib/perf/Makefile
@@ -75,6 +75,7 @@ override CFLAGS += -Werror -Wall
 override CFLAGS += -fPIC
 override CFLAGS += $(INCLUDES)
 override CFLAGS += -fvisibility=hidden
+override CFLAGS += -D_GNU_SOURCE
 
 all:
 
diff --git a/tools/lib/perf/include/perf/threadmap.h b/tools/lib/perf/include/perf/threadmap.h
index a7c50de8d010..6301f0db1db4 100644
--- a/tools/lib/perf/include/perf/threadmap.h
+++ b/tools/lib/perf/include/perf/threadmap.h
@@ -17,4 +17,6 @@ LIBPERF_API pid_t perf_thread_map__pid(struct perf_thread_map *map, int thread);
 LIBPERF_API struct perf_thread_map *perf_thread_map__get(struct perf_thread_map *map);
 LIBPERF_API void perf_thread_map__put(struct perf_thread_map *map);
 
+LIBPERF_API void perf_thread_map__read_comms(struct perf_thread_map *map);
+
 #endif /* __LIBPERF_THREADMAP_H */
diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
index 6fa0d651576b..4345f7b9b0c5 100644
--- a/tools/lib/perf/libperf.map
+++ b/tools/lib/perf/libperf.map
@@ -56,3 +56,8 @@ LIBPERF_0.0.1 {
 	local:
 		*;
 };
+
+LIBPERF_0.0.2 {
+	global:
+		perf_thread_map__read_comms;
+} LIBPERF_0.0.1;
diff --git a/tools/lib/perf/threadmap.c b/tools/lib/perf/threadmap.c
index e92c368b0a6c..f5ad6d939365 100644
--- a/tools/lib/perf/threadmap.c
+++ b/tools/lib/perf/threadmap.c
@@ -6,6 +6,10 @@
 #include <string.h>
 #include <asm/bug.h>
 #include <stdio.h>
+#include <errno.h>
+#include <api/fs/fs.h>
+#include <linux/string.h>
+#include "internal.h"
 
 static void perf_thread_map__reset(struct perf_thread_map *map, int start, int nr)
 {
@@ -89,3 +93,56 @@ pid_t perf_thread_map__pid(struct perf_thread_map *map, int thread)
 {
 	return map->map[thread].pid;
 }
+
+static int get_comm(char **comm, pid_t pid)
+{
+	char *path;
+	size_t size;
+	int err;
+
+	if (asprintf(&path, "%s/%d/comm", procfs__mountpoint(), pid) == -1)
+		return -ENOMEM;
+
+	err = filename__read_str(path, comm, &size);
+	if (!err) {
+		/*
+		 * We're reading 16 bytes, while filename__read_str
+		 * allocates data per BUFSIZ bytes, so we can safely
+		 * mark the end of the string.
+		 */
+		(*comm)[size] = 0;
+		strim(*comm);
+	}
+
+	free(path);
+	return err;
+}
+
+static void comm_init(struct perf_thread_map *map, int i)
+{
+	pid_t pid = perf_thread_map__pid(map, i);
+	char *comm = NULL;
+
+	/* dummy pid comm initialization */
+	if (pid == -1) {
+		map->map[i].comm = strdup("dummy");
+		return;
+	}
+
+	/*
+	 * The comm name is like extra bonus ;-),
+	 * so just warn if we fail for any reason.
+	 */
+	if (get_comm(&comm, pid))
+		pr_warning("Couldn't resolve comm name for pid %d\n", pid);
+
+	map->map[i].comm = comm;
+}
+
+void perf_thread_map__read_comms(struct perf_thread_map *threads)
+{
+	int i;
+
+	for (i = 0; i < threads->nr; ++i)
+		comm_init(threads, i);
+}
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 3f98689dd687..8456ad3e73b8 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2477,7 +2477,7 @@ int cmd_stat(int argc, const char **argv)
 	 * so we could print it out on output.
 	 */
 	if (stat_config.aggr_mode == AGGR_THREAD) {
-		thread_map__read_comms(evsel_list->core.threads);
+		perf_thread_map__read_comms(evsel_list->core.threads);
 		if (target.system_wide) {
 			if (runtime_stat_new(&stat_config,
 				perf_thread_map__nr(evsel_list->core.threads))) {
diff --git a/tools/perf/tests/thread-map.c b/tools/perf/tests/thread-map.c
index e413c1387fcb..00180b56948e 100644
--- a/tools/perf/tests/thread-map.c
+++ b/tools/perf/tests/thread-map.c
@@ -30,7 +30,7 @@ static int test__thread_map(struct test_suite *test __maybe_unused, int subtest
 	map = thread_map__new_by_pid(getpid());
 	TEST_ASSERT_VAL("failed to alloc map", map);
 
-	thread_map__read_comms(map);
+	perf_thread_map__read_comms(map);
 
 	TEST_ASSERT_VAL("wrong nr", map->nr == 1);
 	TEST_ASSERT_VAL("wrong pid",
@@ -46,7 +46,7 @@ static int test__thread_map(struct test_suite *test __maybe_unused, int subtest
 	map = perf_thread_map__new_dummy();
 	TEST_ASSERT_VAL("failed to alloc map", map);
 
-	thread_map__read_comms(map);
+	perf_thread_map__read_comms(map);
 
 	TEST_ASSERT_VAL("wrong nr", map->nr == 1);
 	TEST_ASSERT_VAL("wrong pid", perf_thread_map__pid(map, 0) == -1);
@@ -97,7 +97,7 @@ static int test__thread_map_synthesize(struct test_suite *test __maybe_unused, i
 	threads = thread_map__new_by_pid(getpid());
 	TEST_ASSERT_VAL("failed to alloc map", threads);
 
-	thread_map__read_comms(threads);
+	perf_thread_map__read_comms(threads);
 
 	TEST_ASSERT_VAL("failed to synthesize map",
 		!perf_event__synthesize_thread_map2(NULL, threads, process_event, NULL));
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 9a7209a99e16..285b466ac1c3 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -32,7 +32,6 @@ perf-y += print_binary.o
 perf-y += rlimit.o
 perf-y += argv_split.o
 perf-y += rbtree.o
-perf-y += libstring.o
 perf-y += bitmap.o
 perf-y += hweight.o
 perf-y += smt.o
@@ -279,7 +278,6 @@ $(OUTPUT)util/expr.o: $(OUTPUT)util/expr-flex.c $(OUTPUT)util/expr-bison.c
 CFLAGS_bitmap.o        += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
 CFLAGS_find_bit.o      += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
 CFLAGS_rbtree.o        += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
-CFLAGS_libstring.o     += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
 CFLAGS_hweight.o       += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
 CFLAGS_parse-events.o  += -Wno-redundant-decls
 CFLAGS_expr.o          += -Wno-redundant-decls
@@ -309,10 +307,6 @@ $(OUTPUT)util/rbtree.o: ../lib/rbtree.c FORCE
 	$(call rule_mkdir)
 	$(call if_changed_dep,cc_o_c)
 
-$(OUTPUT)util/libstring.o: ../lib/string.c FORCE
-	$(call rule_mkdir)
-	$(call if_changed_dep,cc_o_c)
-
 $(OUTPUT)util/hweight.o: ../lib/hweight.c FORCE
 	$(call rule_mkdir)
 	$(call if_changed_dep,cc_o_c)
diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
index a685d20165f7..b4cbd727c8b1 100644
--- a/tools/perf/util/python-ext-sources
+++ b/tools/perf/util/python-ext-sources
@@ -20,7 +20,6 @@ util/namespaces.c
 ../lib/find_bit.c
 ../lib/list_sort.c
 ../lib/hweight.c
-../lib/string.c
 ../lib/vsprintf.c
 util/thread_map.c
 util/util.c
diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
index c9bfe4696943..7dff6e315aa9 100644
--- a/tools/perf/util/thread_map.c
+++ b/tools/perf/util/thread_map.c
@@ -315,59 +315,6 @@ size_t thread_map__fprintf(struct perf_thread_map *threads, FILE *fp)
 	return printed + fprintf(fp, "\n");
 }
 
-static int get_comm(char **comm, pid_t pid)
-{
-	char *path;
-	size_t size;
-	int err;
-
-	if (asprintf(&path, "%s/%d/comm", procfs__mountpoint(), pid) == -1)
-		return -ENOMEM;
-
-	err = filename__read_str(path, comm, &size);
-	if (!err) {
-		/*
-		 * We're reading 16 bytes, while filename__read_str
-		 * allocates data per BUFSIZ bytes, so we can safely
-		 * mark the end of the string.
-		 */
-		(*comm)[size] = 0;
-		strim(*comm);
-	}
-
-	free(path);
-	return err;
-}
-
-static void comm_init(struct perf_thread_map *map, int i)
-{
-	pid_t pid = perf_thread_map__pid(map, i);
-	char *comm = NULL;
-
-	/* dummy pid comm initialization */
-	if (pid == -1) {
-		map->map[i].comm = strdup("dummy");
-		return;
-	}
-
-	/*
-	 * The comm name is like extra bonus ;-),
-	 * so just warn if we fail for any reason.
-	 */
-	if (get_comm(&comm, pid))
-		pr_warning("Couldn't resolve comm name for pid %d\n", pid);
-
-	map->map[i].comm = comm;
-}
-
-void thread_map__read_comms(struct perf_thread_map *threads)
-{
-	int i;
-
-	for (i = 0; i < threads->nr; ++i)
-		comm_init(threads, i);
-}
-
 static void thread_map__copy_event(struct perf_thread_map *threads,
 				   struct perf_record_thread_map *event)
 {
diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h
index 3bb860a32b8e..1f84edd38b17 100644
--- a/tools/perf/util/thread_map.h
+++ b/tools/perf/util/thread_map.h
@@ -25,7 +25,6 @@ struct perf_thread_map *thread_map__new_by_tid_str(const char *tid_str);
 
 size_t thread_map__fprintf(struct perf_thread_map *threads, FILE *fp);
 
-void thread_map__read_comms(struct perf_thread_map *threads);
 bool thread_map__has(struct perf_thread_map *threads, pid_t pid);
 int thread_map__remove(struct perf_thread_map *threads, int idx);
 #endif	/* __PERF_THREAD_MAP_H */
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  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
  0 siblings, 1 reply; 15+ messages in thread
From: Jiri Olsa @ 2022-02-23 13:31 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Tzvetomir Stoyanov (VMware), irogers, linux-perf-users

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>

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  2022-02-23 13:31     ` Jiri Olsa
@ 2022-02-23 15:30       ` Arnaldo Carvalho de Melo
  2022-03-21 11:10         ` Tzvetomir Stoyanov
  0 siblings, 1 reply; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-02-23 15:30 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Tzvetomir Stoyanov (VMware),
	irogers, linux-perf-users

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
 
> 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  2022-02-23 13:02           ` Jiri Olsa
@ 2022-02-23 15:47             ` Tzvetomir Stoyanov
  2022-02-23 18:06               ` Jiri Olsa
  0 siblings, 1 reply; 15+ messages in thread
From: Tzvetomir Stoyanov @ 2022-02-23 15:47 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: Arnaldo Carvalho de Melo, Ian Rogers, linux-perf-users

On Wed, Feb 23, 2022 at 3:03 PM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Feb 23, 2022 at 11:08:24AM +0200, Tzvetomir Stoyanov wrote:
> > On Wed, Feb 23, 2022 at 2:21 AM Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > >
> > > Em Tue, Feb 22, 2022 at 04:32:05AM +0200, Tzvetomir Stoyanov escreveu:
> > > > On Mon, Feb 21, 2022 at 10:21 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > > > > On February 21, 2022 4:46:49 PM GMT-03:00, Jiri Olsa <olsajiri@gmail.com> wrote:
> > > > > >On Mon, Feb 21, 2022 at 12:26:28PM +0200, Tzvetomir Stoyanov (VMware) wrote:
> > > > > >looks good, would you also find useful in your use case the comm support
> > > > > >in thread_map? the thread_map__read_comms function? we could move it from
> > > > > >perf to libperf
> > >
> > > > > IOW: we're happy that you're working on libperf, so feel encouraged
> > > > > to move things from tools/perf/util/ to tools/lib/perf/ if you find
> > > > > a supporting use case, brownie points if you also add an entry to
> > > > > tools/perf/tests/, AKA 'perf test'.
> > >
> > > > Thanks, I see that there is a lot of functionality that could be moved
> > > > from perf to libperf and which will be useful for the library users.
> > > > We are going to use libperf in trace-cruncher,
> > > > https://github.com/vmware/trace-cruncher, as an interface to perf.
> > >
> > > Cool, so as you go on adding functionality to trace cruncher and notice
> > > that something that is in tools/perf/util/ that is usable, please submit
> > > patches to move things to tools/lib/perf/, adding tests to 'perf test'
> > > as you go.
> > >
> > > Thanks a lot!
> > >
> > > - Arnaldo
> >
> > Sure, I'll do it. I have one more question - do you plan to release
> > the next version of libperf soon, so we can use this new functionality
> > in trace-cruncher ?
>
> we can discuss that, can be fast ;-)
>
> btw I took a look on that and already put some change together,
> please take a look and feel free to use it
>
> I just moved thread_map__read_comms, but I wonder we want it
> to change to return status if it's in libperf
>
> jirka
>

Thanks Jiri, I played a little with the patch but it does not fit my
use case. It resolves only PID to command, but in my case I put
threads in the map. How do you handle that case in perf ? I use perf
to collect samplings from a given PID, and put all threads of this
process in the map, all entries from /proc/PID/task/ folder. In my
logic, as I know the PID, the commands are resolved using
/proc/PID/task/TID/comm files.
The other problem that I see is - there should be a way to dynamically
resize and add new entries in the thread map. There are cases when new
threads are spawned during the trace - these should be added in the
thread map. In my code, I handle the PERF_RECORD_COMM event - resize
my array and add the new entry. That can be implemented in libperf
also, I think it will be very useful.
Anyway, the patch is useful for PID to command resolving, if there are
only PIDs in the map. It could be extended with additional APIs for
TID to command resolving. Maybe we can have these APIs:
   void perf_thread_map__read_pid_comms(struct perf_thread_map *threads);
   void perf_thread_map__read_tid_comms(pid_t pid, struct
perf_thread_map *threads);

>
> ---
>  tools/lib/perf/Build                    |  7 ++-
>  tools/lib/perf/Makefile                 |  1 +
>  tools/lib/perf/include/perf/threadmap.h |  2 +
>  tools/lib/perf/libperf.map              |  5 +++
>  tools/lib/perf/threadmap.c              | 57 +++++++++++++++++++++++++
>  tools/perf/builtin-stat.c               |  2 +-
>  tools/perf/tests/thread-map.c           |  6 +--
>  tools/perf/util/Build                   |  6 ---
>  tools/perf/util/python-ext-sources      |  1 -
>  tools/perf/util/thread_map.c            | 53 -----------------------
>  tools/perf/util/thread_map.h            |  1 -
>  11 files changed, 75 insertions(+), 66 deletions(-)
>
> diff --git a/tools/lib/perf/Build b/tools/lib/perf/Build
> index e8f5b7fb9973..6d3f63793a51 100644
> --- a/tools/lib/perf/Build
> +++ b/tools/lib/perf/Build
> @@ -7,8 +7,13 @@ libperf-y += mmap.o
>  libperf-y += zalloc.o
>  libperf-y += xyarray.o
>  libperf-y += lib.o
> +libperf-y += string.o

Looks like ctype is required also:
 libperf-y += ctype.o

>
> -$(OUTPUT)zalloc.o: ../../lib/zalloc.c FORCE
> +$(OUTPUT)zalloc.o: ../zalloc.c FORCE
> +       $(call rule_mkdir)
> +       $(call if_changed_dep,cc_o_c)
> +
> +$(OUTPUT)string.o: ../string.c FORCE
>         $(call rule_mkdir)
>         $(call if_changed_dep,cc_o_c)

 +$(OUTPUT)ctype.o: ../ctype.c FORCE
         $(call rule_mkdir)
         $(call if_changed_dep,cc_o_c)

>
> diff --git a/tools/lib/perf/Makefile b/tools/lib/perf/Makefile
> index 08fe6e3c4089..81b495af3cf1 100644
> --- a/tools/lib/perf/Makefile
> +++ b/tools/lib/perf/Makefile
> @@ -75,6 +75,7 @@ override CFLAGS += -Werror -Wall
>  override CFLAGS += -fPIC
>  override CFLAGS += $(INCLUDES)
>  override CFLAGS += -fvisibility=hidden
> +override CFLAGS += -D_GNU_SOURCE
>
>  all:
>
> diff --git a/tools/lib/perf/include/perf/threadmap.h b/tools/lib/perf/include/perf/threadmap.h
> index a7c50de8d010..6301f0db1db4 100644
> --- a/tools/lib/perf/include/perf/threadmap.h
> +++ b/tools/lib/perf/include/perf/threadmap.h
> @@ -17,4 +17,6 @@ LIBPERF_API pid_t perf_thread_map__pid(struct perf_thread_map *map, int thread);
>  LIBPERF_API struct perf_thread_map *perf_thread_map__get(struct perf_thread_map *map);
>  LIBPERF_API void perf_thread_map__put(struct perf_thread_map *map);
>
> +LIBPERF_API void perf_thread_map__read_comms(struct perf_thread_map *map);
> +
>  #endif /* __LIBPERF_THREADMAP_H */
> diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
> index 6fa0d651576b..4345f7b9b0c5 100644
> --- a/tools/lib/perf/libperf.map
> +++ b/tools/lib/perf/libperf.map
> @@ -56,3 +56,8 @@ LIBPERF_0.0.1 {
>         local:
>                 *;
>  };
> +
> +LIBPERF_0.0.2 {
> +       global:
> +               perf_thread_map__read_comms;
> +} LIBPERF_0.0.1;
> diff --git a/tools/lib/perf/threadmap.c b/tools/lib/perf/threadmap.c
> index e92c368b0a6c..f5ad6d939365 100644
> --- a/tools/lib/perf/threadmap.c
> +++ b/tools/lib/perf/threadmap.c
> @@ -6,6 +6,10 @@
>  #include <string.h>
>  #include <asm/bug.h>
>  #include <stdio.h>
> +#include <errno.h>
> +#include <api/fs/fs.h>
> +#include <linux/string.h>
> +#include "internal.h"
>
>  static void perf_thread_map__reset(struct perf_thread_map *map, int start, int nr)
>  {
> @@ -89,3 +93,56 @@ pid_t perf_thread_map__pid(struct perf_thread_map *map, int thread)
>  {
>         return map->map[thread].pid;
>  }
> +
> +static int get_comm(char **comm, pid_t pid)
> +{
> +       char *path;
> +       size_t size;
> +       int err;
> +
> +       if (asprintf(&path, "%s/%d/comm", procfs__mountpoint(), pid) == -1)
> +               return -ENOMEM;
> +
> +       err = filename__read_str(path, comm, &size);
> +       if (!err) {
> +               /*
> +                * We're reading 16 bytes, while filename__read_str
> +                * allocates data per BUFSIZ bytes, so we can safely
> +                * mark the end of the string.
> +                */
> +               (*comm)[size] = 0;
> +               strim(*comm);
> +       }
> +
> +       free(path);
> +       return err;
> +}
> +
> +static void comm_init(struct perf_thread_map *map, int i)
> +{
> +       pid_t pid = perf_thread_map__pid(map, i);
> +       char *comm = NULL;
> +
> +       /* dummy pid comm initialization */
> +       if (pid == -1) {
> +               map->map[i].comm = strdup("dummy");
> +               return;
> +       }
> +
> +       /*
> +        * The comm name is like extra bonus ;-),
> +        * so just warn if we fail for any reason.
> +        */
> +       if (get_comm(&comm, pid))
> +               pr_warning("Couldn't resolve comm name for pid %d\n", pid);
> +
> +       map->map[i].comm = comm;
> +}
> +
> +void perf_thread_map__read_comms(struct perf_thread_map *threads)
> +{
> +       int i;
> +
> +       for (i = 0; i < threads->nr; ++i)
> +               comm_init(threads, i);
> +}
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 3f98689dd687..8456ad3e73b8 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2477,7 +2477,7 @@ int cmd_stat(int argc, const char **argv)
>          * so we could print it out on output.
>          */
>         if (stat_config.aggr_mode == AGGR_THREAD) {
> -               thread_map__read_comms(evsel_list->core.threads);
> +               perf_thread_map__read_comms(evsel_list->core.threads);
>                 if (target.system_wide) {
>                         if (runtime_stat_new(&stat_config,
>                                 perf_thread_map__nr(evsel_list->core.threads))) {
> diff --git a/tools/perf/tests/thread-map.c b/tools/perf/tests/thread-map.c
> index e413c1387fcb..00180b56948e 100644
> --- a/tools/perf/tests/thread-map.c
> +++ b/tools/perf/tests/thread-map.c
> @@ -30,7 +30,7 @@ static int test__thread_map(struct test_suite *test __maybe_unused, int subtest
>         map = thread_map__new_by_pid(getpid());
>         TEST_ASSERT_VAL("failed to alloc map", map);
>
> -       thread_map__read_comms(map);
> +       perf_thread_map__read_comms(map);
>
>         TEST_ASSERT_VAL("wrong nr", map->nr == 1);
>         TEST_ASSERT_VAL("wrong pid",
> @@ -46,7 +46,7 @@ static int test__thread_map(struct test_suite *test __maybe_unused, int subtest
>         map = perf_thread_map__new_dummy();
>         TEST_ASSERT_VAL("failed to alloc map", map);
>
> -       thread_map__read_comms(map);
> +       perf_thread_map__read_comms(map);
>
>         TEST_ASSERT_VAL("wrong nr", map->nr == 1);
>         TEST_ASSERT_VAL("wrong pid", perf_thread_map__pid(map, 0) == -1);
> @@ -97,7 +97,7 @@ static int test__thread_map_synthesize(struct test_suite *test __maybe_unused, i
>         threads = thread_map__new_by_pid(getpid());
>         TEST_ASSERT_VAL("failed to alloc map", threads);
>
> -       thread_map__read_comms(threads);
> +       perf_thread_map__read_comms(threads);
>
>         TEST_ASSERT_VAL("failed to synthesize map",
>                 !perf_event__synthesize_thread_map2(NULL, threads, process_event, NULL));
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 9a7209a99e16..285b466ac1c3 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -32,7 +32,6 @@ perf-y += print_binary.o
>  perf-y += rlimit.o
>  perf-y += argv_split.o
>  perf-y += rbtree.o
> -perf-y += libstring.o
>  perf-y += bitmap.o
>  perf-y += hweight.o
>  perf-y += smt.o
> @@ -279,7 +278,6 @@ $(OUTPUT)util/expr.o: $(OUTPUT)util/expr-flex.c $(OUTPUT)util/expr-bison.c
>  CFLAGS_bitmap.o        += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
>  CFLAGS_find_bit.o      += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
>  CFLAGS_rbtree.o        += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
> -CFLAGS_libstring.o     += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
>  CFLAGS_hweight.o       += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
>  CFLAGS_parse-events.o  += -Wno-redundant-decls
>  CFLAGS_expr.o          += -Wno-redundant-decls
> @@ -309,10 +307,6 @@ $(OUTPUT)util/rbtree.o: ../lib/rbtree.c FORCE
>         $(call rule_mkdir)
>         $(call if_changed_dep,cc_o_c)
>
> -$(OUTPUT)util/libstring.o: ../lib/string.c FORCE
> -       $(call rule_mkdir)
> -       $(call if_changed_dep,cc_o_c)
> -
>  $(OUTPUT)util/hweight.o: ../lib/hweight.c FORCE
>         $(call rule_mkdir)
>         $(call if_changed_dep,cc_o_c)
> diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
> index a685d20165f7..b4cbd727c8b1 100644
> --- a/tools/perf/util/python-ext-sources
> +++ b/tools/perf/util/python-ext-sources
> @@ -20,7 +20,6 @@ util/namespaces.c
>  ../lib/find_bit.c
>  ../lib/list_sort.c
>  ../lib/hweight.c
> -../lib/string.c
>  ../lib/vsprintf.c
>  util/thread_map.c
>  util/util.c
> diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
> index c9bfe4696943..7dff6e315aa9 100644
> --- a/tools/perf/util/thread_map.c
> +++ b/tools/perf/util/thread_map.c
> @@ -315,59 +315,6 @@ size_t thread_map__fprintf(struct perf_thread_map *threads, FILE *fp)
>         return printed + fprintf(fp, "\n");
>  }
>
> -static int get_comm(char **comm, pid_t pid)
> -{
> -       char *path;
> -       size_t size;
> -       int err;
> -
> -       if (asprintf(&path, "%s/%d/comm", procfs__mountpoint(), pid) == -1)
> -               return -ENOMEM;
> -
> -       err = filename__read_str(path, comm, &size);
> -       if (!err) {
> -               /*
> -                * We're reading 16 bytes, while filename__read_str
> -                * allocates data per BUFSIZ bytes, so we can safely
> -                * mark the end of the string.
> -                */
> -               (*comm)[size] = 0;
> -               strim(*comm);
> -       }
> -
> -       free(path);
> -       return err;
> -}
> -
> -static void comm_init(struct perf_thread_map *map, int i)
> -{
> -       pid_t pid = perf_thread_map__pid(map, i);
> -       char *comm = NULL;
> -
> -       /* dummy pid comm initialization */
> -       if (pid == -1) {
> -               map->map[i].comm = strdup("dummy");
> -               return;
> -       }
> -
> -       /*
> -        * The comm name is like extra bonus ;-),
> -        * so just warn if we fail for any reason.
> -        */
> -       if (get_comm(&comm, pid))
> -               pr_warning("Couldn't resolve comm name for pid %d\n", pid);
> -
> -       map->map[i].comm = comm;
> -}
> -
> -void thread_map__read_comms(struct perf_thread_map *threads)
> -{
> -       int i;
> -
> -       for (i = 0; i < threads->nr; ++i)
> -               comm_init(threads, i);
> -}
> -
>  static void thread_map__copy_event(struct perf_thread_map *threads,
>                                    struct perf_record_thread_map *event)
>  {
> diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h
> index 3bb860a32b8e..1f84edd38b17 100644
> --- a/tools/perf/util/thread_map.h
> +++ b/tools/perf/util/thread_map.h
> @@ -25,7 +25,6 @@ struct perf_thread_map *thread_map__new_by_tid_str(const char *tid_str);
>
>  size_t thread_map__fprintf(struct perf_thread_map *threads, FILE *fp);
>
> -void thread_map__read_comms(struct perf_thread_map *threads);
>  bool thread_map__has(struct perf_thread_map *threads, pid_t pid);
>  int thread_map__remove(struct perf_thread_map *threads, int idx);
>  #endif /* __PERF_THREAD_MAP_H */
> --
> 2.35.1
>


-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  2022-02-23 15:47             ` Tzvetomir Stoyanov
@ 2022-02-23 18:06               ` Jiri Olsa
  0 siblings, 0 replies; 15+ messages in thread
From: Jiri Olsa @ 2022-02-23 18:06 UTC (permalink / raw)
  To: Tzvetomir Stoyanov; +Cc: Arnaldo Carvalho de Melo, Ian Rogers, linux-perf-users

On Wed, Feb 23, 2022 at 05:47:09PM +0200, Tzvetomir Stoyanov wrote:
> On Wed, Feb 23, 2022 at 3:03 PM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Wed, Feb 23, 2022 at 11:08:24AM +0200, Tzvetomir Stoyanov wrote:
> > > On Wed, Feb 23, 2022 at 2:21 AM Arnaldo Carvalho de Melo
> > > <arnaldo.melo@gmail.com> wrote:
> > > >
> > > > Em Tue, Feb 22, 2022 at 04:32:05AM +0200, Tzvetomir Stoyanov escreveu:
> > > > > On Mon, Feb 21, 2022 at 10:21 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > > > > > On February 21, 2022 4:46:49 PM GMT-03:00, Jiri Olsa <olsajiri@gmail.com> wrote:
> > > > > > >On Mon, Feb 21, 2022 at 12:26:28PM +0200, Tzvetomir Stoyanov (VMware) wrote:
> > > > > > >looks good, would you also find useful in your use case the comm support
> > > > > > >in thread_map? the thread_map__read_comms function? we could move it from
> > > > > > >perf to libperf
> > > >
> > > > > > IOW: we're happy that you're working on libperf, so feel encouraged
> > > > > > to move things from tools/perf/util/ to tools/lib/perf/ if you find
> > > > > > a supporting use case, brownie points if you also add an entry to
> > > > > > tools/perf/tests/, AKA 'perf test'.
> > > >
> > > > > Thanks, I see that there is a lot of functionality that could be moved
> > > > > from perf to libperf and which will be useful for the library users.
> > > > > We are going to use libperf in trace-cruncher,
> > > > > https://github.com/vmware/trace-cruncher, as an interface to perf.
> > > >
> > > > Cool, so as you go on adding functionality to trace cruncher and notice
> > > > that something that is in tools/perf/util/ that is usable, please submit
> > > > patches to move things to tools/lib/perf/, adding tests to 'perf test'
> > > > as you go.
> > > >
> > > > Thanks a lot!
> > > >
> > > > - Arnaldo
> > >
> > > Sure, I'll do it. I have one more question - do you plan to release
> > > the next version of libperf soon, so we can use this new functionality
> > > in trace-cruncher ?
> >
> > we can discuss that, can be fast ;-)
> >
> > btw I took a look on that and already put some change together,
> > please take a look and feel free to use it
> >
> > I just moved thread_map__read_comms, but I wonder we want it
> > to change to return status if it's in libperf
> >
> > jirka
> >
> 
> Thanks Jiri, I played a little with the patch but it does not fit my
> use case. It resolves only PID to command, but in my case I put
> threads in the map. How do you handle that case in perf ? I use perf
> to collect samplings from a given PID, and put all threads of this
> process in the map, all entries from /proc/PID/task/ folder. In my
> logic, as I know the PID, the commands are resolved using
> /proc/PID/task/TID/comm files.
> The other problem that I see is - there should be a way to dynamically
> resize and add new entries in the thread map. There are cases when new
> threads are spawned during the trace - these should be added in the
> thread map. In my code, I handle the PERF_RECORD_COMM event - resize
> my array and add the new entry. That can be implemented in libperf
> also, I think it will be very useful.

there's perf_thread_map__realloc, but it's not exported

> Anyway, the patch is useful for PID to command resolving, if there are
> only PIDs in the map. It could be extended with additional APIs for
> TID to command resolving. Maybe we can have these APIs:
>    void perf_thread_map__read_pid_comms(struct perf_thread_map *threads);
>    void perf_thread_map__read_tid_comms(pid_t pid, struct
> perf_thread_map *threads);

so thread_map__read_comms just resolves everything that's in the map,
so if you have thread ids in there, they should get resolved

if you expected some other functionality, like unwind pid to threads,
the thread_map__new_by_pid_str function does that

jirka

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  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
  0 siblings, 1 reply; 15+ messages in thread
From: Tzvetomir Stoyanov @ 2022-03-21 11:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, Ian Rogers, linux-perf-users

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!


>
> > 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] libperf: Add API for allocating new thread map
  2022-03-21 11:10         ` Tzvetomir Stoyanov
@ 2022-03-21 21:10           ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 15+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-03-21 21:10 UTC (permalink / raw)
  To: Tzvetomir Stoyanov
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ian Rogers, linux-perf-users

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-03-21 21:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 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.