[PATCH v3 0/3] libperf: Unify scaling of counters obtained from perf_evsel__read()

[PATCH v3 0/3] libperf: Unify scaling of counters obtained from perf_evsel__read()

[Shunsuke Nakamura]

This patch series unifies the counters that can be obtained from
perf_evsel__read() to "no scaling".
The counter scaling will be done using a function moved from
tools/perf/util.

The first patch move perf_counts_values__scale from tools/perf/util
to tools/lib/perf so that it can be used with libperf.

The second patch removes the scaling process from
perf_mmap__read_self().

The third patch adds a verification test to make sure that it scales
correctly when multiplexed.

---
Previous version at:
https://lore.kernel.org/linux-perf-users/20210922101627.3396398-1-nakamura.shun@fujitsu.com/

Changes in v3:
 - Move scaling process from tools/perf/util to tools/lib/perf
 - Remove scaling process from perf_mmap__read_self()
 - Remove scaling process for perf_mmap__read_self
 - Remove test to verify that no division by zero occurs

Changes in v2:
 - Fix not to divide by zero when counter scaling
 - Add test to verify that no division by zero occurs


[1] https://github.com/deater/perf_event_tests/blob/master/tests/rdpmc/rdpmc_multiplexing.c


Shunsuke Nakamura (3):
  libperf: Move perf_counts_values__scale to tools/lib/perf
  libperf: Remove scaling process from perf_mmap__read_self()
  libperf tests: Add test_stat_multiplexing test

 tools/lib/perf/evsel.c              |  19 ++++
 tools/lib/perf/include/perf/evsel.h |   4 +
 tools/lib/perf/libperf.map          |   1 +
 tools/lib/perf/mmap.c               |   2 -
 tools/lib/perf/tests/test-evlist.c  | 157 ++++++++++++++++++++++++++++
 tools/perf/util/evsel.c             |  19 ----
 tools/perf/util/evsel.h             |   3 -
 7 files changed, 181 insertions(+), 24 deletions(-)

-- 
2.27.0

[PATCH v3 1/3] libperf: Move perf_counts_values__scale to tools/lib/perf

[Shunsuke Nakamura]

Move perf_counts_values__scale from tools/perf/util to tools/lib/perf
so that it can be used with libperf.

Signed-off-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
---
 tools/lib/perf/evsel.c              | 19 +++++++++++++++++++
 tools/lib/perf/include/perf/evsel.h |  4 ++++
 tools/lib/perf/libperf.map          |  1 +
 tools/perf/util/evsel.c             | 19 -------------------
 tools/perf/util/evsel.h             |  3 ---
 5 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index 8441e3e1aaac..5097aadea37a 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -431,3 +431,22 @@ void perf_evsel__free_id(struct perf_evsel *evsel)
 	zfree(&evsel->id);
 	evsel->ids = 0;
 }
+
+void perf_counts_values__scale(struct perf_counts_values *count,
+			       bool scale, s8 *pscaled)
+{
+	s8 scaled = 0;
+
+	if (scale) {
+		if (count->run == 0) {
+			scaled = -1;
+			count->val = 0;
+		} else if (count->run < count->ena) {
+			scaled = 1;
+			count->val = (u64)((double)count->val * count->ena / count->run);
+		}
+	}
+
+	if (pscaled)
+		*pscaled = scaled;
+}
diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
index 60eae25076d3..9013d73af22d 100644
--- a/tools/lib/perf/include/perf/evsel.h
+++ b/tools/lib/perf/include/perf/evsel.h
@@ -4,6 +4,8 @@
 
 #include <stdint.h>
 #include <perf/core.h>
+#include <stdbool.h>
+#include <linux/types.h>
 
 struct perf_evsel;
 struct perf_event_attr;
@@ -39,5 +41,7 @@ LIBPERF_API int perf_evsel__disable_cpu(struct perf_evsel *evsel, int cpu);
 LIBPERF_API struct perf_cpu_map *perf_evsel__cpus(struct perf_evsel *evsel);
 LIBPERF_API struct perf_thread_map *perf_evsel__threads(struct perf_evsel *evsel);
 LIBPERF_API struct perf_event_attr *perf_evsel__attr(struct perf_evsel *evsel);
+LIBPERF_API void perf_counts_values__scale(struct perf_counts_values *count,
+					   bool scale, s8 *pscaled);
 
 #endif /* __LIBPERF_EVSEL_H */
diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
index 71468606e8a7..5979bf92d98f 100644
--- a/tools/lib/perf/libperf.map
+++ b/tools/lib/perf/libperf.map
@@ -50,6 +50,7 @@ LIBPERF_0.0.1 {
 		perf_mmap__read_init;
 		perf_mmap__read_done;
 		perf_mmap__read_event;
+		perf_counts_values__scale;
 	local:
 		*;
 };
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index dbfeceb2546c..49e4d0bdd7cc 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1457,25 +1457,6 @@ void evsel__compute_deltas(struct evsel *evsel, int cpu, int thread,
 	count->run = count->run - tmp.run;
 }
 
-void perf_counts_values__scale(struct perf_counts_values *count,
-			       bool scale, s8 *pscaled)
-{
-	s8 scaled = 0;
-
-	if (scale) {
-		if (count->run == 0) {
-			scaled = -1;
-			count->val = 0;
-		} else if (count->run < count->ena) {
-			scaled = 1;
-			count->val = (u64)((double) count->val * count->ena / count->run);
-		}
-	}
-
-	if (pscaled)
-		*pscaled = scaled;
-}
-
 static int evsel__read_one(struct evsel *evsel, int cpu, int thread)
 {
 	struct perf_counts_values *count = perf_counts(evsel->counts, cpu, thread);
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 1f7edfa8568a..8a6a4182c5fd 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -190,9 +190,6 @@ static inline int evsel__nr_cpus(struct evsel *evsel)
 	return evsel__cpus(evsel)->nr;
 }
 
-void perf_counts_values__scale(struct perf_counts_values *count,
-			       bool scale, s8 *pscaled);
-
 void evsel__compute_deltas(struct evsel *evsel, int cpu, int thread,
 			   struct perf_counts_values *count);
 
-- 
2.27.0

[PATCH v3 2/3] libperf: Remove scaling process from perf_mmap__read_self()

[Shunsuke Nakamura]

Remove the scaling process from perf_mmap__read_self(), and unify the
counters that can be obtained from perf_evsel__read() to "no scaling".

Signed-off-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
---
 tools/lib/perf/mmap.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/lib/perf/mmap.c b/tools/lib/perf/mmap.c
index c89dfa5f67b3..aaa457904008 100644
--- a/tools/lib/perf/mmap.c
+++ b/tools/lib/perf/mmap.c
@@ -353,8 +353,6 @@ int perf_mmap__read_self(struct perf_mmap *map, struct perf_counts_values *count
 		count->ena += delta;
 		if (idx)
 			count->run += delta;
-
-		cnt = mul_u64_u64_div64(cnt, count->ena, count->run);
 	}
 
 	count->val = cnt;
-- 
2.27.0

[PATCH v3 3/3] libperf tests: Add test_stat_multiplexing test

[Shunsuke Nakamura]

Adds a test for a counter obtained using read() system call during
multiplexing.

Committer testing:

  $ sudo make tests -C ./tools/lib/perf/ V=1
  make: Entering directory '/home/nakamura/build_work/build_kernel/linux_kernel/linux/tools/lib/perf'
  make -f /home/nakamura/build_work/build_kernel/linux_kernel/linux/tools/build/Makefile.build dir=. obj=libperf
  make -C /home/nakamura/build_work/build_kernel/linux_kernel/linux/tools/lib/api/ O= libapi.a
  make -f /home/nakamura/build_work/build_kernel/linux_kernel/linux/tools/build/Makefile.build dir=./fd obj=libapi
  make -f /home/nakamura/build_work/build_kernel/linux_kernel/linux/tools/build/Makefile.build dir=./fs obj=libapi
  make -f /home/nakamura/build_work/build_kernel/linux_kernel/linux/tools/build/Makefile.build dir=. obj=tests
  make -f /home/nakamura/build_work/build_kernel/linux_kernel/linux/tools/build/Makefile.build dir=./tests obj=tests
  running static:
  - running tests/test-cpumap.c...OK
  - running tests/test-threadmap.c...OK
  - running tests/test-evlist.c...
  Event  0 -- Raw count = 298049842, run = 270269503, enable = 456262127
           Scaled count = 503160191 (59.24%, 270269503/456262127)
  Event  1 -- Raw count = 299134173, run = 271075173, enable = 456257234
           Scaled count = 503484435 (59.41%, 271075173/456257234)
  Event  2 -- Raw count = 300461996, run = 272069283, enable = 456253417
           Scaled count = 503867290 (59.63%, 272069283/456253417)
  Event  3 -- Raw count = 301308704, run = 273063387, enable = 456249352
           Scaled count = 503443183 (59.85%, 273063387/456249352)
  Event  4 -- Raw count = 302531164, run = 274102932, enable = 456244712
           Scaled count = 503563543 (60.08%, 274102932/456244712)
  Event  5 -- Raw count = 303710254, run = 275406214, enable = 456228165
           Scaled count = 503115633 (60.37%, 275406214/456228165)
  Event  6 -- Raw count = 304531302, run = 276396076, enable = 456221130
           Scaled count = 502661313 (60.58%, 276396076/456221130)
  Event  7 -- Raw count = 304486460, run = 276601890, enable = 456213754
           Scaled count = 502205212 (60.63%, 276601890/456213754)
  Event  8 -- Raw count = 304116681, run = 276631326, enable = 456205562
           Scaled count = 501532936 (60.64%, 276631326/456205562)
  Event  9 -- Raw count = 303567766, run = 276188567, enable = 456196839
           Scaled count = 501420666 (60.54%, 276188567/456196839)
  Event 10 -- Raw count = 302238014, run = 275144001, enable = 456185300
           Scaled count = 501106833 (60.31%, 275144001/456185300)
  Event 11 -- Raw count = 300805716, run = 273824589, enable = 456175608
           Scaled count = 501124573 (60.03%, 273824589/456175608)
  Event 12 -- Raw count = 299959051, run = 272834556, enable = 456166593
           Scaled count = 501517477 (59.81%, 272834556/456166593)
  Event 13 -- Raw count = 299037090, run = 271820805, enable = 456157086
           Scaled count = 501830195 (59.59%, 271820805/456157086)
  Event 14 -- Raw count = 298327042, run = 270784311, enable = 456147546
           Scaled count = 502544433 (59.36%, 270784311/456147546)
     Expected: 501614268
     High: 503867290   Low:  298049842   Average:  502438527
     Average Error = 0.16%
  OK
  - running tests/test-evsel.c...
          loop = 65536, count = 328182
          loop = 131072, count = 660214
          loop = 262144, count = 1315534
          loop = 524288, count = 2635364
          loop = 1048576, count = 5271971
          loop = 65536, count = 491952
          loop = 131072, count = 850061
          loop = 262144, count = 1648608
          loop = 524288, count = 3162059
          loop = 1048576, count = 6353393
  OK
  running dynamic:
  - running tests/test-cpumap.c...OK
  - running tests/test-threadmap.c...OK
  - running tests/test-evlist.c...
  Event  0 -- Raw count = 300218292, run = 297528154, enable = 496789343
           Scaled count = 501281125 (59.89%, 297528154/496789343)
  Event  1 -- Raw count = 301438606, run = 298515328, enable = 496784768
           Scaled count = 501649643 (60.09%, 298515328/496784768)
  Event  2 -- Raw count = 302342618, run = 298798983, enable = 496782015
           Scaled count = 502673648 (60.15%, 298798983/496782015)
  Event  3 -- Raw count = 303132319, run = 299230407, enable = 496778508
           Scaled count = 503256412 (60.23%, 299230407/496778508)
  Event  4 -- Raw count = 302758195, run = 299218047, enable = 496774243
           Scaled count = 502651743 (60.23%, 299218047/496774243)
  Event  5 -- Raw count = 303158458, run = 299204274, enable = 496769146
           Scaled count = 503334281 (60.23%, 299204274/496769146)
  Event  6 -- Raw count = 303471397, run = 299197479, enable = 496763124
           Scaled count = 503859189 (60.23%, 299197479/496763124)
  Event  7 -- Raw count = 303583387, run = 299196861, enable = 496756458
           Scaled count = 504039405 (60.23%, 299196861/496756458)
  Event  8 -- Raw count = 303096897, run = 299186924, enable = 496748667
           Scaled count = 503240507 (60.23%, 299186924/496748667)
  Event  9 -- Raw count = 301424173, run = 297845086, enable = 496739994
           Scaled count = 502709122 (59.96%, 297845086/496739994)
  Event 10 -- Raw count = 300876415, run = 296851339, enable = 496729034
           Scaled count = 503464297 (59.76%, 296851339/496729034)
  Event 11 -- Raw count = 300239338, run = 296547963, enable = 496719538
           Scaled count = 502902612 (59.70%, 296547963/496719538)
  Event 12 -- Raw count = 299751948, run = 296547195, enable = 496710036
           Scaled count = 502077926 (59.70%, 296547195/496710036)
  Event 13 -- Raw count = 299341883, run = 296549981, enable = 496700423
           Scaled count = 501376663 (59.70%, 296549981/496700423)
  Event 14 -- Raw count = 299145476, run = 296561684, enable = 496690949
           Scaled count = 501018366 (59.71%, 296561684/496690949)
     Expected: 501669431
     High: 504039405   Low:  300218292   Average:  502635662
     Average Error = 0.19%
  OK
  - running tests/test-evsel.c...
          loop = 65536, count = 329275
          loop = 131072, count = 664638
          loop = 262144, count = 1315367
          loop = 524288, count = 2629617
          loop = 1048576, count = 5273657
          loop = 65536, count = 459641
          loop = 131072, count = 978402
          loop = 262144, count = 1581219
          loop = 524288, count = 3774908
          loop = 1048576, count = 7694417
  OK
  make: Leaving directory '/home/nakamura/build_work/build_kernel/linux_kernel/linux/tools/lib/perf'

Signed-off-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
---
 tools/lib/perf/tests/test-evlist.c | 157 +++++++++++++++++++++++++++++
 1 file changed, 157 insertions(+)

diff --git a/tools/lib/perf/tests/test-evlist.c b/tools/lib/perf/tests/test-evlist.c
index ce91a582f0e4..520a78267743 100644
--- a/tools/lib/perf/tests/test-evlist.c
+++ b/tools/lib/perf/tests/test-evlist.c
@@ -21,6 +21,9 @@
 #include "tests.h"
 #include <internal/evsel.h>
 
+#define EVENT_NUM 15
+#define WAIT_COUNT 100000000UL
+
 static int libperf_print(enum libperf_print_level level,
 			 const char *fmt, va_list ap)
 {
@@ -413,6 +416,159 @@ static int test_mmap_cpus(void)
 	return 0;
 }
 
+static double display_error(long long average,
+			    long long high,
+			    long long low,
+			    long long expected)
+{
+	double error;
+
+	error = (((double)average - expected) / expected) * 100.0;
+
+	__T_VERBOSE("   Expected: %lld\n", expected);
+	__T_VERBOSE("   High: %lld   Low:  %lld   Average:  %lld\n",
+		    high, low, average);
+
+	__T_VERBOSE("   Average Error = %.2f%%\n", error);
+
+	return error;
+}
+
+static int test_stat_multiplexing(void)
+{
+	struct perf_counts_values expected_counts = { .val = 0 };
+	struct perf_counts_values counts[EVENT_NUM] = {{ .val = 0 },};
+	struct perf_thread_map *threads;
+	struct perf_evlist *evlist;
+	struct perf_evsel *evsel;
+	struct perf_event_attr attr = {
+		.type	     = PERF_TYPE_HARDWARE,
+		.config	     = PERF_COUNT_HW_INSTRUCTIONS,
+		.read_format = PERF_FORMAT_TOTAL_TIME_ENABLED |
+			       PERF_FORMAT_TOTAL_TIME_RUNNING,
+		.disabled    = 1,
+	};
+	int err, i, nonzero = 0;
+	unsigned long count;
+	long long max = 0, min = 0, avg = 0;
+	double error = 0.0;
+	s8 scaled = 0;
+
+	/* read for non-multiplexing event count */
+	threads = perf_thread_map__new_dummy();
+	__T("failed to create threads", threads);
+
+	perf_thread_map__set_pid(threads, 0, 0);
+
+	evsel = perf_evsel__new(&attr);
+	__T("failed to create evsel", evsel);
+
+	err = perf_evsel__open(evsel, NULL, threads);
+	__T("failed to open evsel", err == 0);
+
+	err = perf_evsel__enable(evsel);
+	__T("failed to enable evsel", err == 0);
+
+	/* wait loop */
+	count = WAIT_COUNT;
+	while (count--)
+		;
+
+	perf_evsel__read(evsel, 0, 0, &expected_counts);
+	__T("failed to read value for evsel", expected_counts.val != 0);
+	__T("failed to read non-multiplexing event count",
+	    expected_counts.ena == expected_counts.run);
+
+	err = perf_evsel__disable(evsel);
+	__T("failed to enable evsel", err == 0);
+
+	perf_evsel__close(evsel);
+	perf_evsel__delete(evsel);
+
+	perf_thread_map__put(threads);
+
+	/* read for multiplexing event count */
+	threads = perf_thread_map__new_dummy();
+	__T("failed to create threads", threads);
+
+	perf_thread_map__set_pid(threads, 0, 0);
+
+	evlist = perf_evlist__new();
+	__T("failed to create evlist", evlist);
+
+	for (i = 0; i < EVENT_NUM; i++) {
+		evsel = perf_evsel__new(&attr);
+		__T("failed to create evsel", evsel);
+
+		perf_evlist__add(evlist, evsel);
+	}
+	perf_evlist__set_maps(evlist, NULL, threads);
+
+	err = perf_evlist__open(evlist);
+	__T("failed to open evsel", err == 0);
+
+	perf_evlist__enable(evlist);
+
+	/* wait loop */
+	count = WAIT_COUNT;
+	while (count--)
+		;
+
+	i = 0;
+	perf_evlist__for_each_evsel(evlist, evsel) {
+		perf_evsel__read(evsel, 0, 0, &counts[i]);
+		__T("failed to read value for evsel", counts[i].val != 0);
+		i++;
+	}
+
+	perf_evlist__disable(evlist);
+
+	min = counts[0].val;
+	for (i = 0; i < EVENT_NUM; i++) {
+		__T_VERBOSE("Event %2d -- Raw count = %lu, run = %lu, enable = %lu\n",
+			    i, counts[i].val, counts[i].run, counts[i].ena);
+
+		perf_counts_values__scale(&counts[i], true, &scaled);
+		if (scaled == 1) {
+			__T_VERBOSE("\t Scaled count = %lu (%.2lf%%, %lu/%lu)\n",
+				    counts[i].val,
+				    (double)counts[i].run / (double)counts[i].ena * 100.0,
+				    counts[i].run, counts[i].ena);
+		} else if (scaled == -1) {
+			__T_VERBOSE("\t Not Runnnig\n");
+		} else {
+			__T_VERBOSE("\t Not Scaling\n");
+		}
+
+		if (counts[i].val > max)
+			max = counts[i].val;
+
+		if (counts[i].val < min)
+			min = counts[i].val;
+
+		avg += counts[i].val;
+
+		if (counts[i].val != 0)
+			nonzero++;
+	}
+
+	if (nonzero != 0)
+		avg = avg / nonzero;
+	else
+		avg = 0;
+
+	error = display_error(avg, max, min, expected_counts.val);
+
+	__T("Error out of range!", ((error <= 1.0) && (error >= -1.0)));
+
+	perf_evlist__close(evlist);
+	perf_evlist__delete(evlist);
+
+	perf_thread_map__put(threads);
+
+	return 0;
+}
+
 int test_evlist(int argc, char **argv)
 {
 	__T_START;
@@ -424,6 +580,7 @@ int test_evlist(int argc, char **argv)
 	test_stat_thread_enable();
 	test_mmap_thread();
 	test_mmap_cpus();
+	test_stat_multiplexing();
 
 	__T_END;
 	return tests_failed == 0 ? 0 : -1;
-- 
2.27.0

Re: [PATCH v3 1/3] libperf: Move perf_counts_values__scale to tools/lib/perf

[Jiri Olsa]

On Tue, Nov 09, 2021 at 05:58:29PM +0900, Shunsuke Nakamura wrote:
> Move perf_counts_values__scale from tools/perf/util to tools/lib/perf
> so that it can be used with libperf.
> 
> Signed-off-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
> ---
>  tools/lib/perf/evsel.c              | 19 +++++++++++++++++++
>  tools/lib/perf/include/perf/evsel.h |  4 ++++
>  tools/lib/perf/libperf.map          |  1 +
>  tools/perf/util/evsel.c             | 19 -------------------
>  tools/perf/util/evsel.h             |  3 ---
>  5 files changed, 24 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> index 8441e3e1aaac..5097aadea37a 100644
> --- a/tools/lib/perf/evsel.c
> +++ b/tools/lib/perf/evsel.c
> @@ -431,3 +431,22 @@ void perf_evsel__free_id(struct perf_evsel *evsel)
>  	zfree(&evsel->id);
>  	evsel->ids = 0;
>  }
> +
> +void perf_counts_values__scale(struct perf_counts_values *count,
> +			       bool scale, s8 *pscaled)
> +{
> +	s8 scaled = 0;
> +
> +	if (scale) {
> +		if (count->run == 0) {
> +			scaled = -1;
> +			count->val = 0;
> +		} else if (count->run < count->ena) {
> +			scaled = 1;
> +			count->val = (u64)((double)count->val * count->ena / count->run);
> +		}
> +	}
> +
> +	if (pscaled)
> +		*pscaled = scaled;
> +}
> diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> index 60eae25076d3..9013d73af22d 100644
> --- a/tools/lib/perf/include/perf/evsel.h
> +++ b/tools/lib/perf/include/perf/evsel.h
> @@ -4,6 +4,8 @@
>  
>  #include <stdint.h>
>  #include <perf/core.h>
> +#include <stdbool.h>
> +#include <linux/types.h>
>  
>  struct perf_evsel;
>  struct perf_event_attr;
> @@ -39,5 +41,7 @@ LIBPERF_API int perf_evsel__disable_cpu(struct perf_evsel *evsel, int cpu);
>  LIBPERF_API struct perf_cpu_map *perf_evsel__cpus(struct perf_evsel *evsel);
>  LIBPERF_API struct perf_thread_map *perf_evsel__threads(struct perf_evsel *evsel);
>  LIBPERF_API struct perf_event_attr *perf_evsel__attr(struct perf_evsel *evsel);
> +LIBPERF_API void perf_counts_values__scale(struct perf_counts_values *count,
> +					   bool scale, s8 *pscaled);

