[PATCH 1/4] perf tools: Use perf_evsel__match in perf_evsel__is_bpf_output

[PATCH 1/4] perf tools: Use perf_evsel__match in perf_evsel__is_bpf_output

[Jiri Olsa]

Using perf_evsel__match helper in perf_evsel__is_bpf_output.

Link: http://lkml.kernel.org/n/tip-lquycuebqfcb56byv1qdjaix@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/evsel.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index d277930b19a1..890babf9ce86 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -402,10 +402,7 @@ bool perf_evsel__is_function_event(struct perf_evsel *evsel);
 
 static inline bool perf_evsel__is_bpf_output(struct perf_evsel *evsel)
 {
-	struct perf_event_attr *attr = &evsel->attr;
-
-	return (attr->config == PERF_COUNT_SW_BPF_OUTPUT) &&
-		(attr->type == PERF_TYPE_SOFTWARE);
+	return perf_evsel__match(evsel, SOFTWARE, SW_BPF_OUTPUT);
 }
 
 struct perf_attr_details {
-- 
2.17.1

[PATCH 2/4] perf stat: Get rid of extra clock display function

[Jiri Olsa]

There's no reason to have separate function to display clock
events. It's only purpose was to convert the nanosecond value
into microseconds. We do that now in generic code, if the unit
and scale values are properly set, which this patch do for
clock events.

The output differs in the unit field being displayed in its
columns rather than having it added as a suffix of the event
name. Plus the value is rounded into 2 decimal numbers as
for any other event.

Before:
  # perf stat  -e cpu-clock,task-clock -C 0 sleep 3

   Performance counter stats for 'CPU(s) 0':

       3001.123137      cpu-clock (msec)          #    1.000 CPUs utilized
       3001.133250      task-clock (msec)         #    1.000 CPUs utilized

       3.001159813 seconds time elapsed

Now:
  # perf stat  -e cpu-clock,task-clock -C 0 sleep 3

   Performance counter stats for 'CPU(s) 0':

          3,001.05 msec cpu-clock                 #    1.000 CPUs utilized
          3,001.05 msec task-clock                #    1.000 CPUs utilized

       3.001077794 seconds time elapsed

There's small difference in csv output, as we now output the unit
field, which was empty before. It's in the proper spot, so there's
no compatibility issue.

Before:

  # perf stat  -e cpu-clock,task-clock -C 0 -x, sleep 3
  3001.065177,,cpu-clock,3001064187,100.00,1.000,CPUs utilized
  3001.077085,,task-clock,3001077085,100.00,1.000,CPUs utilized

  # perf stat  -e cpu-clock,task-clock -C 0 -x, sleep 3
  3000.80,msec,cpu-clock,3000799026,100.00,1.000,CPUs utilized
  3000.80,msec,task-clock,3000799550,100.00,1.000,CPUs utilized

Adding perf_evsel__is_clock to replace nsec_counter.

Cc: Andi Kleen <ak@linux.intel.com>
Link: http://lkml.kernel.org/n/tip-f5uxatzy4kw9sh2b6hwhx697@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-stat.c     | 48 ++---------------------------------
 tools/perf/util/evsel.c       | 11 ++++++++
 tools/perf/util/evsel.h       |  6 +++++
 tools/perf/util/stat-shadow.c |  5 ++--
 4 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index dfd13d6e2931..d097b5b47eb8 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -296,18 +296,6 @@ static int create_perf_stat_counter(struct perf_evsel *evsel)
 	return perf_evsel__open_per_thread(evsel, evsel_list->threads);
 }
 
-/*
- * Does the counter have nsecs as a unit?
- */
-static inline int nsec_counter(struct perf_evsel *evsel)
-{
-	if (perf_evsel__match(evsel, SOFTWARE, SW_CPU_CLOCK) ||
-	    perf_evsel__match(evsel, SOFTWARE, SW_TASK_CLOCK))
-		return 1;
-
-	return 0;
-}
-
 static int process_synthesized_event(struct perf_tool *tool __maybe_unused,
 				     union perf_event *event,
 				     struct perf_sample *sample __maybe_unused,
@@ -1058,34 +1046,6 @@ static void print_metric_header(void *ctx, const char *color __maybe_unused,
 		fprintf(os->fh, "%*s ", metric_only_len, unit);
 }
 
-static void nsec_printout(int id, int nr, struct perf_evsel *evsel, double avg)
-{
-	FILE *output = stat_config.output;
-	double msecs = avg / NSEC_PER_MSEC;
-	const char *fmt_v, *fmt_n;
-	char name[25];
-
-	fmt_v = csv_output ? "%.6f%s" : "%18.6f%s";
-	fmt_n = csv_output ? "%s" : "%-25s";
-
-	aggr_printout(evsel, id, nr);
-
-	scnprintf(name, sizeof(name), "%s%s",
-		  perf_evsel__name(evsel), csv_output ? "" : " (msec)");
-
-	fprintf(output, fmt_v, msecs, csv_sep);
-
-	if (csv_output)
-		fprintf(output, "%s%s", evsel->unit, csv_sep);
-	else
-		fprintf(output, "%-*s%s", unit_width, evsel->unit, csv_sep);
-
-	fprintf(output, fmt_n, name);
-
-	if (evsel->cgrp)
-		fprintf(output, "%s%s", csv_sep, evsel->cgrp->name);
-}
-
 static int first_shadow_cpu(struct perf_evsel *evsel, int id)
 {
 	int i;
@@ -1241,11 +1201,7 @@ static void printout(int id, int nr, struct perf_evsel *counter, double uval,
 		return;
 	}
 
-	if (metric_only)
-		/* nothing */;
-	else if (nsec_counter(counter))
-		nsec_printout(id, nr, counter, uval);
-	else
+	if (!metric_only)
 		abs_printout(id, nr, counter, uval);
 
 	out.print_metric = pm;
@@ -1331,7 +1287,7 @@ static void collect_all_aliases(struct perf_evsel *counter,
 		    alias->scale != counter->scale ||
 		    alias->cgrp != counter->cgrp ||
 		    strcmp(alias->unit, counter->unit) ||
-		    nsec_counter(alias) != nsec_counter(counter))
+		    perf_evsel__is_clock(alias) != perf_evsel__is_clock(counter))
 			break;
 		alias->merged_stat = true;
 		cb(alias, data, false);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 94fce4f537e9..5285da0417c5 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -260,6 +260,17 @@ struct perf_evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
 		evsel->attr.sample_period = 1;
 	}
 
+	if (perf_evsel__is_clock(evsel)) {
+		/*
+		 * The evsel->unit points to static alias->unit
+		 * so it's ok to use static string in here.
+		 */
+		static const char *unit = "msec";
+
+		evsel->unit = unit;
+		evsel->scale = 1e-6;
+	}
+
 	return evsel;
 }
 
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 890babf9ce86..973c03167947 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -405,6 +405,12 @@ static inline bool perf_evsel__is_bpf_output(struct perf_evsel *evsel)
 	return perf_evsel__match(evsel, SOFTWARE, SW_BPF_OUTPUT);
 }
 
