2017-09-21 19:45:16

by Shuah Khan

[permalink] [raw]
Subject: [PATCH] selftests: timers: set-timer-lat: Fix hang when testing unsupported alarms

When timer_create() fails on a bootime or realtime clock, setup_timer()
returns 0 as if timer has been set. Callers wait forever for the timer
to expire.

This hang is seen on a system that doesn't have support for:

CLOCK_REALTIME_ALARM ABSTIME missing CAP_WAKE_ALARM? : [UNSUPPORTED]

Test hangs waiting for a timer that hasn't been set to expire.

Signed-off-by: Shuah Khan <[email protected]>
---
tools/testing/selftests/timers/set-timer-lat.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/timers/set-timer-lat.c b/tools/testing/selftests/timers/set-timer-lat.c
index 9c92b7bd5641..ab2fe225e051 100644
--- a/tools/testing/selftests/timers/set-timer-lat.c
+++ b/tools/testing/selftests/timers/set-timer-lat.c
@@ -143,7 +143,8 @@ int setup_timer(int clock_id, int flags, int interval, timer_t *tm1)
printf("%-22s %s missing CAP_WAKE_ALARM? : [UNSUPPORTED]\n",
clockstring(clock_id),
flags ? "ABSTIME":"RELTIME");
- return 0;
+ /* Indicate timer isn't set, so caller doesn't wait */
+ return 1;
}
printf("%s - timer_create() failed\n", clockstring(clock_id));
return -1;
--
2.11.0


2017-09-22 14:41:40

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH] selftests: timers: set-timer-lat: Fix hang when testing unsupported alarms

Hi John/Greg,

On 09/21/2017 01:45 PM, Shuah Khan wrote:
> When timer_create() fails on a bootime or realtime clock, setup_timer()
> returns 0 as if timer has been set. Callers wait forever for the timer
> to expire.
>
> This hang is seen on a system that doesn't have support for:
>
> CLOCK_REALTIME_ALARM ABSTIME missing CAP_WAKE_ALARM? : [UNSUPPORTED]
>
> Test hangs waiting for a timer that hasn't been set to expire.
>
> Signed-off-by: Shuah Khan <[email protected]>
> ---
> tools/testing/selftests/timers/set-timer-lat.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/timers/set-timer-lat.c b/tools/testing/selftests/timers/set-timer-lat.c
> index 9c92b7bd5641..ab2fe225e051 100644
> --- a/tools/testing/selftests/timers/set-timer-lat.c
> +++ b/tools/testing/selftests/timers/set-timer-lat.c
> @@ -143,7 +143,8 @@ int setup_timer(int clock_id, int flags, int interval, timer_t *tm1)
> printf("%-22s %s missing CAP_WAKE_ALARM? : [UNSUPPORTED]\n",
> clockstring(clock_id),
> flags ? "ABSTIME":"RELTIME");
> - return 0;
> + /* Indicate timer isn't set, so caller doesn't wait */
> + return 1;
> }
> printf("%s - timer_create() failed\n", clockstring(clock_id));
> return -1;
>

I assume you are good with this fix. I plan to apply it to linux-kselftest fixes
today. Please let me know if you have any objections.

thanks,
-- Shuah

2017-09-22 15:51:18

by Greg Hackmann

[permalink] [raw]
Subject: Re: [PATCH] selftests: timers: set-timer-lat: Fix hang when testing unsupported alarms

On 09/22/2017 07:41 AM, Shuah Khan wrote:
> I assume you are good with this fix. I plan to apply it to linux-kselftest fixes
> today. Please let me know if you have any objections.
>
> thanks,
> -- Shuah
>

AIUI this check was added so set-timer-lat would treat a missing
CAP_WAKE_ALARM as a skipped test (rather than making the entire test
fail). So setup_timer()'s callers would need to treat this as a special
case, by returning immediately but with a return value of 0.

2017-09-22 16:20:13

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH] selftests: timers: set-timer-lat: Fix hang when testing unsupported alarms

On 09/22/2017 09:51 AM, Greg Hackmann wrote:
> On 09/22/2017 07:41 AM, Shuah Khan wrote:
>> I assume you are good with this fix. I plan to apply it to linux-kselftest fixes
>> today. Please let me know if you have any objections.
>>
>> thanks,
>> -- Shuah
>>
>
> AIUI this check was added so set-timer-lat would treat a missing CAP_WAKE_ALARM as a skipped test (rather than making the entire test fail). So setup_timer()'s callers would need to treat this as a special case, by returning immediately but with a return value of 0.

I see. Instead of marking the UNSUPPORTED case as fail, you would like
to see it as skip.

The following is the output with this patch. So I think you don't want to
see the "Bail out!" at the end.

I will make the change and send v2.

