Re: [PATCH RESEND] selftests: rtc: fix flaky date_read_loop test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] selftests: rtc: preemption race between ioctl and clock_gettime
- [Medium] selftests: rtc: CLOCK_MONOTONIC does not account for system suspend
--

commit 470d20b138e75cc7483b890fe5b4c4fbcf96a62c
Author: Wake Liu <wakel@google.com>

selftests: rtc: fix flaky date_read_loop test

This commit modifies the rtc.date_read_loop test to measure real elapsed time
between iterations using CLOCK_MONOTONIC. It then dynamically adjusts the
test assertion to make the test more resilient against OS scheduling delays.

> diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c
> index 8047d9879039c..54eb5c255a452 100644
> --- a/tools/testing/selftests/rtc/rtctest.c
> +++ b/tools/testing/selftests/rtc/rtctest.c
[ ... ]
> @@ -126,25 +127,31 @@ TEST_F_TIMEOUT(rtc, date_read_loop, READ_LOOP_DURATION_SEC + 2) {
>  
>  	rc = ioctl(self->fd, RTC_RD_TIME, &rtc_tm);
>  	ASSERT_NE(-1, rc);
> +	clock_gettime(CLOCK_MONOTONIC, &prev_mono);

[Severity: Medium]
Does CLOCK_MONOTONIC adequately account for system suspend in this test?

Since CLOCK_MONOTONIC stops incrementing during suspend while the hardware
RTC continues ticking, if the system suspends during the sleep loop, could
the RTC advance significantly without delta_s reflecting it?

Would using CLOCK_BOOTTIME avoid this regression and correctly track elapsed
time across suspends?

>  	start_rtc_read = rtc_time_to_timestamp(&rtc_tm);
>  	prev_rtc_read = start_rtc_read;
>  
>  	do  {
>  		time_t rtc_read;
> +		time_t delta_s = 0;
>  
>  		rc = ioctl(self->fd, RTC_RD_TIME, &rtc_tm);
>  		ASSERT_NE(-1, rc);
> +		clock_gettime(CLOCK_MONOTONIC, &cur_mono);

[Severity: Medium]
Could a preemption between the ioctl() and clock_gettime() calls cause
spurious assertion failures?

If the thread is preempted immediately after ioctl(RTC_RD_TIME) in iteration
N, cur_mono will be delayed. When this delayed cur_mono is assigned to
prev_mono for iteration N+1, it artificially shrinks the measured delta_s for
the next iteration.

Could this make delta_s smaller than the actual elapsed RTC time, causing
the assertion below to fail?

>  
>  		rtc_read = rtc_time_to_timestamp(&rtc_tm);
> +		delta_s = cur_mono.tv_sec - prev_mono.tv_sec;
> +
>  		/* Time should not go backwards */
>  		ASSERT_LE(prev_rtc_read, rtc_read);
> -		/* Time should not increase more then 1s at a time */
> -		ASSERT_GE(prev_rtc_read + 1, rtc_read);
> +		/* Time should not increase more then elapsed time + 1s */
> +		ASSERT_GE(prev_rtc_read + delta_s + 1, rtc_read);
>  
>  		/* Sleep 11ms to avoid killing / overheating the RTC */
>  		nanosleep_with_retries(READ_LOOP_SLEEP_MS * 1000000);
>  
>  		prev_rtc_read = rtc_read;
> +		prev_mono = cur_mono;
>  		iter_count++;
>  	} while (prev_rtc_read <= start_rtc_read + READ_LOOP_DURATION_SEC);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929152212.2092062-1-wakel@google.com?part=1