not sure if we should use __s8 for pscaled now when it's exported?
it's just we use it everywhere else with '__' prefix, I forgot what's
the difference actually ;-)

but other that all looks good, for the patchset:

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka


>  
>  #endif /* __LIBPERF_EVSEL_H */
> diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
> index 71468606e8a7..5979bf92d98f 100644
> --- a/tools/lib/perf/libperf.map
> +++ b/tools/lib/perf/libperf.map
> @@ -50,6 +50,7 @@ LIBPERF_0.0.1 {
>  		perf_mmap__read_init;
>  		perf_mmap__read_done;
>  		perf_mmap__read_event;
> +		perf_counts_values__scale;
>  	local:
>  		*;
>  };
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index dbfeceb2546c..49e4d0bdd7cc 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1457,25 +1457,6 @@ void evsel__compute_deltas(struct evsel *evsel, int cpu, int thread,
>  	count->run = count->run - tmp.run;
>  }
>  
> -void perf_counts_values__scale(struct perf_counts_values *count,
> -			       bool scale, s8 *pscaled)
> -{
> -	s8 scaled = 0;
> -
> -	if (scale) {
> -		if (count->run == 0) {
> -			scaled = -1;
> -			count->val = 0;
> -		} else if (count->run < count->ena) {
> -			scaled = 1;
> -			count->val = (u64)((double) count->val * count->ena / count->run);
> -		}
> -	}
> -
> -	if (pscaled)
> -		*pscaled = scaled;
> -}
> -
>  static int evsel__read_one(struct evsel *evsel, int cpu, int thread)
>  {
>  	struct perf_counts_values *count = perf_counts(evsel->counts, cpu, thread);
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 1f7edfa8568a..8a6a4182c5fd 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -190,9 +190,6 @@ static inline int evsel__nr_cpus(struct evsel *evsel)
>  	return evsel__cpus(evsel)->nr;
>  }
>  
> -void perf_counts_values__scale(struct perf_counts_values *count,
> -			       bool scale, s8 *pscaled);
> -
>  void evsel__compute_deltas(struct evsel *evsel, int cpu, int thread,
>  			   struct perf_counts_values *count);
>  
> -- 
> 2.27.0
> 