+static inline bool perf_evsel__is_clock(struct perf_evsel *evsel)
+{
+	return perf_evsel__match(evsel, SOFTWARE, SW_CPU_CLOCK) ||
+	       perf_evsel__match(evsel, SOFTWARE, SW_TASK_CLOCK);
+}
+
 struct perf_attr_details {
 	bool freq;
 	bool verbose;
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 594d14a02b67..99990f5f2512 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -913,11 +913,10 @@ void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
 			ratio = total / avg;
 
 		print_metric(ctxp, NULL, "%8.0f", "cycles / elision", ratio);
-	} else if (perf_evsel__match(evsel, SOFTWARE, SW_TASK_CLOCK) ||
-		   perf_evsel__match(evsel, SOFTWARE, SW_CPU_CLOCK)) {
+	} else if (perf_evsel__is_clock(evsel)) {
 		if ((ratio = avg_stats(&walltime_nsecs_stats)) != 0)
 			print_metric(ctxp, NULL, "%8.3f", "CPUs utilized",
-				     avg / ratio);
+				     avg / (ratio * evsel->scale));
 		else
 			print_metric(ctxp, NULL, NULL, "CPUs utilized", 0);
 	} else if (perf_stat_evsel__is(evsel, TOPDOWN_FETCH_BUBBLES)) {
-- 
2.17.1

[tip:perf/core] perf stat: Get rid of extra clock display function

[tip-bot for Jiri Olsa]

Commit-ID:  0aa802a79469a86ebe143019144cd4df8ae852e4
Gitweb:     https://git.kernel.org/tip/0aa802a79469a86ebe143019144cd4df8ae852e4
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Fri, 20 Jul 2018 13:00:34 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 24 Jul 2018 14:54:58 -0300

perf stat: Get rid of extra clock display function

There's no reason to have separate function to display clock events.
It's only purpose was to convert the nanosecond value into microseconds.
We do that now in generic code, if the unit and scale values are
properly set, which this patch do for clock events.

The output differs in the unit field being displayed in its columns
rather than having it added as a suffix of the event name. Plus the
value is rounded into 2 decimal numbers as for any other event.

Before:

  # perf stat  -e cpu-clock,task-clock -C 0 sleep 3

   Performance counter stats for 'CPU(s) 0':

       3001.123137      cpu-clock (msec)          #    1.000 CPUs utilized
       3001.133250      task-clock (msec)         #    1.000 CPUs utilized

       3.001159813 seconds time elapsed

Now:

  # perf stat  -e cpu-clock,task-clock -C 0 sleep 3

   Performance counter stats for 'CPU(s) 0':

          3,001.05 msec cpu-clock                 #    1.000 CPUs utilized
          3,001.05 msec task-clock                #    1.000 CPUs utilized

       3.001077794 seconds time elapsed

There's a small difference in csv output, as we now output the unit
field, which was empty before. It's in the proper spot, so there's no
compatibility issue.

Before:

  # perf stat  -e cpu-clock,task-clock -C 0 -x, sleep 3
  3001.065177,,cpu-clock,3001064187,100.00,1.000,CPUs utilized
  3001.077085,,task-clock,3001077085,100.00,1.000,CPUs utilized

  # perf stat  -e cpu-clock,task-clock -C 0 -x, sleep 3
  3000.80,msec,cpu-clock,3000799026,100.00,1.000,CPUs utilized
  3000.80,msec,task-clock,3000799550,100.00,1.000,CPUs utilized

Add perf_evsel__is_clock to replace nsec_counter.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180720110036.32251-2-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c     | 48 ++-----------------------------------------
 tools/perf/util/evsel.c       | 11 ++++++++++
 tools/perf/util/evsel.h       |  6 ++++++
 tools/perf/util/stat-shadow.c |  5 ++---
 4 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index dfd13d6e2931..d097b5b47eb8 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -296,18 +296,6 @@ static int create_perf_stat_counter(struct perf_evsel *evsel)
 	return perf_evsel__open_per_thread(evsel, evsel_list->threads);
 }
 
-/*
- * Does the counter have nsecs as a unit?
- */
-static inline int nsec_counter(struct perf_evsel *evsel)
-{
-	if (perf_evsel__match(evsel, SOFTWARE, SW_CPU_CLOCK) ||
-	    perf_evsel__match(evsel, SOFTWARE, SW_TASK_CLOCK))
-		return 1;
-
-	return 0;
-}
-
 static int process_synthesized_event(struct perf_tool *tool __maybe_unused,
 				     union perf_event *event,
 				     struct perf_sample *sample __maybe_unused,
@@ -1058,34 +1046,6 @@ static void print_metric_header(void *ctx, const char *color __maybe_unused,
 		fprintf(os->fh, "%*s ", metric_only_len, unit);
 }
 
-static void nsec_printout(int id, int nr, struct perf_evsel *evsel, double avg)
-{
-	FILE *output = stat_config.output;
-	double msecs = avg / NSEC_PER_MSEC;
-	const char *fmt_v, *fmt_n;
-	char name[25];
-
-	fmt_v = csv_output ? "%.6f%s" : "%18.6f%s";
-	fmt_n = csv_output ? "%s" : "%-25s";
-
-	aggr_printout(evsel, id, nr);
-
-	scnprintf(name, sizeof(name), "%s%s",
-		  perf_evsel__name(evsel), csv_output ? "" : " (msec)");
-
-	fprintf(output, fmt_v, msecs, csv_sep);
-
-	if (csv_output)
-		fprintf(output, "%s%s", evsel->unit, csv_sep);
-	else
-		fprintf(output, "%-*s%s", unit_width, evsel->unit, csv_sep);
-
-	fprintf(output, fmt_n, name);
-
-	if (evsel->cgrp)
-		fprintf(output, "%s%s", csv_sep, evsel->cgrp->name);
-}
-
 static int first_shadow_cpu(struct perf_evsel *evsel, int id)
 {
 	int i;
@@ -1241,11 +1201,7 @@ static void printout(int id, int nr, struct perf_evsel *counter, double uval,
 		return;
 	}
 
-	if (metric_only)
-		/* nothing */;
-	else if (nsec_counter(counter))
-		nsec_printout(id, nr, counter, uval);
-	else
+	if (!metric_only)
 		abs_printout(id, nr, counter, uval);
 
 	out.print_metric = pm;
@@ -1331,7 +1287,7 @@ static void collect_all_aliases(struct perf_evsel *counter,
 		    alias->scale != counter->scale ||
 		    alias->cgrp != counter->cgrp ||
 		    strcmp(alias->unit, counter->unit) ||
-		    nsec_counter(alias) != nsec_counter(counter))
+		    perf_evsel__is_clock(alias) != perf_evsel__is_clock(counter))
 			break;
 		alias->merged_stat = true;
 		cb(alias, data, false);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 94fce4f537e9..5285da0417c5 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -260,6 +260,17 @@ struct perf_evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
 		evsel->attr.sample_period = 1;
 	}
 
+	if (perf_evsel__is_clock(evsel)) {
+		/*
+		 * The evsel->unit points to static alias->unit
+		 * so it's ok to use static string in here.
+		 */
+		static const char *unit = "msec";
+
+		evsel->unit = unit;
+		evsel->scale = 1e-6;
+	}
+
 	return evsel;
 }
 
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 890babf9ce86..973c03167947 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -405,6 +405,12 @@ static inline bool perf_evsel__is_bpf_output(struct perf_evsel *evsel)
 	return perf_evsel__match(evsel, SOFTWARE, SW_BPF_OUTPUT);
 }
 
