Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1651359 > unrolled thread

[PATCH 1/2] perf tests: Fix switch tracking test for P4

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] perf tests: Fix switch tracking test for P4 Jiri Olsa <jolsa@kernel.org> - 2017-05-26 14:40 +0200
    Re: [PATCH 1/2] perf tests: Fix switch tracking test for P4 Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-06-01 15:20 +0200
      Re: [PATCH 1/2] perf tests: Fix switch tracking test for P4 Adrian Hunter <adrian.hunter@intel.com> - 2017-06-02 13:50 +0200
        Re: [PATCH 1/2] perf tests: Fix switch tracking test for P4 Jiri Olsa <jolsa@redhat.com> - 2017-06-04 21:20 +0200

#1651359 — [PATCH 1/2] perf tests: Fix switch tracking test for P4

The switch tracking test keeps failing on P4 cpu,
when NMI watchdog is enabled.

The reason is that P4 pmu uses substitute event for cycles
when it's already taken (in our case by NMI watchdog), but
this event does not give even results like cycles, and we
could end up with no samples at all for our short
measuring period.

Fixing this by using "instructions:u" event instead,
which seems to be stable enough.

Cc: Michael Petlan <mpetlan@redhat.com>
Link: http://lkml.kernel.org/n/tip-4s8vo7skneszacdckv7uiog3@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/switch-tracking.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index 65474fd80da7..e519819ea2e5 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -10,6 +10,7 @@
 #include "thread_map.h"
 #include "cpumap.h"
 #include "tests.h"
+#include "header.h"
 
 static int spin_sleep(void)
 {
@@ -298,6 +299,27 @@ static int process_events(struct perf_evlist *evlist,
 	return ret;
 }
 
+static const char *get_hw_counter(void)
+{
+	const char *counter = "cycles:u";
+	char *cpuid;
+
+	cpuid = get_cpuid_str();
+
+	/*
+	 * P4 pmu uses substitute event for cycles if it's already
+	 * taken, but it does not give even results like cycles,
+	 * and we could end up with no samples at all for our short
+	 * measuring period. Using "instructions:u" event instead,
+	 * which seems to be stable enough.
+	 */
+	if (!strcmp("GenuineIntel-15-4", cpuid))
+		counter = "instructions:u";
+
+	pr_debug("using '%s' HW counter");
+	return counter;
+}
+
 /**
  * test__switch_tracking - test using sched_switch and tracking events.
  *
@@ -308,6 +330,7 @@ static int process_events(struct perf_evlist *evlist,
  */
 int test__switch_tracking(int subtest __maybe_unused)
 {
+	const char *hw_counter = get_hw_counter();
 	const char *sched_switch = "sched:sched_switch";
 	struct switch_tracking switch_tracking = { .tids = NULL, };
 	struct record_opts opts = {
@@ -357,9 +380,9 @@ int test__switch_tracking(int subtest __maybe_unused)
 	cpu_clocks_evsel = perf_evlist__last(evlist);
 
 	/* Second event */
-	err = parse_events(evlist, "cycles:u", NULL);
+	err = parse_events(evlist, hw_counter, NULL);
 	if (err) {
-		pr_debug("Failed to parse event cycles:u\n");
+		pr_debug("Failed to parse event %s\n", hw_counter);
 		goto out_err;
 	}
 
-- 
2.9.4

[toc] | [next] | [standalone]

#1655127

Em Fri, May 26, 2017 at 02:31:40PM +0200, Jiri Olsa escreveu:
> The switch tracking test keeps failing on P4 cpu,
> when NMI watchdog is enabled.
> 
> The reason is that P4 pmu uses substitute event for cycles
> when it's already taken (in our case by NMI watchdog), but
> this event does not give even results like cycles, and we
> could end up with no samples at all for our short
> measuring period.
> 
> Fixing this by using "instructions:u" event instead,
> which seems to be stable enough.

The original author of this test entry is Adrian, so would be nice for
him to take a look and give his Ack, Adrian?

- Arnaldo
 
> Cc: Michael Petlan <mpetlan@redhat.com>
> Link: http://lkml.kernel.org/n/tip-4s8vo7skneszacdckv7uiog3@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/tests/switch-tracking.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
> index 65474fd80da7..e519819ea2e5 100644
> --- a/tools/perf/tests/switch-tracking.c
> +++ b/tools/perf/tests/switch-tracking.c
> @@ -10,6 +10,7 @@
>  #include "thread_map.h"
>  #include "cpumap.h"
>  #include "tests.h"
> +#include "header.h"
>  
>  static int spin_sleep(void)
>  {
> @@ -298,6 +299,27 @@ static int process_events(struct perf_evlist *evlist,
>  	return ret;
>  }
>  
> +static const char *get_hw_counter(void)
> +{
> +	const char *counter = "cycles:u";
> +	char *cpuid;
> +
> +	cpuid = get_cpuid_str();
> +
> +	/*
> +	 * P4 pmu uses substitute event for cycles if it's already
> +	 * taken, but it does not give even results like cycles,
> +	 * and we could end up with no samples at all for our short
> +	 * measuring period. Using "instructions:u" event instead,
> +	 * which seems to be stable enough.
> +	 */
> +	if (!strcmp("GenuineIntel-15-4", cpuid))
> +		counter = "instructions:u";
> +
> +	pr_debug("using '%s' HW counter");
> +	return counter;
> +}
> +
>  /**
>   * test__switch_tracking - test using sched_switch and tracking events.
>   *
> @@ -308,6 +330,7 @@ static int process_events(struct perf_evlist *evlist,
>   */
>  int test__switch_tracking(int subtest __maybe_unused)
>  {
> +	const char *hw_counter = get_hw_counter();
>  	const char *sched_switch = "sched:sched_switch";
>  	struct switch_tracking switch_tracking = { .tids = NULL, };
>  	struct record_opts opts = {
> @@ -357,9 +380,9 @@ int test__switch_tracking(int subtest __maybe_unused)
>  	cpu_clocks_evsel = perf_evlist__last(evlist);
>  
>  	/* Second event */
> -	err = parse_events(evlist, "cycles:u", NULL);
> +	err = parse_events(evlist, hw_counter, NULL);
>  	if (err) {
> -		pr_debug("Failed to parse event cycles:u\n");
> +		pr_debug("Failed to parse event %s\n", hw_counter);
>  		goto out_err;
>  	}
>  
> -- 
> 2.9.4

[toc] | [prev] | [next] | [standalone]

#1656155

On 01/06/17 16:11, Arnaldo Carvalho de Melo wrote:
> Em Fri, May 26, 2017 at 02:31:40PM +0200, Jiri Olsa escreveu:
>> The switch tracking test keeps failing on P4 cpu,
>> when NMI watchdog is enabled.
>>
>> The reason is that P4 pmu uses substitute event for cycles
>> when it's already taken (in our case by NMI watchdog), but
>> this event does not give even results like cycles, and we
>> could end up with no samples at all for our short
>> measuring period.

Did you consider increasing the measuring period?

>>
>> Fixing this by using "instructions:u" event instead,
>> which seems to be stable enough.
> 
> The original author of this test entry is Adrian, so would be nice for
> him to take a look and give his Ack, Adrian?
> 
> - Arnaldo
>  
>> Cc: Michael Petlan <mpetlan@redhat.com>
>> Link: http://lkml.kernel.org/n/tip-4s8vo7skneszacdckv7uiog3@git.kernel.org
>> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>> ---
>>  tools/perf/tests/switch-tracking.c | 27 +++++++++++++++++++++++++--
>>  1 file changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
>> index 65474fd80da7..e519819ea2e5 100644
>> --- a/tools/perf/tests/switch-tracking.c
>> +++ b/tools/perf/tests/switch-tracking.c
>> @@ -10,6 +10,7 @@
>>  #include "thread_map.h"
>>  #include "cpumap.h"
>>  #include "tests.h"
>> +#include "header.h"
>>  
>>  static int spin_sleep(void)
>>  {
>> @@ -298,6 +299,27 @@ static int process_events(struct perf_evlist *evlist,
>>  	return ret;
>>  }
>>  
>> +static const char *get_hw_counter(void)
>> +{
>> +	const char *counter = "cycles:u";
>> +	char *cpuid;
>> +
>> +	cpuid = get_cpuid_str();
>> +
>> +	/*
>> +	 * P4 pmu uses substitute event for cycles if it's already
>> +	 * taken, but it does not give even results like cycles,
>> +	 * and we could end up with no samples at all for our short
>> +	 * measuring period. Using "instructions:u" event instead,
>> +	 * which seems to be stable enough.
>> +	 */
>> +	if (!strcmp("GenuineIntel-15-4", cpuid))

Why just model 4?  Isn't all family 15 P4?

>> +		counter = "instructions:u";
>> +
>> +	pr_debug("using '%s' HW counter");

tests/switch-tracking.c: In function ‘get_hw_counter’:
tests/switch-tracking.c:319:2: error: format ‘%s’ expects a matching ‘char
*’ argument [-Werror=format=]


>> +	return counter;
>> +}
>> +
>>  /**
>>   * test__switch_tracking - test using sched_switch and tracking events.
>>   *
>> @@ -308,6 +330,7 @@ static int process_events(struct perf_evlist *evlist,
>>   */
>>  int test__switch_tracking(int subtest __maybe_unused)
>>  {
>> +	const char *hw_counter = get_hw_counter();
>>  	const char *sched_switch = "sched:sched_switch";
>>  	struct switch_tracking switch_tracking = { .tids = NULL, };
>>  	struct record_opts opts = {
>> @@ -357,9 +380,9 @@ int test__switch_tracking(int subtest __maybe_unused)
>>  	cpu_clocks_evsel = perf_evlist__last(evlist);
>>  
>>  	/* Second event */
>> -	err = parse_events(evlist, "cycles:u", NULL);
>> +	err = parse_events(evlist, hw_counter, NULL);
>>  	if (err) {
>> -		pr_debug("Failed to parse event cycles:u\n");
>> +		pr_debug("Failed to parse event %s\n", hw_counter);
>>  		goto out_err;
>>  	}
>>  
>> -- 
>> 2.9.4
> 

[toc] | [prev] | [next] | [standalone]

#1657140

On Fri, Jun 02, 2017 at 02:35:59PM +0300, Adrian Hunter wrote:
> On 01/06/17 16:11, Arnaldo Carvalho de Melo wrote:
> > Em Fri, May 26, 2017 at 02:31:40PM +0200, Jiri Olsa escreveu:
> >> The switch tracking test keeps failing on P4 cpu,
> >> when NMI watchdog is enabled.
> >>
> >> The reason is that P4 pmu uses substitute event for cycles
> >> when it's already taken (in our case by NMI watchdog), but
> >> this event does not give even results like cycles, and we
> >> could end up with no samples at all for our short
> >> measuring period.
> 
> Did you consider increasing the measuring period?

not really, in some cases I saw no samples generated for bigger
periods for another workloads, so I did not think of that in here,
but I'll check

SNIP

> >>  
> >>  static int spin_sleep(void)
> >>  {
> >> @@ -298,6 +299,27 @@ static int process_events(struct perf_evlist *evlist,
> >>  	return ret;
> >>  }
> >>  
> >> +static const char *get_hw_counter(void)
> >> +{
> >> +	const char *counter = "cycles:u";
> >> +	char *cpuid;
> >> +
> >> +	cpuid = get_cpuid_str();
> >> +
> >> +	/*
> >> +	 * P4 pmu uses substitute event for cycles if it's already
> >> +	 * taken, but it does not give even results like cycles,
> >> +	 * and we could end up with no samples at all for our short
> >> +	 * measuring period. Using "instructions:u" event instead,
> >> +	 * which seems to be stable enough.
> >> +	 */
> >> +	if (!strcmp("GenuineIntel-15-4", cpuid))
> 
> Why just model 4?  Isn't all family 15 P4?

I thought there's just one model.. but just based on the kernel code

> 
> >> +		counter = "instructions:u";
> >> +
> >> +	pr_debug("using '%s' HW counter");
> 
> tests/switch-tracking.c: In function ‘get_hw_counter’:
> tests/switch-tracking.c:319:2: error: format ‘%s’ expects a matching ‘char
> *’ argument [-Werror=format=]

omg.. sure ;-)

thanks,
jirka

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web