Re: [PATCH v3 1/3] libperf: Move perf_counts_values__scale to tools/lib/perf

[Arnaldo Carvalho de Melo]

Em Sun, Nov 14, 2021 at 05:19:45PM +0100, Jiri Olsa escreveu:
> On Tue, Nov 09, 2021 at 05:58:29PM +0900, Shunsuke Nakamura wrote:
> > Move perf_counts_values__scale from tools/perf/util to tools/lib/perf
> > so that it can be used with libperf.
> > 
> > Signed-off-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
> > ---
> >  tools/lib/perf/evsel.c              | 19 +++++++++++++++++++
> >  tools/lib/perf/include/perf/evsel.h |  4 ++++
> >  tools/lib/perf/libperf.map          |  1 +
> >  tools/perf/util/evsel.c             | 19 -------------------
> >  tools/perf/util/evsel.h             |  3 ---
> >  5 files changed, 24 insertions(+), 22 deletions(-)
> > 
> > diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> > index 8441e3e1aaac..5097aadea37a 100644
> > --- a/tools/lib/perf/evsel.c
> > +++ b/tools/lib/perf/evsel.c
> > @@ -431,3 +431,22 @@ void perf_evsel__free_id(struct perf_evsel *evsel)
> >  	zfree(&evsel->id);
> >  	evsel->ids = 0;
> >  }
> > +
> > +void perf_counts_values__scale(struct perf_counts_values *count,
> > +			       bool scale, s8 *pscaled)
> > +{
> > +	s8 scaled = 0;
> > +
> > +	if (scale) {
> > +		if (count->run == 0) {
> > +			scaled = -1;
> > +			count->val = 0;
> > +		} else if (count->run < count->ena) {
> > +			scaled = 1;
> > +			count->val = (u64)((double)count->val * count->ena / count->run);
> > +		}
> > +	}
> > +
> > +	if (pscaled)
> > +		*pscaled = scaled;
> > +}
> > diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> > index 60eae25076d3..9013d73af22d 100644
> > --- a/tools/lib/perf/include/perf/evsel.h
> > +++ b/tools/lib/perf/include/perf/evsel.h
> > @@ -4,6 +4,8 @@
> >  
> >  #include <stdint.h>
> >  #include <perf/core.h>
> > +#include <stdbool.h>
> > +#include <linux/types.h>
> >  
> >  struct perf_evsel;
> >  struct perf_event_attr;
> > @@ -39,5 +41,7 @@ LIBPERF_API int perf_evsel__disable_cpu(struct perf_evsel *evsel, int cpu);
> >  LIBPERF_API struct perf_cpu_map *perf_evsel__cpus(struct perf_evsel *evsel);
> >  LIBPERF_API struct perf_thread_map *perf_evsel__threads(struct perf_evsel *evsel);
> >  LIBPERF_API struct perf_event_attr *perf_evsel__attr(struct perf_evsel *evsel);
> > +LIBPERF_API void perf_counts_values__scale(struct perf_counts_values *count,
> > +					   bool scale, s8 *pscaled);
> 
> not sure if we should use __s8 for pscaled now when it's exported?
> it's just we use it everywhere else with '__' prefix, I forgot what's
> the difference actually ;-)

I'm moving this to be __s8, following what is being used in
tools/lib/bpf/bpf.h.
 
> but other that all looks good, for the patchset:
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> 
> thanks,
> jirka
> 
> 
> >  
> >  #endif /* __LIBPERF_EVSEL_H */
> > diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
> > index 71468606e8a7..5979bf92d98f 100644
> > --- a/tools/lib/perf/libperf.map
> > +++ b/tools/lib/perf/libperf.map
> > @@ -50,6 +50,7 @@ LIBPERF_0.0.1 {
> >  		perf_mmap__read_init;
> >  		perf_mmap__read_done;
> >  		perf_mmap__read_event;
> > +		perf_counts_values__scale;
> >  	local:
> >  		*;
> >  };
> > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> > index dbfeceb2546c..49e4d0bdd7cc 100644
> > --- a/tools/perf/util/evsel.c
> > +++ b/tools/perf/util/evsel.c
> > @@ -1457,25 +1457,6 @@ void evsel__compute_deltas(struct evsel *evsel, int cpu, int thread,
> >  	count->run = count->run - tmp.run;
> >  }
> >  
> > -void perf_counts_values__scale(struct perf_counts_values *count,
> > -			       bool scale, s8 *pscaled)
> > -{
> > -	s8 scaled = 0;
> > -
> > -	if (scale) {
> > -		if (count->run == 0) {
> > -			scaled = -1;
> > -			count->val = 0;
> > -		} else if (count->run < count->ena) {
> > -			scaled = 1;
> > -			count->val = (u64)((double) count->val * count->ena / count->run);
> > -		}
> > -	}
> > -
> > -	if (pscaled)
> > -		*pscaled = scaled;
> > -}
> > -
> >  static int evsel__read_one(struct evsel *evsel, int cpu, int thread)
> >  {
> >  	struct perf_counts_values *count = perf_counts(evsel->counts, cpu, thread);
> > diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> > index 1f7edfa8568a..8a6a4182c5fd 100644
> > --- a/tools/perf/util/evsel.h
> > +++ b/tools/perf/util/evsel.h
> > @@ -190,9 +190,6 @@ static inline int evsel__nr_cpus(struct evsel *evsel)
> >  	return evsel__cpus(evsel)->nr;
> >  }
> >  
> > -void perf_counts_values__scale(struct perf_counts_values *count,
> > -			       bool scale, s8 *pscaled);
> > -
> >  void evsel__compute_deltas(struct evsel *evsel, int cpu, int thread,
> >  			   struct perf_counts_values *count);
> >  
> > -- 
> > 2.27.0
> > 

