Re: [Xenomai-core] latencys drifting into negative (Xenomai 2.4.2/2.4.3)

[Sebastian Smolorz <smolorz@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 4323 bytes --]

Jan Kiszka wrote:
> Sebastian Smolorz wrote:
>> Gilles Chanteperdrix wrote:
>>> On Wed, Apr 2, 2008 at 5:58 PM, Sebastian Smolorz
>>> <smolorz@domain.hid> wrote:
>>>> Jan Kiszka wrote:
>>>>  > Sebastian Smolorz wrote:
>>>>  >> Jan Kiszka wrote:
>>>>  >>> Cornelius Köpp wrote:
>>>>  >>>> I talked with Sebastian Smolorz about this and he builds his own
>>>>  >>>> independent kernel-config to check. He got the same 
>>>> drifting-effect
>>>>  >>>> with Xenomai 2.4.2 and Xenomai 2.4.3 running latency over several
>>>>  >>>> hours. His kernel-config ist attached as
>>>>  >>>> 'config-2.6.24-xenomai-2.4.3__ssm'.
>>>>  >>>>
>>>>  >>>> Our kernel-configs are both based on a config used with 
>>>> Xenomai 2.3.4
>>>>  >>>> and Linux 2.6.20.15 without any drifting effects.
>>>>  >>> 2.3.x did not incorporate the new TSC-to-ns conversion. Maybe 
>>>> it is
>>>>  >>> not a PIC vs. APIC thing, but rather a rounding problem of 
>>>> larger TSC
>>>>  >>> values (that naturally show up when the system runs for a 
>>>> longer time).
>>>>  >> This hint seems to point into the right direction. I tried out a
>>>>  >> modified pod_32.h (xnarch_tsc_to_ns() commented out) so that the 
>>>> old
>>>>  >> implementation in include/asm-generic/bits/pod.h was used. The 
>>>> drifting
>>>>  >> bug disappeared. So there seems so be a buggy x86-specific
>>>>  >> implementation of this routine.
>>>>  >
>>>>  > Hmm, maybe even a conceptional issue: the multiply-shift-based
>>>>  > xnarch_tsc_to_ns is not as precise as the still 
>>>> multiply-divide-based
>>>>  > xnarch_ns_to_tsc. So when converting from tsc over ns back to 
>>>> tsc, we
>>>>  > may loose some bits, maybe too many bits...
>>>>  >
>>>>  > It looks like this bites us in the kernel latency tests (-t2 should
>>>>  > suffer as well). Those recalculate their timeouts each round 
>>>> based on
>>>>  > absolute nanoseconds. In contrast, the periodic user mode task of 
>>>> -t0
>>>>  > uses a periodic timer that is forwarded via a tsc-based interval.
>>>>  >
>>>>  > You (or Cornelius) could try to analyse the calculation path of the
>>>>  > involved timeouts, specifically to understand why the scheduled 
>>>> timeout
>>>>  > of the underlying task timer (which is tsc-based) tend to diverge 
>>>> from
>>>>  > the calculated one (ns-based).
>>>>
>>>>  So here comes the explanation. The error is inside the function
>>>>  rthal_llmulshft(). It returns wrong values which are too small - the
>>>>  higher the given TSC value the bigger the error. The function
>>>>  rtdm_clock_read_monotonic() calls rthal_llmulshft(). As
>>>>  rtdm_clock_read_monotonic() is called every time the latency kernel
>>>>  thread runs [1] the values reported by latency become smaller over 
>>>> time.
>>>>
>>>>  In contrast, the latency task in user space only uses the conversion
>>>>  from TSC to ns only once when calling rt_timer_inquire [2].
>>>>  timer_info.date is too small, timer_info.tsc is right. So all 
>>>> calculated
>>>>   deltas in [3] are shifted to a smaller value. This value is constant
>>>>  during the runtime of lateny in user space because no more conversion
>>>>  from TSC to ns occurs.
>>>
>>> latency does conversions from tsc to ns, but it converts time
>>> differences, so the error is small relative to the results.
>>
>> Of course. I wasn't precise with my last statement. It should be: No 
>> more conversions from *absolute* TSC values to ns occur.
>>
> 
> This patch may do the trick: it uses the inverted tsc-to-ns function 
> instead of the frequency-based one. Be warned, it is totally untested 
> inside Xenomai, I just ran it in a user space test program. But it may 
> give an idea.

Your patch needed two minor corrections (ns instead of ts in functions 
xnarch_ns_to_tsc()) in order to compile. A short run (30 minutes) of 
latency -t1 seems to prove your bug-fix: There seems to be no drift.

