[Intel-gfx] [PATCH] lib/igt_aux: make igt_wait_for_pm_status() resist the signal helper

[Paulo Zanoni]

2014-10-21 13:18 GMT-02:00 Daniel Vetter <daniel at ffwll.ch>:
> On Tue, Oct 14, 2014 at 04:12:22PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>
>> If the signal helper is active, the usleep() calls return earlier, and
>> we may end up returning false way before the 10s timeout, failing the
>> subtests. This currently happens on the kms_flip RPM interruptible
>> subtests.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=78893
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> ---
>>  lib/igt_aux.c | 17 ++++++++++++-----
>>  1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
>> index b32297e..01654f0 100644
>> --- a/lib/igt_aux.c
>> +++ b/lib/igt_aux.c
>> @@ -42,6 +42,7 @@
>>  #include <stdlib.h>
>>  #include <unistd.h>
>>  #include <sys/wait.h>
>> +#include <sys/time.h>
>>  #include <sys/types.h>
>>  #include <sys/syscall.h>
>>  #include <sys/utsname.h>
>> @@ -502,21 +503,27 @@ enum igt_runtime_pm_status igt_get_runtime_pm_status(void)
>>   * Waits until for the driver to switch to into the desired runtime PM status,
>>   * with a 10 second timeout.
>>   *
>> + * Some subtests call this function while the signal helper is active, so we
>> + * can't assume each usleep() call will sleep for 100ms.
>> + *
>>   * Returns:
>>   * True if the desired runtime PM status was attained, false if the operation
>>   * timed out.
>>   */
>>  bool igt_wait_for_pm_status(enum igt_runtime_pm_status status)
>>  {
>> -     int i;
>> -     int hundred_ms = 100 * 1000, ten_s = 10 * 1000 * 1000;
>> +     struct timeval start, end, diff;
>>
>> -     for (i = 0; i < ten_s; i += hundred_ms) {
>> +     igt_assert(gettimeofday(&start, NULL) == 0);
>> +     do {
>>               if (igt_get_runtime_pm_status() == status)
>>                       return true;
>>
>> -             usleep(hundred_ms);
>> -     }
>> +             usleep(100 * 1000);
>> +
>> +             igt_assert(gettimeofday(&end, NULL) == 0);
>> +             timersub(&end, &start, &diff);
>
> Should we have a generic igt_usleep helper which is signal-robust? Just a
> quick idea since there's probably more like these.

If we think we could use it in many places, it makes sense. I'm not
aware yet of where we could use it.

> -Daniel
>
>> +     } while (diff.tv_sec < 10);
>>
>>       return false;
>>  }
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch



-- 
Paulo Zanoni