-- 

- Arnaldo

Re: [PATCH v3 1/3] libperf: Move perf_counts_values__scale to tools/lib/perf

[nakamura.shun]

Hi Arnaldo, jirka
Sorry for the late reply.

Em Sun, Nov 14, 2021 at 05:19:45PM +0100, Jiri Olsa escreveu:
> > On Tue, Nov 09, 2021 at 05:58:29PM +0900, Shunsuke Nakamura wrote:
> > > Move perf_counts_values__scale from tools/perf/util to tools/lib/perf
> > > so that it can be used with libperf.
> > > 
> > > Signed-off-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
> > > ---
> > >  tools/lib/perf/evsel.c              | 19 +++++++++++++++++++
> > >  tools/lib/perf/include/perf/evsel.h |  4 ++++
> > >  tools/lib/perf/libperf.map          |  1 +
> > >  tools/perf/util/evsel.c             | 19 -------------------
> > >  tools/perf/util/evsel.h             |  3 ---
> > >  5 files changed, 24 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> > > index 8441e3e1aaac..5097aadea37a 100644
> > > --- a/tools/lib/perf/evsel.c
> > > +++ b/tools/lib/perf/evsel.c
> > > @@ -431,3 +431,22 @@ void perf_evsel__free_id(struct perf_evsel *evsel)
> > >      zfree(&evsel->id);
> > >      evsel->ids = 0;
> > >  }
> > > +
> > > +void perf_counts_values__scale(struct perf_counts_values *count,
> > > +                          bool scale, s8 *pscaled)
> > > +{
> > > +   s8 scaled = 0;
> > > +
> > > +   if (scale) {
> > > +           if (count->run == 0) {
> > > +                   scaled = -1;
> > > +                   count->val = 0;
> > > +           } else if (count->run < count->ena) {
> > > +                   scaled = 1;
> > > +                   count->val = (u64)((double)count->val * count->ena / count->run);
> > > +           }
> > > +   }
> > > +
> > > +   if (pscaled)
> > > +           *pscaled = scaled;
> > > +}
> > > diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> > > index 60eae25076d3..9013d73af22d 100644
> > > --- a/tools/lib/perf/include/perf/evsel.h
> > > +++ b/tools/lib/perf/include/perf/evsel.h
> > > @@ -4,6 +4,8 @@
> > >  
> > >  #include <stdint.h>
> > >  #include <perf/core.h>
> > > +#include <stdbool.h>
> > > +#include <linux/types.h>
> > >  
> > >  struct perf_evsel;
> > >  struct perf_event_attr;
> > > @@ -39,5 +41,7 @@ LIBPERF_API int perf_evsel__disable_cpu(struct perf_evsel *evsel, int cpu);
> > >  LIBPERF_API struct perf_cpu_map *perf_evsel__cpus(struct perf_evsel *evsel);
> > >  LIBPERF_API struct perf_thread_map *perf_evsel__threads(struct perf_evsel *evsel);
> > >  LIBPERF_API struct perf_event_attr *perf_evsel__attr(struct perf_evsel *evsel);
> > > +LIBPERF_API void perf_counts_values__scale(struct perf_counts_values *count,
> > > +                                      bool scale, s8 *pscaled);
> > 
> > not sure if we should use __s8 for pscaled now when it's exported?
> > it's just we use it everywhere else with '__' prefix, I forgot what's
> > the difference actually ;-)
> 
> I'm moving this to be __s8, following what is being used in
> tools/lib/bpf/bpf.h.
I will fix it.
 
> > but other that all looks good, for the patchset:
> > 
> > Acked-by: Jiri Olsa <jolsa@kernel.org>
Thanks for review.

Best Regards
Shunsuke