[igt-dev] [Intel-gfx] [PATCH i-g-t] tests/perf_pmu: Avoid off-lining all CPUs

[Tvrtko Ursulin]

On 19/02/2018 13:54, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-02-19 13:40:37)
>> From: Tvrtko Ursulin <tvrtko.ursulin at 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 at 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 at 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