diff mbox

[i-g-t] tests/perf_pmu: Avoid off-lining all CPUs

Message ID 20180219134037.28708-1-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tvrtko Ursulin Feb. 19, 2018, 1:40 p.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Bail out from the cpu-hotplug test if we failed to bring a CPU back
online.

This still leaves the machine in a quite bad state, but at least it
avoids hard hanging it.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/perf_pmu.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Chris Wilson Feb. 19, 2018, 1:54 p.m. UTC | #1
Quoting Tvrtko Ursulin (2018-02-19 13:40:37)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Bail out from the cpu-hotplug test if we failed to bring a CPU back
> online.
> 
> This still leaves the machine in a quite bad state, but at least it
> avoids hard hanging it.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  tests/perf_pmu.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> index 7fab73e22c2d..a334d3b5770e 100644
> --- a/tests/perf_pmu.c
> +++ b/tests/perf_pmu.c
> @@ -965,6 +965,7 @@ static void cpu_hotplug(int gem_fd)
>         int link[2];
>         int fd, ret;
>         int cur = 0;
> +       char buf;
>  
>         igt_require(cpu0_hotplug_support());
>  
> @@ -1012,7 +1013,15 @@ static void cpu_hotplug(int gem_fd)
>                         /* Offline followed by online a CPU. */
>                         igt_assert_eq(write(cpufd, "0", 2), 2);
>                         usleep(1e6);
> -                       igt_assert_eq(write(cpufd, "1", 2), 2);
> +                       ret = write(cpufd, "1", 2);
> +                       if (ret < 0) {
> +                               /*
> +                                * Abort the test if we failed to bring a CPU
> +                                * back online.
> +                                */
> +                               igt_assert_eq(write(link[1], "s", 1), 1);
> +                               break;
> +                       }
>  
>                         close(cpufd);
>                         cpu++;
> @@ -1026,7 +1035,6 @@ static void cpu_hotplug(int gem_fd)
>          * until the CPU core shuffler finishes one loop.
>          */
>         for (;;) {
> -               char buf;
>                 int ret2;
>  
>                 usleep(500e3);
> @@ -1053,6 +1061,9 @@ static void cpu_hotplug(int gem_fd)
>         close(fd);
>         close(link[0]);
>  
> +       /* Skip if child signals a problem with bringing a CPU back online. */
> +       igt_skip_on(buf == 's');

The logic makes sense.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

What happens if we try to online an already on cpu? Will that report the
failure or just tell us to stop wasting its time?
-Chris
Tvrtko Ursulin Feb. 19, 2018, 2:08 p.m. UTC | #2
On 19/02/2018 13:54, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-02-19 13:40:37)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Bail out from the cpu-hotplug test if we failed to bring a CPU back
>> online.
>>
>> This still leaves the machine in a quite bad state, but at least it
>> avoids hard hanging it.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   tests/perf_pmu.c | 15 +++++++++++++--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
>> index 7fab73e22c2d..a334d3b5770e 100644
>> --- a/tests/perf_pmu.c
>> +++ b/tests/perf_pmu.c
>> @@ -965,6 +965,7 @@ static void cpu_hotplug(int gem_fd)
>>          int link[2];
>>          int fd, ret;
>>          int cur = 0;
>> +       char buf;
>>   
>>          igt_require(cpu0_hotplug_support());
>>   
>> @@ -1012,7 +1013,15 @@ static void cpu_hotplug(int gem_fd)
>>                          /* Offline followed by online a CPU. */
>>                          igt_assert_eq(write(cpufd, "0", 2), 2);
>>                          usleep(1e6);
>> -                       igt_assert_eq(write(cpufd, "1", 2), 2);
>> +                       ret = write(cpufd, "1", 2);
>> +                       if (ret < 0) {
>> +                               /*
>> +                                * Abort the test if we failed to bring a CPU
>> +                                * back online.
>> +                                */
>> +                               igt_assert_eq(write(link[1], "s", 1), 1);
>> +                               break;
>> +                       }
>>   
>>                          close(cpufd);
>>                          cpu++;
>> @@ -1026,7 +1035,6 @@ static void cpu_hotplug(int gem_fd)
>>           * until the CPU core shuffler finishes one loop.
>>           */
>>          for (;;) {
>> -               char buf;
>>                  int ret2;
>>   
>>                  usleep(500e3);
>> @@ -1053,6 +1061,9 @@ static void cpu_hotplug(int gem_fd)
>>          close(fd);
>>          close(link[0]);
>>   
>> +       /* Skip if child signals a problem with bringing a CPU back online. */
>> +       igt_skip_on(buf == 's');
> 
> The logic makes sense.
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

I wasn't so sure after I've sent it. There was an assert on write error 
already so that would have aborted the child. Will see what shards will 
say..

> What happens if we try to online an already on cpu? Will that report the
> failure or just tell us to stop wasting its time?

Seems the attempt is simply ignored.

Regards,

Tvrtko
diff mbox

Patch

diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
index 7fab73e22c2d..a334d3b5770e 100644
--- a/tests/perf_pmu.c
+++ b/tests/perf_pmu.c
@@ -965,6 +965,7 @@  static void cpu_hotplug(int gem_fd)
 	int link[2];
 	int fd, ret;
 	int cur = 0;
+	char buf;
 
 	igt_require(cpu0_hotplug_support());
 
@@ -1012,7 +1013,15 @@  static void cpu_hotplug(int gem_fd)
 			/* Offline followed by online a CPU. */
 			igt_assert_eq(write(cpufd, "0", 2), 2);
 			usleep(1e6);
-			igt_assert_eq(write(cpufd, "1", 2), 2);
+			ret = write(cpufd, "1", 2);
+			if (ret < 0) {
+				/*
+				 * Abort the test if we failed to bring a CPU
+				 * back online.
+				 */
+				igt_assert_eq(write(link[1], "s", 1), 1);
+				break;
+			}
 
 			close(cpufd);
 			cpu++;
@@ -1026,7 +1035,6 @@  static void cpu_hotplug(int gem_fd)
 	 * until the CPU core shuffler finishes one loop.
 	 */
 	for (;;) {
-		char buf;
 		int ret2;
 
 		usleep(500e3);
@@ -1053,6 +1061,9 @@  static void cpu_hotplug(int gem_fd)
 	close(fd);
 	close(link[0]);
 
+	/* Skip if child signals a problem with bringing a CPU back online. */
+	igt_skip_on(buf == 's');
+
 	assert_within_epsilon(val, ts[1] - ts[0], tolerance);
 }