Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 24 Jan 2022 12:28:18 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
Cc:     Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Andi Kleen <ak@linux.intel.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Alexander Antonov <alexander.antonov@linux.intel.com>,
        Alexei Budankov <abudankov@huawei.com>,
        Riccardo Mancini <rickyman7@gmail.com>
Subject: Re: [PATCH v13 11/16] perf record: Introduce data transferred and
 compressed stats
Message-ID: <Ye7FkknVAUJjM+oV@kernel.org>
References: <cover.1642440724.git.alexey.v.bayduraev@linux.intel.com>
 <b5d598034c507dfb7544d2125500280b7d434764.1642440724.git.alexey.v.bayduraev@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <b5d598034c507dfb7544d2125500280b7d434764.1642440724.git.alexey.v.bayduraev@linux.intel.com>
Precedence: bulk

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@linux.intel.com>
> Acked-by: Namhyung Kim <namhyung@gmail.com>
> Reviewed-by: Riccardo Mancini <rickyman7@gmail.com>
> Tested-by: Riccardo Mancini <rickyman7@gmail.com>
> Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
> ---
>  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;