Re: [LTP] [PATCH v2] clock_settime: Detect external clock adjustments via CLOCK_MONOTONIC

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> These tests manipulate CLOCK_REALTIME to verify timer and
> clock_nanosleep behavior, but NTP or VM time sync can also adjust
> CLOCK_REALTIME during the test, causing timers to fire at wrong times
> and producing spurious failures.
> 
> Use CLOCK_MONOTONIC as a sideband check to detect external interference.
> If the monotonic elapsed time is anomalous, report UNTESTED instead of
> a false FAIL. Guard the checks with _POSIX_MONOTONIC_CLOCK since
> CLOCK_MONOTONIC is optional in POSIX.1-2001.

Maybe we can retry a few times before we give up and report untested?

> Co-authored-by: Claude <noreply@anthropic.com>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> I used Claude to generate the right patches to fix sporadic issues in the
> clock_settime testing suite. We tried many attempt for fixing it, but the
> CLOCK_REALTIME usage seems to be the real issue in here. The idea is that
> we use internal Openposix mechanisms to skip tests if CLOCK_MONOTONIC
> usage spots a possible interference from external tools.
> ---
> Changes in v2:
> - print a message if CLOCK_MONOTONIC is not available
> - always verify that CLOCK_MONOTONIC is available
> - Link to v1: https://lore.kernel.org/r/20260331-clock_settime_fix-v1-1-dfae06df2436@suse.com
> ---
>  .../conformance/interfaces/clock_settime/4-1.c     | 36 ++++++++++++++++++
>  .../conformance/interfaces/clock_settime/5-1.c     | 43 +++++++++++++++++++++-
>  .../conformance/interfaces/clock_settime/5-2.c     | 43 +++++++++++++++++++++-
>  .../conformance/interfaces/clock_settime/7-1.c     | 38 +++++++++++++++++++
>  .../conformance/interfaces/clock_settime/7-2.c     | 41 ++++++++++++++++++++-
>  5 files changed, 198 insertions(+), 3 deletions(-)
> 
> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/clock_settime/4-1.c b/testcases/open_posix_testsuite/conformance/interfaces/clock_settime/4-1.c
> index f8a3e9c542ca8c9767cdd0057005e519f58b2b55..5f3f816a5fc71a9e62bb3577e1a18a19f987a802 100644
> --- a/testcases/open_posix_testsuite/conformance/interfaces/clock_settime/4-1.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/clock_settime/4-1.c
> @@ -42,6 +42,9 @@ int main(void)
>  {
>  	struct sigevent ev;
>  	struct timespec tpT0, tpT2, tpreset;
> +#ifdef _POSIX_MONOTONIC_CLOCK
> +	struct timespec mono_before, mono_after;
> +#endif
>  	struct itimerspec its;
>  	timer_t tid;
>  	int delta;
> @@ -55,6 +58,10 @@ int main(void)
>  		return PTS_UNTESTED;
>  	}
>  
> +#ifndef _POSIX_MONOTONIC_CLOCK
> +	printf("CLOCK_MONOTONIC unavailable, test may fail due to external clock adjustments\n");
> +#endif
> +
>  	/*
>  	 * set up sigevent for timer
>  	 * set up signal set for sigwait
> @@ -82,6 +89,13 @@ int main(void)
>  		return PTS_UNRESOLVED;
>  	}
>  
> +#ifdef _POSIX_MONOTONIC_CLOCK
> +	if (clock_gettime(CLOCK_MONOTONIC, &mono_before) != 0) {
> +		perror("clock_gettime() was not successful\n");
> +		return PTS_UNRESOLVED;
> +	}
> +#endif
> +
>  	if (timer_create(CLOCK_REALTIME, &ev, &tid) != 0) {
>  		perror("timer_create() did not return success\n");
>  		return PTS_UNRESOLVED;
> @@ -120,6 +134,28 @@ int main(void)
>  	tpreset.tv_sec += tpT2.tv_sec - tpT0.tv_sec;
>  	setBackTime(tpreset);
>  
> +#ifdef _POSIX_MONOTONIC_CLOCK
> +	if (clock_gettime(CLOCK_MONOTONIC, &mono_after) != 0) {
> +		perror("clock_gettime() was not successful\n");
> +		return PTS_UNRESOLVED;
> +	}
> +
> +	/*
> +	 * The expected monotonic elapsed time is SLEEPTIME + TIMEROFFSET
> +	 * since after resetting the clock, the timer must wait another
> +	 * full TIMEROFFSET. If monotonic time is much shorter, an external
> +	 * clock adjustment (NTP, VM sync) interfered with the test.
> +	 */
> +	if (mono_after.tv_sec - mono_before.tv_sec <
> +	    SLEEPTIME + TIMEROFFSET - ACCEPTABLEDELTA) {
> +		printf("UNTESTED: external clock adjustment detected "
> +		       "(monotonic elapsed %ds, expected ~%ds)\n",
> +		       (int)(mono_after.tv_sec - mono_before.tv_sec),
> +		       SLEEPTIME + TIMEROFFSET);
> +		return PTS_UNTESTED;
> +	}
> +#endif

This is a bit more than a line or two, maybe we can put this into a
common header as a static inline functions instead?

Something as having a function to start measuring the time and to return
true of false based on if the measured interval lenght is as expected?

e.g.:

static inline void pts_mono_time_start(void);

static inline bool pts_mono_time_end(unsigned int expected_delta);

Then we could loop the actual test a few times while pts_mon_time_end()
returns false.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp