[Devel] [PATCH vz10 1/3] ms/selftests/posix_timers: Use CLOCK_THREAD_CPUTIME_ID for ITIMER_PROF measurements

[Devel] [PATCH vz10 1/3] ms/selftests/posix_timers: Use CLOCK_THREAD_CPUTIME_ID for ITIMER_PROF measurements

[Eva Kurchatova]

From: John Stultz <jstultz@google.com>

It was reported that the posix_timers test was at times seeing failures
with ITIMER_PROF timers, specifically in cases where the RCU_SOFTIRQ was
taking up significant amounts of time.

Analysis showed that as the time in softirq isn't included in the task
stime + utime accounting used to trigger the SIGPROF so delays from softirq
work could cause it to appear that the signal was incorrectly delayed.

Contributing to this is that the test uses gettimeofday() to measure
itimers, which also means any scheduling delay can also cause failures (as
the task may not be running the entire time).

To fix this, convert all the itimer measurements to use clock_gettime(),
tweaking the logic to use nsecs instead of usecs. Then for ITIMER_PROF
timers, utilize the CLOCK_THREAD_CPUTIME_ID clockid so that it is similarly
measuring the time the task was running.

Signed-off-by: John Stultz <jstultz@google.com>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Link: https://patch.msgid.link/20260428173957.1394265-1-jstultz@google.com
The ITIMER_PROF case of it fails on the CI machines, which are two
cpu guests under load:

  # Diff too high: 2561992..not ok 3 ITIMER_PROF

2.56 seconds of wall time for the 2 seconds of cpu time the timer
counts, so the task had about four fifths of a cpu.

(cherry picked from commit b00385b8d081ce74f36ea178e04e1b106505fb36)

https://virtuozzo.atlassian.net/browse/VSTOR-143363
Feature: fix selftests
Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/timers/posix_timers.c | 55 ++++++++++---------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c
index f0eceb0faf34..077a98fb1b1d 100644
--- a/tools/testing/selftests/timers/posix_timers.c
+++ b/tools/testing/selftests/timers/posix_timers.c
@@ -77,19 +77,25 @@ static void sig_handler(int nr)
 	done = 1;
 }
 
+static inline int64_t calcdiff_ns(struct timespec t1, struct timespec t2)
+{
+	int64_t diff;
+
+	diff = NSEC_PER_SEC * (int64_t)((int) t1.tv_sec - (int) t2.tv_sec);
+	diff += ((int) t1.tv_nsec - (int) t2.tv_nsec);
+	return diff;
+}
+
 /*
  * Check the expected timer expiration matches the GTOD elapsed delta since
  * we armed the timer. Keep a 0.5 sec error margin due to various jitter.
  */