./set-timer-lat
Setting timers for every 1 seconds
CLOCK_REALTIME ABSTIME PERIODIC timer fired early: 0 : [OK]
CLOCK_REALTIME ABSTIME PERIODIC max latency: 479361 ns : [OK]
CLOCK_REALTIME RELTIME PERIODIC timer fired early: 0 : [OK]
CLOCK_REALTIME RELTIME PERIODIC max latency: 210625 ns : [OK]
CLOCK_REALTIME ABSTIME ONE-SHOT timer fired early: 0 : [OK]
CLOCK_REALTIME ABSTIME ONE-SHOT max latency: 152892 ns : [OK]
CLOCK_REALTIME ABSTIME ONE-SHOT count: 1 : [OK]
CLOCK_REALTIME RELTIME ONE-SHOT timer fired early: 0 : [OK]
CLOCK_REALTIME RELTIME ONE-SHOT max latency: 167839 ns : [OK]
CLOCK_REALTIME RELTIME ONE-SHOT count: 1 : [OK]
CLOCK_MONOTONIC ABSTIME PERIODIC timer fired early: 0 : [OK]
CLOCK_MONOTONIC ABSTIME PERIODIC max latency: 190204 ns : [OK]
CLOCK_MONOTONIC RELTIME PERIODIC timer fired early: 0 : [OK]
CLOCK_MONOTONIC RELTIME PERIODIC max latency: 212418 ns : [OK]
CLOCK_MONOTONIC ABSTIME ONE-SHOT timer fired early: 0 : [OK]
CLOCK_MONOTONIC ABSTIME ONE-SHOT max latency: 158816 ns : [OK]
CLOCK_MONOTONIC ABSTIME ONE-SHOT count: 1 : [OK]
CLOCK_MONOTONIC RELTIME ONE-SHOT timer fired early: 0 : [OK]
CLOCK_MONOTONIC RELTIME ONE-SHOT max latency: 172362 ns : [OK]
CLOCK_MONOTONIC RELTIME ONE-SHOT count: 1 : [OK]
CLOCK_BOOTTIME ABSTIME PERIODIC timer fired early: 0 : [OK]
CLOCK_BOOTTIME ABSTIME PERIODIC max latency: 180767 ns : [OK]
CLOCK_BOOTTIME RELTIME PERIODIC timer fired early: 0 : [OK]
CLOCK_BOOTTIME RELTIME PERIODIC max latency: 202772 ns : [OK]
CLOCK_BOOTTIME ABSTIME ONE-SHOT timer fired early: 0 : [OK]
CLOCK_BOOTTIME ABSTIME ONE-SHOT max latency: 117733 ns : [OK]
CLOCK_BOOTTIME ABSTIME ONE-SHOT count: 1 : [OK]
CLOCK_BOOTTIME RELTIME ONE-SHOT timer fired early: 0 : [OK]
CLOCK_BOOTTIME RELTIME ONE-SHOT max latency: 171882 ns : [OK]
CLOCK_BOOTTIME RELTIME ONE-SHOT count: 1 : [OK]
CLOCK_REALTIME_ALARM ABSTIME missing CAP_WAKE_ALARM? : [UNSUPPORTED]
CLOCK_REALTIME_ALARM RELTIME missing CAP_WAKE_ALARM? : [UNSUPPORTED]
CLOCK_REALTIME_ALARM ABSTIME missing CAP_WAKE_ALARM? : [UNSUPPORTED]
CLOCK_REALTIME_ALARM RELTIME missing CAP_WAKE_ALARM? : [UNSUPPORTED]
CLOCK_BOOTTIME_ALARM ABSTIME missing CAP_WAKE_ALARM? : [UNSUPPORTED]
CLOCK_BOOTTIME_ALARM RELTIME missing CAP_WAKE_ALARM? : [UNSUPPORTED]
CLOCK_BOOTTIME_ALARM ABSTIME missing CAP_WAKE_ALARM? : [UNSUPPORTED]
CLOCK_BOOTTIME_ALARM RELTIME missing CAP_WAKE_ALARM? : [UNSUPPORTED]
CLOCK_TAI ABSTIME PERIODIC timer fired early: 0 : [OK]
CLOCK_TAI ABSTIME PERIODIC max latency: 183220 ns : [OK]
CLOCK_TAI RELTIME PERIODIC timer fired early: 0 : [OK]
CLOCK_TAI RELTIME PERIODIC max latency: 201380 ns : [OK]
CLOCK_TAI ABSTIME ONE-SHOT timer fired early: 0 : [OK]
CLOCK_TAI ABSTIME ONE-SHOT max latency: 117230 ns : [OK]
CLOCK_TAI ABSTIME ONE-SHOT count: 1 : [OK]
CLOCK_TAI RELTIME ONE-SHOT timer fired early: 0 : [OK]
CLOCK_TAI RELTIME ONE-SHOT max latency: 154236 ns : [OK]
CLOCK_TAI RELTIME ONE-SHOT count: 1 : [OK]
Bail out!
Pass 0 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 0
1..0


thanks,
-- Shuah