lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:	Wed, 29 Jul 2009 14:31:51 +0200
From:	Stanislaw Gruszka <sgruszka@...hat.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	linux-kernel@...r.kernel.org, Oleg Nesterov <oleg@...hat.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Stanislaw Gruszka <sgruszka@...hat.com>
Subject: [RFC PATCH] Fix posix CPUCLOCK_{PROF,VIRT} periodic precision

I found we have similar lack of precision as in itimers with posix cpu
timers when clockid is based on CPUCLOCK_PROF and CPUCLOCK_VIRT. This can
be showed by attached program. It generate below output on my laptop:

Without patch:

CLK_PROF
Period 10000:	counted time 6.46 , real time 7.12 , error -9.2 %
Period 9998:	counted time 7.11 , real time 7.11 , error -0.1 %
Period 1000:	counted time 3.56 , real time 7.12 , error -50.0 %
Period 11111:	counted time 6.58 , real time 7.11 , error -7.5 %
Period 21315:	counted time 6.88 , real time 7.11 , error -3.1 %
Period 1500:	counted time 5.34 , real time 7.12 , error -25.0 %
Period 15000:	counted time 6.66 , real time 7.11 , error -6.3 %
Period 50000:	counted time 6.95 , real time 7.11 , error -2.2 %
Period 500:	counted time 3.57 , real time 7.13 , error -50.0 %
CLK_VIRT
Period 10000:	counted time 6.46 , real time 7.11 , error -9.1 %
Period 9998:	counted time 7.10 , real time 7.11 , error -0.1 %
Period 1000:	counted time 3.55 , real time 7.12 , error -50.1 %
Period 11111:	counted time 6.58 , real time 7.11 , error -7.5 %
Period 21315:	counted time 6.88 , real time 7.11 , error -3.1 %
Period 1500:	counted time 5.33 , real time 7.12 , error -25.0 %
Period 15000:	counted time 6.64 , real time 7.11 , error -6.5 %
Period 50000:	counted time 6.95 , real time 7.11 , error -2.2 %
Period 500:	counted time 3.56 , real time 7.13 , error -50.0 % 

With patch:

CLK_PROF
Period 10000:	counted time 7.10 , real time 7.11 , error -0.2 %
Period 9998:	counted time 7.10 , real time 7.11 , error -0.2 %
Period 1000:	counted time 7.12 , real time 7.13 , error -0.1 %
Period 11111:	counted time 7.10 , real time 7.11 , error -0.1 %
Period 21315:	counted time 7.08 , real time 7.10 , error -0.3 %
Period 1500:	counted time 7.11 , real time 7.12 , error -0.1 %
Period 15000:	counted time 7.08 , real time 7.10 , error -0.3 %
Period 50000:	counted time 7.05 , real time 7.10 , error -0.7 %
Period 500:	counted time 3.56 , real time 7.12 , error -50.0 %
CLK_VIRT
Period 10000:	counted time 7.09 , real time 7.10 , error -0.2 %
Period 9998:	counted time 7.09 , real time 7.10 , error -0.2 %
Period 1000:	counted time 7.11 , real time 7.12 , error -0.2 %
Period 11111:	counted time 7.10 , real time 7.10 , error -0.0 %
Period 21315:	counted time 7.08 , real time 7.11 , error -0.4 %
Period 1500:	counted time 7.11 , real time 7.11 , error -0.1 %
Period 15000:	counted time 7.08 , real time 7.10 , error -0.3 %
Period 50000:	counted time 7.05 , real time 7.10 , error -0.7 %
Period 500:	counted time 3.56 , real time 7.12 , error -50.0 %

I think timers should be fixed (or maybe removed at all, is that
possible ?) This is first version of patch with fix, just for review. 
It's written on top of my itimer patches.

Signed-off-by: Stanislaw Gruszka <sgruszka@...hat.com>