+static inline bool perf_evsel__is_clock(struct perf_evsel *evsel)
+{
+	return perf_evsel__match(evsel, SOFTWARE, SW_CPU_CLOCK) ||
+	       perf_evsel__match(evsel, SOFTWARE, SW_TASK_CLOCK);
+}
+
 struct perf_attr_details {
 	bool freq;
 	bool verbose;
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 594d14a02b67..99990f5f2512 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -913,11 +913,10 @@ void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
 			ratio = total / avg;
 
 		print_metric(ctxp, NULL, "%8.0f", "cycles / elision", ratio);
-	} else if (perf_evsel__match(evsel, SOFTWARE, SW_TASK_CLOCK) ||
-		   perf_evsel__match(evsel, SOFTWARE, SW_CPU_CLOCK)) {
+	} else if (perf_evsel__is_clock(evsel)) {
 		if ((ratio = avg_stats(&walltime_nsecs_stats)) != 0)
 			print_metric(ctxp, NULL, "%8.3f", "CPUs utilized",
-				     avg / ratio);
+				     avg / (ratio * evsel->scale));
 		else
 			print_metric(ctxp, NULL, NULL, "CPUs utilized", 0);
 	} else if (perf_stat_evsel__is(evsel, TOPDOWN_FETCH_BUBBLES)) {

[PATCH 3/4] perf tools: Fix check-headers.sh output file variables

[Jiri Olsa]

The warning message in check_w function uses wrongly
the $file variable instead of $file1 and $file2.

Fixes: 582472973593 ("perf check-headers.sh: Add support to check 2 independent files")
Link: http://lkml.kernel.org/n/tip-oh56ckqztoc07we7mtdphu7r@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/check-headers.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index 814aaf269949..73e723675c5f 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -67,7 +67,7 @@ check_2 () {
   cmd="diff $* $file1 $file2 > /dev/null"
 
   test -f $file2 &&
-  eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
+  eval $cmd || echo "Warning: Kernel ABI header at '$file1' differs from latest version at '$file2'" >&2
 }
 
 check () {
-- 
2.17.1

Re: [PATCH 3/4] perf tools: Fix check-headers.sh output file variables

[Arnaldo Carvalho de Melo]

Em Fri, Jul 20, 2018 at 01:00:35PM +0200, Jiri Olsa escreveu:
> The warning message in check_w function uses wrongly
> the $file variable instead of $file1 and $file2.

Humm,

Before:

Warning: Kernel ABI header at 'tools/arch/powerpc/include/uapi/asm/unistd.h' differs from latest version at 'arch/powerpc/include/uapi/asm/unistd.h'

After:

Warning: Kernel ABI header at '../arch/powerpc/include/uapi/asm/unistd.h' differs from latest version at '../../arch/powerpc/include/uapi/asm/unistd.h'


The previous version is better, I can then just use:

diff -u tools/arch/powerpc/include/uapi/asm/unistd.h arch/powerpc/include/uapi/asm/unistd.h

and get what changed, with your change I have to go to tools/perf before
doing that diff, which is an unnecessary extra step in at least my
workflow.

- Arnaldo

 
> Fixes: 582472973593 ("perf check-headers.sh: Add support to check 2 independent files")
> Link: http://lkml.kernel.org/n/tip-oh56ckqztoc07we7mtdphu7r@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/check-headers.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> index 814aaf269949..73e723675c5f 100755
> --- a/tools/perf/check-headers.sh
> +++ b/tools/perf/check-headers.sh
> @@ -67,7 +67,7 @@ check_2 () {
>    cmd="diff $* $file1 $file2 > /dev/null"
>  
>    test -f $file2 &&
> -  eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
> +  eval $cmd || echo "Warning: Kernel ABI header at '$file1' differs from latest version at '$file2'" >&2
>  }
>  
>  check () {
> -- 
> 2.17.1

Re: [PATCH 3/4] perf tools: Fix check-headers.sh output file variables

[Jiri Olsa]

On Fri, Jul 20, 2018 at 11:57:45AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jul 20, 2018 at 01:00:35PM +0200, Jiri Olsa escreveu:
> > The warning message in check_w function uses wrongly
> > the $file variable instead of $file1 and $file2.
> 
> Humm,
> 
> Before:
> 
> Warning: Kernel ABI header at 'tools/arch/powerpc/include/uapi/asm/unistd.h' differs from latest version at 'arch/powerpc/include/uapi/asm/unistd.h'
> 
> After:
> 
> Warning: Kernel ABI header at '../arch/powerpc/include/uapi/asm/unistd.h' differs from latest version at '../../arch/powerpc/include/uapi/asm/unistd.h'
> 
> 
> The previous version is better, I can then just use:
> 
> diff -u tools/arch/powerpc/include/uapi/asm/unistd.h arch/powerpc/include/uapi/asm/unistd.h
> 
> and get what changed, with your change I have to go to tools/perf before
> doing that diff, which is an unnecessary extra step in at least my
> workflow.

so all paths output based in kernel tree root then, will change

jirka

Re: [PATCH 3/4] perf tools: Fix check-headers.sh output file variables

[Alexander Kapshuk]

On Fri, Jul 20, 2018 at 6:16 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Jul 20, 2018 at 11:57:45AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Jul 20, 2018 at 01:00:35PM +0200, Jiri Olsa escreveu:
> > > The warning message in check_w function uses wrongly
> > > the $file variable instead of $file1 and $file2.
> >
> > Humm,
> >
> > Before:
> >
> > Warning: Kernel ABI header at 'tools/arch/powerpc/include/uapi/asm/unistd.h' differs from latest version at 'arch/powerpc/include/uapi/asm/unistd.h'
> >
> > After:
> >
> > Warning: Kernel ABI header at '../arch/powerpc/include/uapi/asm/unistd.h' differs from latest version at '../../arch/powerpc/include/uapi/asm/unistd.h'
> >
> >
> > The previous version is better, I can then just use:
> >
> > diff -u tools/arch/powerpc/include/uapi/asm/unistd.h arch/powerpc/include/uapi/asm/unistd.h
> >
> > and get what changed, with your change I have to go to tools/perf before
> > doing that diff, which is an unnecessary extra step in at least my
> > workflow.
>
> so all paths output based in kernel tree root then, will change
>
> jirka

I was going to ask about this in a separate email initially, but then
thought I'd use this email exchange instead, as my question is about
the code in question. Hope you don't mind.

If I'm reading this right, the intended behavoir of the block of code
below is to test file2 for existance, and if it exists, to evaluate $cmd.
If file1 and file2 are found to differ, print the warning.

test -f $file2 &&
  eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file'
differs from latest version at '$file'" >&2

The '||' path of execution is however also taken if file2 doesn't exist,
which is probably very unlikely to happen. See below.

% file1=file1; file2=file2
% cmd="echo diff $file1 $file2"
% test -f $file2 &&
eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file1'
differs from latest version at '$file2'" >&2
Warning: Kernel ABI header at 'tools/file1' differs from latest
version at 'file2'

Is this something you would rather leave as is, or perhaps use something
along the lines of the code below instead:

test -f $file2 && {
eval $cmd ||
echo "Warning: Kernel ABI header at 'tools/$file' differs from latest
version at '$file'" >&2
}

Thanks.

Re: [PATCH 3/4] perf tools: Fix check-headers.sh output file variables

[Jiri Olsa]

On Fri, Jul 20, 2018 at 06:22:49PM +0300, Alexander Kapshuk wrote:
> On Fri, Jul 20, 2018 at 6:16 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Fri, Jul 20, 2018 at 11:57:45AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Fri, Jul 20, 2018 at 01:00:35PM +0200, Jiri Olsa escreveu:
> > > > The warning message in check_w function uses wrongly
> > > > the $file variable instead of $file1 and $file2.
> > >
> > > Humm,
> > >
> > > Before:
> > >
> > > Warning: Kernel ABI header at 'tools/arch/powerpc/include/uapi/asm/unistd.h' differs from latest version at 'arch/powerpc/include/uapi/asm/unistd.h'
> > >
> > > After:
> > >
> > > Warning: Kernel ABI header at '../arch/powerpc/include/uapi/asm/unistd.h' differs from latest version at '../../arch/powerpc/include/uapi/asm/unistd.h'
> > >
> > >
> > > The previous version is better, I can then just use:
> > >
> > > diff -u tools/arch/powerpc/include/uapi/asm/unistd.h arch/powerpc/include/uapi/asm/unistd.h
> > >
> > > and get what changed, with your change I have to go to tools/perf before
> > > doing that diff, which is an unnecessary extra step in at least my
> > > workflow.
> >
> > so all paths output based in kernel tree root then, will change
> >
> > jirka
> 
> I was going to ask about this in a separate email initially, but then
> thought I'd use this email exchange instead, as my question is about
> the code in question. Hope you don't mind.
> 
> If I'm reading this right, the intended behavoir of the block of code
> below is to test file2 for existance, and if it exists, to evaluate $cmd.
> If file1 and file2 are found to differ, print the warning.
> 
> test -f $file2 &&
>   eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file'
> differs from latest version at '$file'" >&2
> 
> The '||' path of execution is however also taken if file2 doesn't exist,
> which is probably very unlikely to happen. See below.
> 
> % file1=file1; file2=file2
> % cmd="echo diff $file1 $file2"
> % test -f $file2 &&
> eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file1'
> differs from latest version at '$file2'" >&2
> Warning: Kernel ABI header at 'tools/file1' differs from latest
> version at 'file2'
> 
> Is this something you would rather leave as is, or perhaps use something
> along the lines of the code below instead:
> 
> test -f $file2 && {
> eval $cmd ||
> echo "Warning: Kernel ABI header at 'tools/$file' differs from latest
> version at '$file'" >&2
> }

hi,
yea, probably..  please feel free to post a patch.. just make sure all
the displayed files paths are based on the kernel root

thanks,
jirka

Re: [PATCH 3/4] perf tools: Fix check-headers.sh output file variables

[Jiri Olsa]

On Tue, Jul 24, 2018 at 08:20:07AM +0100, Alexander Kapshuk wrote:

SNIP

> > > % file1=file1; file2=file2
> > > % cmd="echo diff $file1 $file2"
> > > % test -f $file2 &&
> > > eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file1'
> > > differs from latest version at '$file2'" >&2
> > > Warning: Kernel ABI header at 'tools/file1' differs from latest
> > > version at 'file2'
> > >
> > > Is this something you would rather leave as is, or perhaps use something
> > > along the lines of the code below instead:
> > >
> > > test -f $file2 && {
> > > eval $cmd ||
> > > echo "Warning: Kernel ABI header at 'tools/$file' differs from latest
> > > version at '$file'" >&2
> > > }
> >
> > hi,
> > yea, probably..  please feel free to post a patch.. just make sure all
> > the displayed files paths are based on the kernel root
> >
> > thanks,
> > jirka
> >
> 
> I'm away traveling till August 10th, and I may not be able to send the
> patch in until I get back. Is that OK?

sure, thanks

jirka

[PATCH] perf tools: Fix check-headers.sh AND list path of execution

[Alexander Kapshuk]

The '||' path of execution in the 'test' block of the check_2() function
may also be taken if file2 does not exist, in which case the warning
message about the ABI headers being different would still be printed
where it should not be.  See below.

% file1=file1; file2=file2
% cmd="echo diff $file1 $file2"
% test -f $file2 &&
eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file1'
differs from latest version at '$file2'" >&2
Warning: Kernel ABI header at 'tools/file1' differs from latest
version at 'file2'

The proposed patch converts the code following the '&&' operator into a
compound list to be executed in the current process environment only if
file2 does exist. Should the files being compared differ, a diff command
to compare the files concerned is printed on standard output. E.g.

diff -u tools/arch/x86/lib/memcpy_64.S arch/x86/lib/memcpy_64.S

Signed-off-by: Alexander Kapshuk <alexander.kapshuk@gmail.com>
---
 tools/perf/check-headers.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index de28466c0186..ea48aa6f8d19 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -67,8 +67,12 @@ check_2 () {
 
   cmd="diff $* $file1 $file2 > /dev/null"
 
-  test -f $file2 &&
-  eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
+  test -f $file2 && {
+    eval $cmd || {
+      echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
+      echo diff -u tools/$file $file
+    }
+  }
 }
 
 check () {
-- 
2.18.0

[PATCH 1/2] perf tools: Make check-headers.sh check based on kernel dir

[Jiri Olsa]

Changing the logic to compare files with paths relative
to kernel source base dir. This way we can keep the output
message for 2 unrelated files, which is coming in following
patch.

Link: http://lkml.kernel.org/n/tip-gpfv1vdqfjda80m451w9a7mt@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/check-headers.sh | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index ea48aa6f8d19..9d466e853aec 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -69,8 +69,8 @@ check_2 () {
 
   test -f $file2 && {
     eval $cmd || {
-      echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
-      echo diff -u tools/$file $file
+      echo "Warning: Kernel ABI header at '$file1' differs from latest version at '$file2'" >&2
+      echo diff -u $file1 $file2
     }
   }
 }
@@ -80,7 +80,7 @@ check () {
 
   shift
 
-  check_2 ../$file ../../$file $*
+  check_2 tools/$file $file $*
 }
 
 # Check if we have the kernel headers (tools/perf/../../include), else
@@ -88,6 +88,8 @@ check () {
 # differences.
 test -d ../../include || exit 0
 
+pushd ../.. > /dev/null
+
 # simple diff check
 for i in $HEADERS; do
   check $i -B
@@ -98,3 +100,5 @@ check arch/x86/lib/memcpy_64.S        '-I "^EXPORT_SYMBOL" -I "^#include <asm/ex
 check arch/x86/lib/memset_64.S        '-I "^EXPORT_SYMBOL" -I "^#include <asm/export.h>"'
 check include/uapi/asm-generic/mman.h '-I "^#include <\(uapi/\)*asm-generic/mman-common.h>"'
 check include/uapi/linux/mman.h       '-I "^#include <\(uapi/\)*asm/mman.h>"'
+
+popd > /dev/null
-- 
2.17.1

[PATCH 2/2] perf tools: Move syscall_64.tbl check into check-headers.sh

[Jiri Olsa]

Probably leftover from the time we introducd the check-headers.sh script.

Link: http://lkml.kernel.org/n/tip-oh56ckqztoc07we7mtdphu7r@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/arch/x86/Makefile | 3 ---
 tools/perf/check-headers.sh  | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
index 1a38e78117ce..8cc6642fce7a 100644
--- a/tools/perf/arch/x86/Makefile
+++ b/tools/perf/arch/x86/Makefile
@@ -19,9 +19,6 @@ systbl := $(sys)/syscalltbl.sh
 _dummy := $(shell [ -d '$(out)' ] || mkdir -p '$(out)')
 
 $(header): $(sys)/syscall_64.tbl $(systbl)
-	@(test -d ../../kernel -a -d ../../tools -a -d ../perf && ( \
-        (diff -B arch/x86/entry/syscalls/syscall_64.tbl ../../arch/x86/entry/syscalls/syscall_64.tbl >/dev/null) \
-        || echo "Warning: Kernel ABI header at 'tools/perf/arch/x86/entry/syscalls/syscall_64.tbl' differs from latest version at 'arch/x86/entry/syscalls/syscall_64.tbl'" >&2 )) || true
 	$(Q)$(SHELL) '$(systbl)' $(sys)/syscall_64.tbl 'x86_64' > $@
 
 clean::
diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index 9d466e853aec..80bf84803677 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -101,4 +101,7 @@ check arch/x86/lib/memset_64.S        '-I "^EXPORT_SYMBOL" -I "^#include <asm/ex
 check include/uapi/asm-generic/mman.h '-I "^#include <\(uapi/\)*asm-generic/mman-common.h>"'
 check include/uapi/linux/mman.h       '-I "^#include <\(uapi/\)*asm/mman.h>"'
 
+# diff non-symmetric files
+check_2 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl
+
 popd > /dev/null
-- 
2.17.1

[tip:perf/urgent] perf tools: Move syscall_64.tbl check into check-headers.sh

[tip-bot for Jiri Olsa]

Commit-ID:  c9b51a017065bf514ab5a380f2fb3b0dada5bc68
Gitweb:     https://git.kernel.org/tip/c9b51a017065bf514ab5a380f2fb3b0dada5bc68
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 13 Aug 2018 13:15:04 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 14 Aug 2018 15:10:40 -0300

perf tools: Move syscall_64.tbl check into check-headers.sh

Probably leftover from the time we introducd the check-headers.sh script.

Committer testing:

Remove the 'rseq' syscall from tools/perf/arch/x86/entry/syscalls/syscall_64.tbl
to fake a diff:

make: Entering directory '/home/acme/git/perf/tools/perf'
  BUILD:   Doing 'make -j4' parallel build
Warning: Kernel ABI header at 'tools/perf/arch/x86/entry/syscalls/syscall_64.tbl' differs from latest version at 'arch/x86/entry/syscalls/syscall_64.tbl'
diff -u tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl
  CC       /tmp/build/perf/util/syscalltbl.o
  INSTALL  trace_plugins
<SNIP>
  $ diff -u tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl
  --- tools/perf/arch/x86/entry/syscalls/syscall_64.tbl	2018-08-13 15:49:50.896585176 -0300
  +++ arch/x86/entry/syscalls/syscall_64.tbl	2018-07-20 12:04:04.536858304 -0300
  @@ -342,6 +342,7 @@
   331	common	pkey_free		__x64_sys_pkey_free
   332	common	statx			__x64_sys_statx
   333	common	io_pgetevents		__x64_sys_io_pgetevents
  +334	common	rseq			__x64_sys_rseq

  #
  # x32-specific system call numbers start at 512 to avoid cache impact
  $

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Kapshuk <alexander.kapshuk@gmail.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180813111504.3568-2-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/x86/Makefile | 3 ---
 tools/perf/check-headers.sh  | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
index 1a38e78117ce..8cc6642fce7a 100644
--- a/tools/perf/arch/x86/Makefile
+++ b/tools/perf/arch/x86/Makefile
@@ -19,9 +19,6 @@ systbl := $(sys)/syscalltbl.sh
 _dummy := $(shell [ -d '$(out)' ] || mkdir -p '$(out)')
 
 $(header): $(sys)/syscall_64.tbl $(systbl)
-	@(test -d ../../kernel -a -d ../../tools -a -d ../perf && ( \
-        (diff -B arch/x86/entry/syscalls/syscall_64.tbl ../../arch/x86/entry/syscalls/syscall_64.tbl >/dev/null) \
-        || echo "Warning: Kernel ABI header at 'tools/perf/arch/x86/entry/syscalls/syscall_64.tbl' differs from latest version at 'arch/x86/entry/syscalls/syscall_64.tbl'" >&2 )) || true
 	$(Q)$(SHELL) '$(systbl)' $(sys)/syscall_64.tbl 'x86_64' > $@
 
 clean::
diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index b61f8a4dfca3..466540ee8ea7 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -101,4 +101,7 @@ check arch/x86/lib/memset_64.S        '-I "^EXPORT_SYMBOL" -I "^#include <asm/ex
 check include/uapi/asm-generic/mman.h '-I "^#include <\(uapi/\)*asm-generic/mman-common.h>"'
 check include/uapi/linux/mman.h       '-I "^#include <\(uapi/\)*asm/mman.h>"'
 
+# diff non-symmetric files
+check_2 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl
+
 cd tools/perf

Re: [PATCH 1/2] perf tools: Make check-headers.sh check based on kernel dir

[Michael Ellerman]

Jiri Olsa <jolsa@kernel.org> writes:
> diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> index ea48aa6f8d19..9d466e853aec 100755
> --- a/tools/perf/check-headers.sh
> +++ b/tools/perf/check-headers.sh
> @@ -88,6 +88,8 @@ check () {
>  # differences.
>  test -d ../../include || exit 0
>  
> +pushd ../.. > /dev/null
> +
>  # simple diff check
>  for i in $HEADERS; do
>    check $i -B

This breaks the build when sh is not bash:

  ./check-headers.sh: 91: ./check-headers.sh: pushd: not found
  ./check-headers.sh: 107: ./check-headers.sh: popd: not found
  Makefile.perf:205: recipe for target 'sub-make' failed

cheers

Re: [PATCH 1/2] perf tools: Make check-headers.sh check based on kernel dir

[Jiri Olsa]

On Tue, Aug 14, 2018 at 11:47:39AM +1000, Michael Ellerman wrote:
> Jiri Olsa <jolsa@kernel.org> writes:
> > diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> > index ea48aa6f8d19..9d466e853aec 100755
> > --- a/tools/perf/check-headers.sh
> > +++ b/tools/perf/check-headers.sh
> > @@ -88,6 +88,8 @@ check () {
> >  # differences.
> >  test -d ../../include || exit 0
> >  
> > +pushd ../.. > /dev/null
> > +
> >  # simple diff check
> >  for i in $HEADERS; do
> >    check $i -B
> 
> This breaks the build when sh is not bash:
> 
>   ./check-headers.sh: 91: ./check-headers.sh: pushd: not found
>   ./check-headers.sh: 107: ./check-headers.sh: popd: not found
>   Makefile.perf:205: recipe for target 'sub-make' failed

sry.. Arnaldo, would you change it for simple cd (attached below)
or should I send the fix?

thanks,
jirka


---
diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index 80bf84803677..466540ee8ea7 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -88,7 +88,7 @@ check () {
 # differences.
 test -d ../../include || exit 0
 
-pushd ../.. > /dev/null
+cd ../..
 
 # simple diff check
 for i in $HEADERS; do
@@ -104,4 +104,4 @@ check include/uapi/linux/mman.h       '-I "^#include <\(uapi/\)*asm/mman.h>"'
 # diff non-symmetric files
 check_2 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl
 
-popd > /dev/null
+cd tools/perf

Re: [PATCH 1/2] perf tools: Make check-headers.sh check based on kernel dir

[Arnaldo Carvalho de Melo]

Em Tue, Aug 14, 2018 at 09:27:26AM +0200, Jiri Olsa escreveu:
> On Tue, Aug 14, 2018 at 11:47:39AM +1000, Michael Ellerman wrote:
> > Jiri Olsa <jolsa@kernel.org> writes:
> > > diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> > > index ea48aa6f8d19..9d466e853aec 100755
> > > --- a/tools/perf/check-headers.sh
> > > +++ b/tools/perf/check-headers.sh
> > > @@ -88,6 +88,8 @@ check () {
> > >  # differences.
> > >  test -d ../../include || exit 0
> > >  
> > > +pushd ../.. > /dev/null
> > > +
> > >  # simple diff check
> > >  for i in $HEADERS; do
> > >    check $i -B
> > 
> > This breaks the build when sh is not bash:
> > 
> >   ./check-headers.sh: 91: ./check-headers.sh: pushd: not found
> >   ./check-headers.sh: 107: ./check-headers.sh: popd: not found
> >   Makefile.perf:205: recipe for target 'sub-make' failed
> 
> sry.. Arnaldo, would you change it for simple cd (attached below)
> or should I send the fix?

Nah, I'm folding this in, to keep it bisectable.
 
> thanks,
> jirka
> 
> 
> ---
> diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> index 80bf84803677..466540ee8ea7 100755
> --- a/tools/perf/check-headers.sh
> +++ b/tools/perf/check-headers.sh
> @@ -88,7 +88,7 @@ check () {
>  # differences.
>  test -d ../../include || exit 0
>  
> -pushd ../.. > /dev/null
> +cd ../..
>  
>  # simple diff check
>  for i in $HEADERS; do
> @@ -104,4 +104,4 @@ check include/uapi/linux/mman.h       '-I "^#include <\(uapi/\)*asm/mman.h>"'
>  # diff non-symmetric files
>  check_2 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl
>  
> -popd > /dev/null
> +cd tools/perf

Re: [PATCH 1/2] perf tools: Make check-headers.sh check based on kernel dir

[Jiri Olsa]

On Tue, Aug 14, 2018 at 03:06:44PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Aug 14, 2018 at 09:27:26AM +0200, Jiri Olsa escreveu:
> > On Tue, Aug 14, 2018 at 11:47:39AM +1000, Michael Ellerman wrote:
> > > Jiri Olsa <jolsa@kernel.org> writes:
> > > > diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> > > > index ea48aa6f8d19..9d466e853aec 100755
> > > > --- a/tools/perf/check-headers.sh
> > > > +++ b/tools/perf/check-headers.sh
> > > > @@ -88,6 +88,8 @@ check () {
> > > >  # differences.
> > > >  test -d ../../include || exit 0
> > > >  
> > > > +pushd ../.. > /dev/null
> > > > +
> > > >  # simple diff check
> > > >  for i in $HEADERS; do
> > > >    check $i -B
> > > 
> > > This breaks the build when sh is not bash:
> > > 
> > >   ./check-headers.sh: 91: ./check-headers.sh: pushd: not found
> > >   ./check-headers.sh: 107: ./check-headers.sh: popd: not found
> > >   Makefile.perf:205: recipe for target 'sub-make' failed
> > 
> > sry.. Arnaldo, would you change it for simple cd (attached below)
> > or should I send the fix?
> 
> Nah, I'm folding this in, to keep it bisectable.

any chance one of your docker tests could run build in sh/zsh? ;-)

thanks,
jirka

Re: [PATCH 1/2] perf tools: Make check-headers.sh check based on kernel dir

[Michael Ellerman]

Jiri Olsa <jolsa@redhat.com> writes:
> On Tue, Aug 14, 2018 at 03:06:44PM -0300, Arnaldo Carvalho de Melo wrote:
>> Em Tue, Aug 14, 2018 at 09:27:26AM +0200, Jiri Olsa escreveu:
>> > On Tue, Aug 14, 2018 at 11:47:39AM +1000, Michael Ellerman wrote:
>> > > Jiri Olsa <jolsa@kernel.org> writes:
>> > > > diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
>> > > > index ea48aa6f8d19..9d466e853aec 100755
>> > > > --- a/tools/perf/check-headers.sh
>> > > > +++ b/tools/perf/check-headers.sh
>> > > > @@ -88,6 +88,8 @@ check () {
>> > > >  # differences.
>> > > >  test -d ../../include || exit 0
>> > > >  
>> > > > +pushd ../.. > /dev/null
>> > > > +
>> > > >  # simple diff check
>> > > >  for i in $HEADERS; do
>> > > >    check $i -B
>> > > 
>> > > This breaks the build when sh is not bash:
>> > > 
>> > >   ./check-headers.sh: 91: ./check-headers.sh: pushd: not found
>> > >   ./check-headers.sh: 107: ./check-headers.sh: popd: not found
>> > >   Makefile.perf:205: recipe for target 'sub-make' failed
>> > 
>> > sry.. Arnaldo, would you change it for simple cd (attached below)
>> > or should I send the fix?
>> 
>> Nah, I'm folding this in, to keep it bisectable.
>
> any chance one of your docker tests could run build in sh/zsh? ;-)

Just using an Ubuntu image, where /bin/sh == dash should work, that's
how I hit it.

cheers

Re: [PATCH 1/2] perf tools: Make check-headers.sh check based on kernel dir

[Arnaldo Carvalho de Melo]

Em Wed, Aug 15, 2018 at 08:02:48PM +1000, Michael Ellerman escreveu:
> Jiri Olsa <jolsa@redhat.com> writes:
> > On Tue, Aug 14, 2018 at 03:06:44PM -0300, Arnaldo Carvalho de Melo wrote:
> >> Em Tue, Aug 14, 2018 at 09:27:26AM +0200, Jiri Olsa escreveu:
> >> > sry.. Arnaldo, would you change it for simple cd (attached below)
> >> > or should I send the fix?

> >> Nah, I'm folding this in, to keep it bisectable.

> > any chance one of your docker tests could run build in sh/zsh? ;-)

It does already, see below :-)

> Just using an Ubuntu image, where /bin/sh == dash should work, that's
> how I hit it.

So, I do the tests only prior to pushing to Ingo, so didn't catch this,
lemme check, put that change back on, start a ubuntu:18.04 perf build
container, try to build, see if it would fail, yeah, I'd have detected
this before pushing to Ingo, so probably I have to run the tests before
pushing to my acme/perf/core branch, will try to operate like that from
now on.

[root@jouet perf]# git diff
diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index 466540ee8ea7..80bf84803677 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -88,7 +88,7 @@ check () {
 # differences.
 test -d ../../include || exit 0
 
-cd ../..
+pushd ../.. > /dev/null
 
 # simple diff check
 for i in $HEADERS; do
@@ -104,4 +104,4 @@ check include/uapi/linux/mman.h       '-I "^#include <\(uapi/\)*asm/mman.h>"'
 # diff non-symmetric files
 check_2 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl
 
-cd tools/perf
+popd > /dev/null
[root@jouet perf]# docker run --privileged --entrypoint=/bin/sh -v /home/acme/git:/git:Z --rm -ti docker.io/acmel/linux-perf-tools-build-ubuntu:18.04
$ bash
perfbuilder@65ead4a4734a:/$ cd /git/perf
perfbuilder@65ead4a4734a:/git/perf$ make -C tools/perf O=/tmp/build/perf/ install-bin
make: Entering directory '/git/perf/tools/perf'
  BUILD:   Doing 'make -j4' parallel build
  HOSTCC   /tmp/build/perf/fixdep.o
  HOSTLD   /tmp/build/perf/fixdep-in.o
  LINK     /tmp/build/perf/fixdep
./check-headers.sh: 91: ./check-headers.sh: pushd: not found
diff: tools/perf/arch/x86/entry/syscalls/syscall_64.tbl: No such file or directory
Warning: Kernel ABI header at 'tools/perf/arch/x86/entry/syscalls/syscall_64.tbl' differs from latest version at 'arch/x86/entry/syscalls/syscall_64.tbl'
diff -u tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl
./check-headers.sh: 107: ./check-headers.sh: popd: not found
Makefile.perf:205: recipe for target 'sub-make' failed
make[1]: *** [sub-make] Error 127
Makefile:109: recipe for target 'install-bin' failed
make: *** [install-bin] Error 2
make: Leaving directory '/git/perf/tools/perf'
perfbuilder@65ead4a4734a:/git/perf$

Re: [PATCH 1/2] perf tools: Make check-headers.sh check based on kernel dir

[Michael Ellerman]

Arnaldo Carvalho de Melo <acme@kernel.org> writes:
> Em Wed, Aug 15, 2018 at 08:02:48PM +1000, Michael Ellerman escreveu:
>> Jiri Olsa <jolsa@redhat.com> writes:
>> > On Tue, Aug 14, 2018 at 03:06:44PM -0300, Arnaldo Carvalho de Melo wrote:
>> >> Em Tue, Aug 14, 2018 at 09:27:26AM +0200, Jiri Olsa escreveu:
>> >> > sry.. Arnaldo, would you change it for simple cd (attached below)
>> >> > or should I send the fix?
>
>> >> Nah, I'm folding this in, to keep it bisectable.
>
>> > any chance one of your docker tests could run build in sh/zsh? ;-)
>
> It does already, see below :-)
>
>> Just using an Ubuntu image, where /bin/sh == dash should work, that's
>> how I hit it.
>
> So, I do the tests only prior to pushing to Ingo, so didn't catch this,
> lemme check, put that change back on, start a ubuntu:18.04 perf build
> container, try to build, see if it would fail, yeah, I'd have detected
> this before pushing to Ingo, so probably I have to run the tests before
> pushing to my acme/perf/core branch, will try to operate like that from
> now on.

Or you can tell me to test a different branch :)

cheers

[tip:perf/urgent] perf tools: Make check-headers.sh check based on kernel dir

[tip-bot for Jiri Olsa]

Commit-ID:  7ea6e983b2cce9a4277ea602c2976d18bd75a908
Gitweb:     https://git.kernel.org/tip/7ea6e983b2cce9a4277ea602c2976d18bd75a908
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 13 Aug 2018 13:15:03 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 14 Aug 2018 15:08:33 -0300

perf tools: Make check-headers.sh check based on kernel dir

Changing the logic to compare files with paths relative to kernel source
base dir. This way we can keep the output message for 2 unrelated files,
which is coming in following patch.

Committer testing:

Remove a line from tools/arch/x86/lib/memcpy_64.S to have it detected:

make: Entering directory '/home/acme/git/perf/tools/perf'
  BUILD:   Doing 'make -j4' parallel build
Warning: Kernel ABI header at 'tools/arch/x86/lib/memcpy_64.S' differs from latest version at 'arch/x86/lib/memcpy_64.S'
diff -u tools/arch/x86/lib/memcpy_64.S arch/x86/lib/memcpy_64.S
  INSTALL  GTK UI
  INSTALL  binaries

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Kapshuk <alexander.kapshuk@gmail.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180813111504.3568-1-jolsa@kernel.org
Link: http://lkml.kernel.org/r/20180814072726.GA13931@krava
[ Do not use pushd/popd, its a bashism, reported by Michael Ellerman, fixed by Jiri Olsa ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/check-headers.sh | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index ea48aa6f8d19..b61f8a4dfca3 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -69,8 +69,8 @@ check_2 () {
 
   test -f $file2 && {
     eval $cmd || {
-      echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
-      echo diff -u tools/$file $file
+      echo "Warning: Kernel ABI header at '$file1' differs from latest version at '$file2'" >&2
+      echo diff -u $file1 $file2
     }
   }
 }
@@ -80,7 +80,7 @@ check () {
 
   shift
 
-  check_2 ../$file ../../$file $*
+  check_2 tools/$file $file $*
 }
 
 # Check if we have the kernel headers (tools/perf/../../include), else
@@ -88,6 +88,8 @@ check () {
 # differences.
 test -d ../../include || exit 0
 
+cd ../..
+
 # simple diff check
 for i in $HEADERS; do
   check $i -B
@@ -98,3 +100,5 @@ check arch/x86/lib/memcpy_64.S        '-I "^EXPORT_SYMBOL" -I "^#include <asm/ex
 check arch/x86/lib/memset_64.S        '-I "^EXPORT_SYMBOL" -I "^#include <asm/export.h>"'
 check include/uapi/asm-generic/mman.h '-I "^#include <\(uapi/\)*asm-generic/mman-common.h>"'
 check include/uapi/linux/mman.h       '-I "^#include <\(uapi/\)*asm/mman.h>"'
+
+cd tools/perf

Re: [PATCH] perf tools: Fix check-headers.sh AND list path of execution

[Jiri Olsa]

On Sat, Aug 11, 2018 at 11:39:15AM +0300, Alexander Kapshuk wrote:
> The '||' path of execution in the 'test' block of the check_2() function
> may also be taken if file2 does not exist, in which case the warning
> message about the ABI headers being different would still be printed
> where it should not be.  See below.
> 
> % file1=file1; file2=file2
> % cmd="echo diff $file1 $file2"
> % test -f $file2 &&
> eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file1'
> differs from latest version at '$file2'" >&2
> Warning: Kernel ABI header at 'tools/file1' differs from latest
> version at 'file2'
> 
> The proposed patch converts the code following the '&&' operator into a
> compound list to be executed in the current process environment only if
> file2 does exist. Should the files being compared differ, a diff command
> to compare the files concerned is printed on standard output. E.g.
> 
> diff -u tools/arch/x86/lib/memcpy_64.S arch/x86/lib/memcpy_64.S
> 
> Signed-off-by: Alexander Kapshuk <alexander.kapshuk@gmail.com>

I posted follow up patches based on this one
as a reply on this patch

for this one:

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

thanks,
jirka

> ---
>  tools/perf/check-headers.sh | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> index de28466c0186..ea48aa6f8d19 100755
> --- a/tools/perf/check-headers.sh
> +++ b/tools/perf/check-headers.sh
> @@ -67,8 +67,12 @@ check_2 () {
>  
>    cmd="diff $* $file1 $file2 > /dev/null"
>  
> -  test -f $file2 &&
> -  eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
> +  test -f $file2 && {
> +    eval $cmd || {
> +      echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
> +      echo diff -u tools/$file $file
> +    }
> +  }
>  }
>  
>  check () {
> -- 
> 2.18.0
> 

Re: [PATCH] perf tools: Fix check-headers.sh AND list path of execution

[Alexander Kapshuk]

On Mon, Aug 13, 2018 at 2:16 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Sat, Aug 11, 2018 at 11:39:15AM +0300, Alexander Kapshuk wrote:
> > The '||' path of execution in the 'test' block of the check_2() function
> > may also be taken if file2 does not exist, in which case the warning
> > message about the ABI headers being different would still be printed
> > where it should not be.  See below.
> >
> > % file1=file1; file2=file2
> > % cmd="echo diff $file1 $file2"
> > % test -f $file2 &&
> > eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file1'
> > differs from latest version at '$file2'" >&2
> > Warning: Kernel ABI header at 'tools/file1' differs from latest
> > version at 'file2'
> >
> > The proposed patch converts the code following the '&&' operator into a
> > compound list to be executed in the current process environment only if
> > file2 does exist. Should the files being compared differ, a diff command
> > to compare the files concerned is printed on standard output. E.g.
> >
> > diff -u tools/arch/x86/lib/memcpy_64.S arch/x86/lib/memcpy_64.S
> >
> > Signed-off-by: Alexander Kapshuk <alexander.kapshuk@gmail.com>
>
> I posted follow up patches based on this one
> as a reply on this patch
>
> for this one:
>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
>
> thanks,
> jirka

Noted.
Thanks.


>
> > ---
> >  tools/perf/check-headers.sh | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> > index de28466c0186..ea48aa6f8d19 100755
> > --- a/tools/perf/check-headers.sh
> > +++ b/tools/perf/check-headers.sh
> > @@ -67,8 +67,12 @@ check_2 () {
> >
> >    cmd="diff $* $file1 $file2 > /dev/null"
> >
> > -  test -f $file2 &&
> > -  eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
> > +  test -f $file2 && {
> > +    eval $cmd || {
> > +      echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
> > +      echo diff -u tools/$file $file
> > +    }
> > +  }
> >  }
> >
> >  check () {
> > --
> > 2.18.0
> >

Re: [PATCH] perf tools: Fix check-headers.sh AND list path of execution

[Arnaldo Carvalho de Melo]

Em Mon, Aug 13, 2018 at 02:19:10PM +0300, Alexander Kapshuk escreveu:
> On Mon, Aug 13, 2018 at 2:16 PM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Sat, Aug 11, 2018 at 11:39:15AM +0300, Alexander Kapshuk wrote:
> > > The '||' path of execution in the 'test' block of the check_2() function
> > > may also be taken if file2 does not exist, in which case the warning
> > > message about the ABI headers being different would still be printed
> > > where it should not be.  See below.
> > >
> > > % file1=file1; file2=file2
> > > % cmd="echo diff $file1 $file2"
> > > % test -f $file2 &&
> > > eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file1'
> > > differs from latest version at '$file2'" >&2
> > > Warning: Kernel ABI header at 'tools/file1' differs from latest
> > > version at 'file2'
> > >
> > > The proposed patch converts the code following the '&&' operator into a
> > > compound list to be executed in the current process environment only if
> > > file2 does exist. Should the files being compared differ, a diff command
> > > to compare the files concerned is printed on standard output. E.g.
> > >
> > > diff -u tools/arch/x86/lib/memcpy_64.S arch/x86/lib/memcpy_64.S
> > >
> > > Signed-off-by: Alexander Kapshuk <alexander.kapshuk@gmail.com>
> >
> > I posted follow up patches based on this one
> > as a reply on this patch
> >
> > for this one:
> >
> > Acked-by: Jiri Olsa <jolsa@kernel.org>
> >
> > thanks,
> > jirka
> 
> Noted.
> Thanks.

Thanks, applied the three patches to acme/perf/core.

- Arnaldo

Re: [PATCH] perf tools: Fix check-headers.sh AND list path of execution

[Alexander Kapshuk]

On Mon, Aug 13, 2018 at 9:58 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
> Thanks, applied the three patches to acme/perf/core.
>
> - Arnaldo

Thanks.

[tip:perf/urgent] perf tools: Fix check-headers.sh AND list path of execution

[tip-bot for Alexander Kapshuk]

Commit-ID:  51d8aac236493833acf60d9c3b6c42437a18da36
Gitweb:     https://git.kernel.org/tip/51d8aac236493833acf60d9c3b6c42437a18da36
Author:     Alexander Kapshuk <alexander.kapshuk@gmail.com>
AuthorDate: Sat, 11 Aug 2018 11:39:15 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 13 Aug 2018 15:46:19 -0300

perf tools: Fix check-headers.sh AND list path of execution

The '||' path of execution in the 'test' block of the check_2() function
may also be taken if file2 does not exist, in which case the warning
message about the ABI headers being different would still be printed
where it should not be.  See below.

  % file1=file1; file2=file2
  % cmd="echo diff $file1 $file2"
  % test -f $file2 && \
    eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file1'
                       differs from latest version at '$file2'" >&2
                       Warning: Kernel ABI header at 'tools/file1' differs from latest
                       version at 'file2'

The proposed patch converts the code following the '&&' operator into a
compound list to be executed in the current process environment only if file2
does exist. Should the files being compared differ, a diff command to compare
the files concerned is printed on standard output. E.g.

  $ diff -u tools/arch/x86/lib/memcpy_64.S arch/x86/lib/memcpy_64.S

Committer testing:

Remove a line from that tools/arch/x86/lib/memcpy_64.S file to test
this:

  BUILD:   Doing 'make -j4' parallel build
Warning: Kernel ABI header at 'tools/arch/x86/lib/memcpy_64.S' differs from latest version at 'arch/x86/lib/memcpy_64.S'
diff -u tools/arch/x86/lib/memcpy_64.S arch/x86/lib/memcpy_64.S
  CC       /tmp/build/perf/bench/mem-memcpy-x86-64-asm.o

Signed-off-by: Alexander Kapshuk <alexander.kapshuk@gmail.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180811083915.17471-1-alexander.kapshuk@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/check-headers.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index de28466c0186..ea48aa6f8d19 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -67,8 +67,12 @@ check_2 () {
 
   cmd="diff $* $file1 $file2 > /dev/null"
 
-  test -f $file2 &&
-  eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
+  test -f $file2 && {
+    eval $cmd || {
+      echo "Warning: Kernel ABI header at 'tools/$file' differs from latest version at '$file'" >&2
+      echo diff -u tools/$file $file
+    }
+  }
 }
 
 check () {

[PATCH 4/4] perf tools: Move syscall_64.tbl check into check-headers.sh

[Jiri Olsa]

Probably leftover from the time we introducd the check-headers.sh script.

Link: http://lkml.kernel.org/n/tip-oh56ckqztoc07we7mtdphu7r@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/arch/x86/Makefile | 3 ---
 tools/perf/check-headers.sh  | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
index 1a38e78117ce..8cc6642fce7a 100644
--- a/tools/perf/arch/x86/Makefile
+++ b/tools/perf/arch/x86/Makefile
@@ -19,9 +19,6 @@ systbl := $(sys)/syscalltbl.sh
 _dummy := $(shell [ -d '$(out)' ] || mkdir -p '$(out)')
 
 $(header): $(sys)/syscall_64.tbl $(systbl)
-	@(test -d ../../kernel -a -d ../../tools -a -d ../perf && ( \
-        (diff -B arch/x86/entry/syscalls/syscall_64.tbl ../../arch/x86/entry/syscalls/syscall_64.tbl >/dev/null) \
-        || echo "Warning: Kernel ABI header at 'tools/perf/arch/x86/entry/syscalls/syscall_64.tbl' differs from latest version at 'arch/x86/entry/syscalls/syscall_64.tbl'" >&2 )) || true
 	$(Q)$(SHELL) '$(systbl)' $(sys)/syscall_64.tbl 'x86_64' > $@
 
 clean::
diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
index 73e723675c5f..2c10e46d98e9 100755
--- a/tools/perf/check-headers.sh
+++ b/tools/perf/check-headers.sh
@@ -93,3 +93,6 @@ check arch/x86/lib/memcpy_64.S        '-I "^EXPORT_SYMBOL" -I "^#include <asm/ex
 check arch/x86/lib/memset_64.S        '-I "^EXPORT_SYMBOL" -I "^#include <asm/export.h>"'
 check include/uapi/asm-generic/mman.h '-I "^#include <\(uapi/\)*asm-generic/mman-common.h>"'
 check include/uapi/linux/mman.h       '-I "^#include <\(uapi/\)*asm/mman.h>"'
+
+# diff non-symmetric files
+check_2 arch/x86/entry/syscalls/syscall_64.tbl ../../arch/x86/entry/syscalls/syscall_64.tbl
-- 
2.17.1

Re: [PATCH 4/4] perf tools: Move syscall_64.tbl check into check-headers.sh

[Arnaldo Carvalho de Melo]

Em Fri, Jul 20, 2018 at 01:00:36PM +0200, Jiri Olsa escreveu:
> Probably leftover from the time we introducd the check-headers.sh script.

So, with your patch:

[acme@jouet perf]$ git diff
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index f0b1709a5ffb..cfc1b0949bb1 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -343,6 +343,7 @@
 332    common  statx                   __x64_sys_statx
 333    common  io_pgetevents           __x64_sys_io_pgetevents
 334    common  rseq                    __x64_sys_rseq
+335    common  krava                   __x64_sys_krava
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
[acme@jouet perf]$


It doesn't warn about that new cool syscall, without your patch:

Warning: Kernel ABI header at 'tools/perf/arch/x86/entry/syscalls/syscall_64.tbl' differs from latest version at 'arch/x86/entry/syscalls/syscall_64.tbl'

Can you check this?

- Arnaldo
 
> Link: http://lkml.kernel.org/n/tip-oh56ckqztoc07we7mtdphu7r@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/arch/x86/Makefile | 3 ---
>  tools/perf/check-headers.sh  | 3 +++
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
> index 1a38e78117ce..8cc6642fce7a 100644
> --- a/tools/perf/arch/x86/Makefile
> +++ b/tools/perf/arch/x86/Makefile
> @@ -19,9 +19,6 @@ systbl := $(sys)/syscalltbl.sh
>  _dummy := $(shell [ -d '$(out)' ] || mkdir -p '$(out)')
>  
>  $(header): $(sys)/syscall_64.tbl $(systbl)
> -	@(test -d ../../kernel -a -d ../../tools -a -d ../perf && ( \
> -        (diff -B arch/x86/entry/syscalls/syscall_64.tbl ../../arch/x86/entry/syscalls/syscall_64.tbl >/dev/null) \
> -        || echo "Warning: Kernel ABI header at 'tools/perf/arch/x86/entry/syscalls/syscall_64.tbl' differs from latest version at 'arch/x86/entry/syscalls/syscall_64.tbl'" >&2 )) || true
>  	$(Q)$(SHELL) '$(systbl)' $(sys)/syscall_64.tbl 'x86_64' > $@
>  
>  clean::
> diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh
> index 73e723675c5f..2c10e46d98e9 100755
> --- a/tools/perf/check-headers.sh
> +++ b/tools/perf/check-headers.sh
> @@ -93,3 +93,6 @@ check arch/x86/lib/memcpy_64.S        '-I "^EXPORT_SYMBOL" -I "^#include <asm/ex
>  check arch/x86/lib/memset_64.S        '-I "^EXPORT_SYMBOL" -I "^#include <asm/export.h>"'
>  check include/uapi/asm-generic/mman.h '-I "^#include <\(uapi/\)*asm-generic/mman-common.h>"'
>  check include/uapi/linux/mman.h       '-I "^#include <\(uapi/\)*asm/mman.h>"'
> +
> +# diff non-symmetric files
> +check_2 arch/x86/entry/syscalls/syscall_64.tbl ../../arch/x86/entry/syscalls/syscall_64.tbl
> -- 
> 2.17.1

Re: [PATCH 4/4] perf tools: Move syscall_64.tbl check into check-headers.sh

[Jiri Olsa]

On Fri, Jul 20, 2018 at 12:03:54PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jul 20, 2018 at 01:00:36PM +0200, Jiri Olsa escreveu:
> > Probably leftover from the time we introducd the check-headers.sh script.
> 
> So, with your patch:
> 
> [acme@jouet perf]$ git diff
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index f0b1709a5ffb..cfc1b0949bb1 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -343,6 +343,7 @@
>  332    common  statx                   __x64_sys_statx
>  333    common  io_pgetevents           __x64_sys_io_pgetevents
>  334    common  rseq                    __x64_sys_rseq
> +335    common  krava                   __x64_sys_krava
>  
>  #
>  # x32-specific system call numbers start at 512 to avoid cache impact
> [acme@jouet perf]$
> 
> 
> It doesn't warn about that new cool syscall, without your patch:
> 
> Warning: Kernel ABI header at 'tools/perf/arch/x86/entry/syscalls/syscall_64.tbl' differs from latest version at 'arch/x86/entry/syscalls/syscall_64.tbl'
> 
> Can you check this?

hum, did you have the previous patch applied?
without it it displays misleading output 

will check

jirka

[tip:perf/core] perf tools: Use perf_evsel__match instead of open coded equivalent

[tip-bot for Jiri Olsa]

Commit-ID:  2d6cae13f10d5d8b370038dbfdf60317c22b9f52
Gitweb:     https://git.kernel.org/tip/2d6cae13f10d5d8b370038dbfdf60317c22b9f52
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Fri, 20 Jul 2018 13:00:33 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 24 Jul 2018 14:54:13 -0300

perf tools: Use perf_evsel__match instead of open coded equivalent

Use perf_evsel__match() helper in perf_evsel__is_bpf_output().

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180720110036.32251-1-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index d277930b19a1..890babf9ce86 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -402,10 +402,7 @@ bool perf_evsel__is_function_event(struct perf_evsel *evsel);
 
 static inline bool perf_evsel__is_bpf_output(struct perf_evsel *evsel)
 {
-	struct perf_event_attr *attr = &evsel->attr;
-
-	return (attr->config == PERF_COUNT_SW_BPF_OUTPUT) &&
-		(attr->type == PERF_TYPE_SOFTWARE);
+	return perf_evsel__match(evsel, SOFTWARE, SW_BPF_OUTPUT);
 }
 
 struct perf_attr_details {