Re: [PATCH v13 11/16] perf record: Introduce data transferred and compressed stats
From: Arnaldo Carvalho de Melo
Date: Mon Jan 24 2022 - 10:30:25 EST
Em Mon, Jan 17, 2022 at 09:34:31PM +0300, Alexey Bayduraev escreveu:
> Introduce bytes_transferred and bytes_compressed stats so they
> would capture statistics for the related data buffer transfers.
>
> Acked-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> Acked-by: Namhyung Kim <namhyung@xxxxxxxxx>
> Reviewed-by: Riccardo Mancini <rickyman7@xxxxxxxxx>
> Tested-by: Riccardo Mancini <rickyman7@xxxxxxxxx>
> Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@xxxxxxxxxxxxxxx>
> ---
> tools/perf/builtin-record.c | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 7d0338b5a0e3..0f8488d12f44 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -111,6 +111,8 @@ struct record_thread {
> unsigned long long samples;
> unsigned long waking;
> u64 bytes_written;
> + u64 bytes_transferred;
> + u64 bytes_compressed;
> };
>
> static __thread struct record_thread *thread;
> @@ -1407,8 +1409,13 @@ static size_t zstd_compress(struct perf_session *session, struct mmap *map,
> compressed = zstd_compress_stream_to_records(zstd_data, dst, dst_size, src, src_size,
> max_record_size, process_comp_header);
>
> - session->bytes_transferred += src_size;
> - session->bytes_compressed += compressed;
> + if (map && map->file) {
> + thread->bytes_transferred += src_size;
> + thread->bytes_compressed += compressed;
> + } else {
> + session->bytes_transferred += src_size;
> + session->bytes_compressed += compressed;
> + }
>
> return compressed;
> }
> @@ -2098,8 +2105,20 @@ static int record__stop_threads(struct record *rec)
> for (t = 1; t < rec->nr_threads; t++)
> record__terminate_thread(&thread_data[t]);
>
> - for (t = 0; t < rec->nr_threads; t++)
> + for (t = 0; t < rec->nr_threads; t++) {
> rec->samples += thread_data[t].samples;
> + if (!record__threads_enabled(rec))
> + continue;
> + rec->session->bytes_transferred += thread_data[t].bytes_transferred;
> + rec->session->bytes_compressed += thread_data[t].bytes_compressed;
> + pr_debug("threads[%d]: samples=%lld, wakes=%ld, ", thread_data[t].tid,
> + thread_data[t].samples, thread_data[t].waking);
> + if (thread_data[t].bytes_transferred && thread_data[t].bytes_compressed)
> + pr_debug("trasferred=%ld, compressed=%ld\n",
> + thread_data[t].bytes_transferred, thread_data[t].bytes_compressed);
> + else
> + pr_debug("written=%ld\n", thread_data[t].bytes_written);
In file included from builtin-record.c:22:
builtin-record.c: In function 'record__stop_threads':
builtin-record.c:2138:13: error: format '%ld' expects argument of type 'long int', but argument 4 has type 'u64' {aka 'long long unsigned int'} [-Werror=format=]
2138 | pr_debug("trasferred=%ld, compressed=%ld\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/debug.h:18:21: note: in definition of macro 'pr_fmt'
18 | #define pr_fmt(fmt) fmt
| ^~~
builtin-record.c:2138:4: note: in expansion of macro 'pr_debug'
2138 | pr_debug("trasferred=%ld, compressed=%ld\n",
| ^~~~~~~~
builtin-record.c:2138:27: note: format string is defined here
2138 | pr_debug("trasferred=%ld, compressed=%ld\n",
| ~~^
| |
| long int
| %lld
In file included from builtin-record.c:22:
builtin-record.c:2138:13: error: format '%ld' expects argument of type 'long int', but argument 5 has type 'u64' {aka 'long long unsigned int'} [-Werror=format=]
2138 | pr_debug("trasferred=%ld, compressed=%ld\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/debug.h:18:21: note: in definition of macro 'pr_fmt'
18 | #define pr_fmt(fmt) fmt
| ^~~
builtin-record.c:2138:4: note: in expansion of macro 'pr_debug'
2138 | pr_debug("trasferred=%ld, compressed=%ld\n",
| ^~~~~~~~
LINK /tmp/build/perf/libtraceevent.a
builtin-record.c:2138:43: note: format string is defined here
2138 | pr_debug("trasferred=%ld, compressed=%ld\n",
| ~~^
| |
| long int
| %lld
In file included from builtin-record.c:22:
builtin-record.c:2141:13: error: format '%ld' expects argument of type 'long int', but argument 4 has type 'u64' {aka 'long long unsigned int'} [-Werror=format=]
2141 | pr_debug("written=%ld\n", thread_data[t].bytes_written);
| ^~~~~~~~~~~~~~~
util/debug.h:18:21: note: in definition of macro 'pr_fmt'
18 | #define pr_fmt(fmt) fmt
| ^~~
builtin-record.c:2141:4: note: in expansion of macro 'pr_debug'
2141 | pr_debug("written=%ld\n", thread_data[t].bytes_written);
| ^~~~~~~~
builtin-record.c:2141:24: note: format string is defined here
2141 | pr_debug("written=%ld\n", thread_data[t].bytes_written);
| ~~^
| |
| long int
| %lld
Fixed with the following patch, no need to resend, I'll fix several
other similar issues and put the result in a tmp.perf/thread branch
while I review/test it.
- Arnaldo
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 0f8488d12f446b84..d19d0639c3f1abc0 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2114,10 +2114,10 @@ static int record__stop_threads(struct record *rec)
pr_debug("threads[%d]: samples=%lld, wakes=%ld, ", thread_data[t].tid,
thread_data[t].samples, thread_data[t].waking);
if (thread_data[t].bytes_transferred && thread_data[t].bytes_compressed)
- pr_debug("trasferred=%ld, compressed=%ld\n",
+ pr_debug("transferred=%" PRIu64 ", compressed=%" PRIu64 "\n",
thread_data[t].bytes_transferred, thread_data[t].bytes_compressed);
else
- pr_debug("written=%ld\n", thread_data[t].bytes_written);
+ pr_debug("written=%" PRIu64 "\n", thread_data[t].bytes_written);
}
return 0;