---
 include/linux/posix-timers.h |    1 +
 kernel/posix-cpu-timers.c    |   51 +++++++++++++++++++++++++++++++++++------
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index 4f71bf4..d429c82 100644
--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -13,6 +13,7 @@ union cpu_time_count {
 struct cpu_timer_list {
 	struct list_head entry;
 	union cpu_time_count expires, incr;
+	u32 error, incr_error;
 	struct task_struct *task;
 	int firing;
 };
diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index 18bdde6..2bbe76a 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/posix-cpu-timers.c
@@ -9,6 +9,19 @@
 #include <asm/uaccess.h>
 #include <linux/kernel_stat.h>
 
+static u32 onecputick;
+
+static inline u32 cputime_sub_ns(cputime_t ct, s64 real_ns)
+{
+	struct timespec ts;
+	s64 cpu_ns;
+
+	cputime_to_timespec(ct, &ts);
+	cpu_ns = timespec_to_ns(&ts);
+
+	return (cpu_ns <= real_ns) ? 0 : cpu_ns - real_ns;
+}
+
 /*
  * Called after updating RLIMIT_CPU to set timer expiration if necessary.
  */
@@ -147,6 +160,8 @@ static void bump_cpu_timer(struct k_itimer *timer,
 		}
 	} else {
 		cputime_t delta, incr;
+		u64 error, incr_error, incr_error_val;
+		unsigned long nticks;
 
 		if (cputime_lt(now.cpu, timer->it.cpu.expires.cpu))
 			return;
@@ -154,16 +169,32 @@ static void bump_cpu_timer(struct k_itimer *timer,
 		delta = cputime_sub(cputime_add(now.cpu, incr),
 				    timer->it.cpu.expires.cpu);
 		/* Don't use (incr*2 < delta), incr*2 might overflow. */
-		for (i = 0; cputime_lt(incr, cputime_sub(delta, incr)); i++)
-			     incr = cputime_add(incr, incr);
-		for (; i >= 0; incr = cputime_halve(incr), i--) {
-			if (cputime_lt(delta, incr))
+		incr_error = timer->it.cpu.incr_error;
+		for (i = 0; cputime_lt(incr, cputime_sub(delta, incr)); i++) {
+			incr = cputime_add(incr, incr);
+			incr_error += incr_error;
+		}
+
+		incr_error_val = incr_error;
+		for ( ; i >= 0; incr = cputime_halve(incr), incr_error >>= 1, i--) {
+			if (cputime_lt(delta, incr)) {
+				incr_error_val -= incr_error;
 				continue;
+			}
 			timer->it.cpu.expires.cpu =
 				cputime_add(timer->it.cpu.expires.cpu, incr);
 			timer->it_overrun += 1 << i;
 			delta = cputime_sub(delta, incr);
 		}
+
+		error = timer->it.cpu.error + incr_error_val;
+		timer->it.cpu.error = do_div(error, onecputick);
+		nticks = (unsigned long) error;
+		timer->it_overrun -= nticks;
+		if (timer->it_overrun < 0)
+			timer->it_overrun = 0;
+		timer->it.cpu.expires.cpu = cputime_sub(timer->it.cpu.expires.cpu,
+							jiffies_to_cputime(nticks));
 	}
 }
 
@@ -397,6 +428,8 @@ int posix_cpu_timer_create(struct k_itimer *new_timer)
 	INIT_LIST_HEAD(&new_timer->it.cpu.entry);
 	new_timer->it.cpu.incr.sched = 0;
 	new_timer->it.cpu.expires.sched = 0;
+	new_timer->it.cpu.error = 0;
+	new_timer->it.cpu.incr_error = 0;
 
 	read_lock(&tasklist_lock);
 	if (CPUCLOCK_PERTHREAD(new_timer->it_clock)) {
@@ -840,6 +873,9 @@ int posix_cpu_timer_set(struct k_itimer *timer, int flags,
 	 */
 	timer->it.cpu.incr = timespec_to_sample(timer->it_clock,
 						&new->it_interval);
+	timer->it.cpu.incr_error = cputime_sub_ns(timer->it.cpu.incr.cpu,
+						  timespec_to_ns(&new->it_interval));
+	timer->it.cpu.error = 0;
 
 	/*
 	 * This acts as a modification timestamp for the timer,
@@ -1072,7 +1108,6 @@ static void stop_process_timers(struct task_struct *tsk)
 	spin_unlock_irqrestore(&cputimer->lock, flags);
 }
 
-static u32 onecputick;
 
 static void check_cpu_itimer(struct task_struct *tsk, struct cpu_itimer *it,
 			     cputime_t *expires, cputime_t cur_time, int signo)
@@ -1709,13 +1744,13 @@ static __init int init_posix_cpu_timers(void)
 	};
 	struct timespec ts;
 
-	register_posix_clock(CLOCK_PROCESS_CPUTIME_ID, &process);
-	register_posix_clock(CLOCK_THREAD_CPUTIME_ID, &thread);
-
 	cputime_to_timespec(cputime_one_jiffy, &ts);
 	onecputick = ts.tv_nsec;
 	WARN_ON(ts.tv_sec != 0);
 
+	register_posix_clock(CLOCK_PROCESS_CPUTIME_ID, &process);
+	register_posix_clock(CLOCK_THREAD_CPUTIME_ID, &thread);
+
 	return 0;
 }
 __initcall(init_posix_cpu_timers);
-- 
1.6.2.5


View attachment "cputimer_test.c" of type "text/plain" (2671 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