-static int check_diff(struct timeval start, struct timeval end)
+static int check_diff(struct timespec start, struct timespec end)
 {
-	long long diff;
-
-	diff = end.tv_usec - start.tv_usec;
-	diff += (end.tv_sec - start.tv_sec) * USEC_PER_SEC;
+	long long diff = calcdiff_ns(end, start);
 
-	if (llabs(diff - DELAY * USEC_PER_SEC) > USEC_PER_SEC / 2) {
-		printf("Diff too high: %lld..", diff);
+	if (llabs(diff - DELAY * NSEC_PER_SEC) > NSEC_PER_SEC / 2) {
+		printf("Diff too high: %lld ns..", diff);
 		return -1;
 	}
 
@@ -98,22 +104,25 @@ static int check_diff(struct timeval start, struct timeval end)
 
 static void check_itimer(int which, const char *name)
 {
-	struct timeval start, end;
+	struct timespec start, end;
 	struct itimerval val = {
 		.it_value.tv_sec = DELAY,
 	};
+	int clock_id = CLOCK_REALTIME;
 
 	done = 0;
 
 	if (which == ITIMER_VIRTUAL)
 		signal(SIGVTALRM, sig_handler);
-	else if (which == ITIMER_PROF)
+	else if (which == ITIMER_PROF) {
+		clock_id = CLOCK_THREAD_CPUTIME_ID;
 		signal(SIGPROF, sig_handler);
+	}
 	else if (which == ITIMER_REAL)
 		signal(SIGALRM, sig_handler);
 
-	if (gettimeofday(&start, NULL) < 0)
-		fatal_error(name, "gettimeofday()");
+	if (clock_gettime(clock_id, &start))
+		fatal_error(name, "clock_gettime()");
 
 	if (setitimer(which, &val, NULL) < 0)
 		fatal_error(name, "setitimer()");
@@ -125,18 +134,19 @@ static void check_itimer(int which, const char *name)
 	else if (which == ITIMER_REAL)
 		idle_loop();
 
-	if (gettimeofday(&end, NULL) < 0)
-		fatal_error(name, "gettimeofday()");
+	if (clock_gettime(clock_id, &end))
+		fatal_error(name, "clock_gettime()");
 
 	ksft_test_result(check_diff(start, end) == 0, "%s\n", name);
 }
 
 static void check_timer_create(int which, const char *name)
 {
-	struct timeval start, end;
+	struct timespec start, end;
 	struct itimerspec val = {
 		.it_value.tv_sec = DELAY,
 	};
+	int clock_id = CLOCK_REALTIME;
 	timer_t id;
 
 	done = 0;
@@ -147,16 +157,16 @@ static void check_timer_create(int which, const char *name)
 	if (signal(SIGALRM, sig_handler) == SIG_ERR)
 		fatal_error(name, "signal()");
 
-	if (gettimeofday(&start, NULL) < 0)
-		fatal_error(name, "gettimeofday()");
+	if (clock_gettime(clock_id, &start))
+		fatal_error(name, "clock_gettime()");
 
 	if (timer_settime(id, 0, &val, NULL) < 0)
 		fatal_error(name, "timer_settime()");
 
 	user_loop();
 
-	if (gettimeofday(&end, NULL) < 0)
-		fatal_error(name, "gettimeofday()");
+	if (clock_gettime(clock_id, &end))
+		fatal_error(name, "clock_gettime()");
 
 	ksft_test_result(check_diff(start, end) == 0,
 			 "timer_create() per %s\n", name);
@@ -444,15 +454,6 @@ static void check_delete(void)
 	ksft_test_result(!tsig.signals, "check_delete\n");
 }
 
-static inline int64_t calcdiff_ns(struct timespec t1, struct timespec t2)
-{
-	int64_t diff;
-
-	diff = NSEC_PER_SEC * (int64_t)((int) t1.tv_sec - (int) t2.tv_sec);
-	diff += ((int) t1.tv_nsec - (int) t2.tv_nsec);
-	return diff;
-}
-
 static void check_sigev_none(int which, const char *name)
 {
 	struct timespec start, now;
-- 
2.55.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH vz10 2/3] selftests: timers: count what tick is worth in the drift estimate

[Eva Kurchatova]

raw_skew measures the drift between CLOCK_MONOTONIC and
CLOCK_MONOTONIC_RAW and compares it against the adjustment adjtimex
reports, but it reads only tx.freq for the latter. The kernel takes
the tick value into the adjustment as well, a microsecond of the tick
length per unit, a hundred ppm each, and a time sync daemon does put
the coarse part of its correction there. Where it does, the two
numbers cannot meet:

  # Estimating clock drift: -51.724(est) 48.277(act)	[FAILED]

On the machine measured, chronyd held freq at 48.278 ppm with tick at
9999, so the
correction really applied is -51.722 ppm, which is what the clocks
show. The skip for an externally adjusted clock does not catch it:
the clock is steady, offset is zero and neither freq nor tick move
during the run.

Count what tick is worth alongside freq, on the same machine:

  # Estimating clock drift: -51.724(est) -51.723(act)	[OK]

https://virtuozzo.atlassian.net/browse/VSTOR-143363
Feature: fix selftests
Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/timers/raw_skew.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/testing/selftests/timers/raw_skew.c b/tools/testing/selftests/timers/raw_skew.c
index 957f7cd29cb1..b798fa1ba37e 100644
--- a/tools/testing/selftests/timers/raw_skew.c
+++ b/tools/testing/selftests/timers/raw_skew.c
@@ -91,6 +91,7 @@ int main(int argc, char **argv)
 {
 	struct timespec mon, raw, start, end;
 	long long delta1, delta2, interval, eppm, ppm;
+	long tick_nom;
 	struct timex tx1, tx2;
 
 	setbuf(stdout, NULL);
@@ -129,6 +130,17 @@ int main(int argc, char **argv)
 	/* Avg the two actual freq samples adjtimex gave us */
 	ppm = (long long)(tx1.freq + tx2.freq) * 1000 / 2;
 	ppm = shift_right(ppm, 16);
+
+	/*
+	 * The tick value holds the coarse part of the adjustment, a
+	 * microsecond of the tick length per unit, and time sync daemons
+	 * do put part of their correction there.  What it is worth has to
+	 * be counted in as well, or a clock disciplined through it looks
+	 * off by a hundred ppm a unit against what the two clocks show.
+	 */
+	tick_nom = USEC_PER_SEC / sysconf(_SC_CLK_TCK);
+	ppm += (long long)(tx1.tick + tx2.tick - 2 * tick_nom) *
+	       (USEC_PER_SEC / tick_nom) * 1000 / 2;
 	printf(" %lld.%i(act)", ppm/1000, abs((int)(ppm%1000)));
 
 	if (llabs(eppm - ppm) > 1000) {
-- 
2.55.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH vz10 3/3] selftests: timers: measure the cpu timers on the clock they count

[Eva Kurchatova]

Commit b00385b8d081 ("selftests/posix_timers: Use CLOCK_THREAD_CPUTIME_ID
for ITIMER_PROF measurements") gave ITIMER_PROF the clock it counts,
as measuring a cpu timer against the wall says nothing on a machine
where the task does not have a cpu to itself. The other cpu timers
are still measured against CLOCK_REALTIME and fail the same way.
With four busy loops on the two cpus of a test machine:

  not ok 2 ITIMER_VIRTUAL
  not ok 5 timer_create() per CLOCK_THREAD_CPUTIME_ID
  not ok 6 timer_create() per CLOCK_PROCESS_CPUTIME_ID

ITIMER_VIRTUAL counts the time the task spends in userspace, so take
CLOCK_THREAD_CPUTIME_ID for it as well; the loop it runs stays in
userspace, so what that clock adds for system time is far inside the
half a second the check allows. check_timer_create() arms its timer on
the clock it is given, so measure on that one.

The test then passes whole on that machine, 19 of 19, busy or idle.

https://virtuozzo.atlassian.net/browse/VSTOR-143363
Feature: fix selftests
Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 tools/testing/selftests/timers/posix_timers.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/timers/posix_timers.c b/tools/testing/selftests/timers/posix_timers.c
index 077a98fb1b1d..66993bd632ec 100644
--- a/tools/testing/selftests/timers/posix_timers.c
+++ b/tools/testing/selftests/timers/posix_timers.c
@@ -112,9 +112,10 @@ static void check_itimer(int which, const char *name)
 
 	done = 0;
 
-	if (which == ITIMER_VIRTUAL)
+	if (which == ITIMER_VIRTUAL) {
+		clock_id = CLOCK_THREAD_CPUTIME_ID;
 		signal(SIGVTALRM, sig_handler);
-	else if (which == ITIMER_PROF) {
+	} else if (which == ITIMER_PROF) {
 		clock_id = CLOCK_THREAD_CPUTIME_ID;
 		signal(SIGPROF, sig_handler);
 	}
@@ -146,7 +147,7 @@ static void check_timer_create(int which, const char *name)
 	struct itimerspec val = {
 		.it_value.tv_sec = DELAY,
 	};
-	int clock_id = CLOCK_REALTIME;
+	int clock_id = which;
 	timer_t id;
 
 	done = 0;
-- 
2.55.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel