[PATCH 0/2] perf tools: minor fixes regarding macro usage

[PATCH 0/2] perf tools: minor fixes regarding macro usage

[Emmanouil Maroudas]

    Hello,

    Here are some minor fixes for perf tools regarding *SEC_PER*SEC macro usage,
    found while reading the code.

    Patch 1 replaces a macro with a more suitable one (same value, different name).
    Patch 2 converts some hardcoded values (which seem appropriate IMHO) with their
    respective macros.

    Based on v5.8-rc3.

    No functionality change. The final perf binary is the same, before and after
    these patches.

    Thanks,
    Emmanouil Maroudas

Emmanouil Maroudas (2):
  perf stat: use proper macro for time conversions
  perf tools: use *SEC_PER_*SEC macros

 tools/perf/builtin-record.c |  2 +-
 tools/perf/builtin-stat.c   | 12 ++++++------
 tools/perf/builtin-trace.c  |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.17.1

[PATCH 1/2] perf stat: use proper macro for time conversions

[Emmanouil Maroudas]

    The values of interval and timeout are in msecs as documented in the
    -I and --timeout options.

    Use MSEC_PER_SEC instead USEC_PER_MSEC to convert to struct timespec.
    Both macros have the same value 1000L (see tools/include/linux/time64.h).

    No functional change intended.

Signed-off-by: Emmanouil Maroudas <emmanouil.maroudas@gmail.com>
---
 tools/perf/builtin-stat.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 9be020e0098a..6aa866e2d512 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -613,11 +613,11 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
 	bool second_pass = false;
 
 	if (interval) {
-		ts.tv_sec  = interval / USEC_PER_MSEC;
-		ts.tv_nsec = (interval % USEC_PER_MSEC) * NSEC_PER_MSEC;
+		ts.tv_sec  = interval / MSEC_PER_SEC;
+		ts.tv_nsec = (interval % MSEC_PER_SEC) * NSEC_PER_MSEC;
 	} else if (timeout) {
-		ts.tv_sec  = timeout / USEC_PER_MSEC;
-		ts.tv_nsec = (timeout % USEC_PER_MSEC) * NSEC_PER_MSEC;
+		ts.tv_sec  = timeout / MSEC_PER_SEC;
+		ts.tv_nsec = (timeout % MSEC_PER_SEC) * NSEC_PER_MSEC;
 	} else {
 		ts.tv_sec  = 1;
 		ts.tv_nsec = 0;
-- 
2.17.1

[PATCH 2/2] perf tools: use *SEC_PER_*SEC macros

[Emmanouil Maroudas]

    builtin-stat.c: typecast macro to u64
    see commit ea9eb1f456a0 ("perf stat: Fix duration_time value for higher intervals")

    No functional change intended.

Signed-off-by: Emmanouil Maroudas <emmanouil.maroudas@gmail.com>
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/builtin-stat.c   | 4 ++--
 tools/perf/builtin-trace.c  | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e108d90ae2ed..c3c7823966bc 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -249,7 +249,7 @@ static int record__aio_sync(struct mmap *md, bool sync_all)
 {
 	struct aiocb **aiocb = md->aio.aiocb;
 	struct aiocb *cblocks = md->aio.cblocks;
-	struct timespec timeout = { 0, 1000 * 1000  * 1 }; /* 1ms */
+	struct timespec timeout = { 0, NSEC_PER_MSEC * 1 }; /* 1ms */
 	int i, do_suspend;
 
 	do {
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 6aa866e2d512..2f4fe1aee865 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -305,7 +305,7 @@ static int read_single_counter(struct evsel *counter, int cpu,
 			       int thread, struct timespec *rs)
 {
 	if (counter->tool_event == PERF_TOOL_DURATION_TIME) {
-		u64 val = rs->tv_nsec + rs->tv_sec*1000000000ULL;
+		u64 val = rs->tv_nsec + rs->tv_sec*(u64)NSEC_PER_SEC;
 		struct perf_counts_values *count =
 			perf_counts(counter->counts, cpu, thread);
 		count->ena = count->run = val;
@@ -471,7 +471,7 @@ static void process_interval(void)
 	}
 
 	init_stats(&walltime_nsecs_stats);
-	update_stats(&walltime_nsecs_stats, stat_config.interval * 1000000ULL);
+	update_stats(&walltime_nsecs_stats, stat_config.interval * (u64)NSEC_PER_MSEC);
 	print_counters(&rs, 0, NULL);
 }
 
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 4cbb64edc998..db0a2369e2f2 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -4043,7 +4043,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 		perf_evlist__start_workload(evlist);
 
 	if (trace->opts.initial_delay) {
-		usleep(trace->opts.initial_delay * 1000);
+		usleep(trace->opts.initial_delay * USEC_PER_MSEC);
 		evlist__enable(evlist);
 	}
 
-- 
2.17.1

Re: [PATCH 0/2] perf tools: minor fixes regarding macro usage

[Jiri Olsa]

On Mon, Jun 29, 2020 at 02:25:19AM +0300, Emmanouil Maroudas wrote:
>     Hello,
> 
>     Here are some minor fixes for perf tools regarding *SEC_PER*SEC macro usage,
>     found while reading the code.
> 
>     Patch 1 replaces a macro with a more suitable one (same value, different name).
>     Patch 2 converts some hardcoded values (which seem appropriate IMHO) with their
>     respective macros.
> 
>     Based on v5.8-rc3.
> 
>     No functionality change. The final perf binary is the same, before and after
>     these patches.
> 
>     Thanks,
>     Emmanouil Maroudas
> 
> Emmanouil Maroudas (2):
>   perf stat: use proper macro for time conversions
>   perf tools: use *SEC_PER_*SEC macros

Acked-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka

> 
>  tools/perf/builtin-record.c |  2 +-
>  tools/perf/builtin-stat.c   | 12 ++++++------
>  tools/perf/builtin-trace.c  |  2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> -- 
> 2.17.1
>