If I got your patch correctly, it doesn't make xnarch_tsc_to_ns more 
precise but introduces a new function xnarch_ns_to_tsc() which is also 
less precise than the generic xnarch_ns_to_tsc(), right? So isn't there 
still the danger of getting wrong values when calling xnarch_tsc_to_ns() 
  not in combination with xnarch_ns_to_tsc()?

-- 
Sebastian

[-- Attachment #2: fixup-scaled-ns2tsc-conversion.patch --]
[-- Type: text/plain, Size: 2870 bytes --]

---
 include/asm-x86/bits/init_32.h |    3 ++-
 include/asm-x86/bits/init_64.h |    3 ++-
 include/asm-x86/bits/pod_32.h  |    7 +++++++
 include/asm-x86/bits/pod_64.h  |    7 +++++++
 4 files changed, 18 insertions(+), 2 deletions(-)

Index: b/include/asm-x86/bits/init_32.h
===================================================================
--- a/include/asm-x86/bits/init_32.h
+++ b/include/asm-x86/bits/init_32.h
@@ -73,7 +73,7 @@ int xnarch_calibrate_sched(void)
 
 static inline int xnarch_init(void)
 {
-	extern unsigned xnarch_tsc_scale, xnarch_tsc_shift;
+	extern unsigned xnarch_tsc_scale, xnarch_tsc_shift, xnarch_tsc_divide;
 	int err;
 
 	err = rthal_init();
@@ -89,6 +89,7 @@ static inline int xnarch_init(void)
 
 	xnarch_init_llmulshft(1000000000, RTHAL_CPU_FREQ,
 			      &xnarch_tsc_scale, &xnarch_tsc_shift);
+	xnarch_tsc_divide = 1 << xnarch_tsc_shift;
 
 	err = xnarch_calibrate_sched();
 
Index: b/include/asm-x86/bits/init_64.h
===================================================================
--- a/include/asm-x86/bits/init_64.h
+++ b/include/asm-x86/bits/init_64.h
@@ -70,7 +70,7 @@ int xnarch_calibrate_sched(void)
 
 static inline int xnarch_init(void)
 {
-	extern unsigned xnarch_tsc_scale, xnarch_tsc_shift;
+	extern unsigned xnarch_tsc_scale, xnarch_tsc_shift, xnarch_tsc_divide;
 	int err;
 
 	err = rthal_init();
@@ -86,6 +86,7 @@ static inline int xnarch_init(void)
 
 	xnarch_init_llmulshft(1000000000, RTHAL_CPU_FREQ,
 			      &xnarch_tsc_scale, &xnarch_tsc_shift);
+	xnarch_tsc_divide = 1 << xnarch_tsc_shift;
 
 	err = xnarch_calibrate_sched();
 
Index: b/include/asm-x86/bits/pod_32.h
===================================================================
--- a/include/asm-x86/bits/pod_32.h
+++ b/include/asm-x86/bits/pod_32.h
@@ -25,6 +25,7 @@
 
 unsigned xnarch_tsc_scale;
 unsigned xnarch_tsc_shift;
+unsigned xnarch_tsc_divide;
 
 long long xnarch_tsc_to_ns(long long ts)
 {
@@ -32,6 +33,12 @@ long long xnarch_tsc_to_ns(long long ts)
 }
 #define XNARCH_TSC_TO_NS
 
+long long xnarch_ns_to_tsc(long long ns)
+{
+	return xnarch_llimd(ns, xnarch_tsc_divide, xnarch_tsc_scale);
+}
+#define XNARCH_NS_TO_TSC
+
 #include <asm-generic/xenomai/bits/pod.h>
 #include <asm/xenomai/switch.h>
 
Index: b/include/asm-x86/bits/pod_64.h
===================================================================
--- a/include/asm-x86/bits/pod_64.h
+++ b/include/asm-x86/bits/pod_64.h
@@ -24,6 +24,7 @@
 
 unsigned xnarch_tsc_scale;
 unsigned xnarch_tsc_shift;
+unsigned xnarch_tsc_divide;
 
 long long xnarch_tsc_to_ns(long long ts)
 {
@@ -31,6 +32,12 @@ long long xnarch_tsc_to_ns(long long ts)
 }
 #define XNARCH_TSC_TO_NS
 
+long long xnarch_ns_to_tsc(long long ns)
+{
+	return xnarch_llimd(ns, xnarch_tsc_divide, xnarch_tsc_scale);
+}
+#define XNARCH_NS_TO_TSC
+
 #include <asm-generic/xenomai/bits/pod.h>
 #include <asm/xenomai/switch.h>