Re: [PATCH 1/5] Add functions producing system time given a backing counter value

[Thomas Gleixner]

On Wed, 29 Jul 2015, Thomas Gleixner wrote:
 On Mon, 27 Jul 2015, John Stultz wrote:
  On Mon, Jul 27, 2015 at 8:44 PM, John Stultz john.stu...@linaro.org wrote:
   On Mon, Jul 27, 2015 at 5:46 PM, Christopher Hall
   christopher.s.h...@intel.com wrote:
   * counter_to_rawmono64
   * counter_to_mono64
   * counter_to_realtime64
  
   Enables drivers to translate a captured system clock counter to system
   time. This is useful for network and audio devices that capture 
   timestamps
   in terms of both the system clock and device clock.
  
   Huh.  So for counter_to_realtime64  mono64, this seems to ignore the
   fact that the multiplier is constantly adjusted and corrected. So that
   calling the function twice with the same counter value may result in
   different returned values.
  
   I've not yet groked the whole patchset, but it seems like there needs
   to be some mechanism that ensures the counter value is captured and
   used in the same (or at least close) interval that the timekeeper data
   is valid for.
  
  
  So reading through. It looks like you only use art_to_realtime(), right?
  
  So again, since CLOCK_MONOTONIC and CLOCK_REALTIME are constaly being
  frequency adjusted, it might be best to construct this delta in the
  following way.
  
  
  Add counter_to_rawmono64(), which should be able to safely calculate
  the corresponding CLOCK_MONOTONIC_RAW time from any given cycle value.
  
  Use getnstime_raw_and_real() to get a immediate snapshot of current
  MONOTONIC_RAW  and REALTIME clocks.
  
  Then calculate the delta between the snapshotted counter raw time, and
  the current raw time.  Then apply that offset to the current realtime.
  
  The larger the raw-time delta, the larger the possible realtime error.
  But I think that will be as good as it gets.
 
 I think that's still not the right approach. The whole purpose of this
 is to get a precise snapshot of 
 
- PTP time from the ETH device
 and
- current system time
 
 Right now this does
 
   ktime_get_real();
   read_ptp_time();
 
 which is obviously not precise.
 
 The new hardware allows you to latch PTP time and ART time atomically
 in the ETH device and read them out.
 
 ART is the base clock of the TSC where
 
 TSC = K + (ART * n) / d;
 
 So for this to work proper, we need a function which converts ART to
 TSC. This is obviously x86/TSC specific code.
 
 Now on the PTP side we need a callback provided by the device driver
 to get the snapshot of the PTP and the ART.
 
 So the proposed implementation merily calls that callback from the PTP
 ioctl and then tries to do a magic CLOCK_REALTIME conversion of the
 ART value. But that's just wrong as it does not guarantee a proper
 correlation to the core timekeeping.
 
 So what we really need is a function in the timekeeper core which gets
 the PTP/ART timestamps from the device under the timekeeper sequence
 counter and converts to clock realtime and raw monotonic.
 
 That function is then called from the PTP ioctl.
 
 Anything else is just 'lets hope it works and is precise enough'
 voodoo.
 
 Something like the below untested patch should be all we need for PTP
 to be as precise as possible.
 
 I don't know whether we need functionality to convert arbitrary
 timestamps at all, but if we really need them then they are going to
 be pretty simple and explicitely not precise for anything else than
 clock monotonic raw. But that's a different story.
 
 Lets concentrate on PTP first and talk about the other stuff once we
 settled the use case which actually has a precision requirement.

Gah crap. Picked a stale version of the patch.
 
Thanks,

tglx

-
Subject: ptp: Get sync timestamps
From: Thomas Gleixner t...@linutronix.de
Date: Wed, 29 Jul 2015 10:52:06 +0200

The ART stuff wants to be splitted out.

Not-Signed-off-by: Thomas Gleixner t...@linutronix.de
---
 arch/x86/kernel/tsc.c   |   31 +
 include/linux/clocksource.h |   30 
 include/linux/timekeeping.h |4 ++
 kernel/time/timekeeping.c   |   63 
 4 files changed, 128 insertions(+)

Index: linux/arch/x86/kernel/tsc.c
===
--- linux.orig/arch/x86/kernel/tsc.c
+++ linux/arch/x86/kernel/tsc.c
@@ -1059,6 +1059,27 @@ int unsynchronized_tsc(void)
return 0;
 }
 
+static u32 tsc_numerator;
+static u32 tsc_denominator;
+/*
+ * CHECKME: Do we need the adjust value? It should be 0, but if we run
+ * in a VM this might be a different story.
+ */
+static u64 tsc_adjust;
+
+static u64 art_to_tsc(u64 cycles)
+{
+   u64 tmp, res = tsc_adjust;
+
+   res += (cycles / tsc_denominator) * tsc_numerator;
+   tmp = (cycles % tsc_denominator) * tsc_numerator;
+   res += tmp / tsc_denominator;
+   return res;
+}
+
+struct correlated_cs art_timestamper = {
+   .convert= art_to_tsc,
+};
 
 

Re: [PATCH 1/5] Add functions producing system time given a backing counter value

[Richard Cochran]

On Wed, Jul 29, 2015 at 12:49:44PM +0200, Peter Zijlstra wrote:
 This is still fuzzy, right? The hardware ART timestamp could be from
 _before_ the NTP adjust; or the NTP adjust could happen while we do this
 conversion and we'll take the retry loop.

In the original series, yes.
 
 In both cases, the resulting value is computed using a different slope
 than was in effect at the time of the stamp. With the end result being
 slightly off from what it should be.

In Thomas' patch the get_ts() is meant to read fresh pairs of time
stamps from within the loop.


Thanks,
Richard
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] Add functions producing system time given a backing counter value

[Peter Zijlstra]

On Wed, Jul 29, 2015 at 12:19:06PM +0200, Thomas Gleixner wrote:
 I don't know whether we need functionality to convert arbitrary
 timestamps at all, but if we really need them then they are going to
 be pretty simple and explicitely not precise for anything else than
 clock monotonic raw. But that's a different story.

I think we need that too, and agreed, given NTP anything other than
MONO_RAW is going to be fuzzy at best.

 Index: linux/arch/x86/kernel/tsc.c
 ===
 --- linux.orig/arch/x86/kernel/tsc.c
 +++ linux/arch/x86/kernel/tsc.c
 @@ -1059,6 +1059,23 @@ int unsynchronized_tsc(void)
   return 0;
  }
  
 +static u32 tsc_numerator;
 +static u32 tsc_denominator;
 +/*
 + * CHECKME: Do we need the adjust value? It should be 0, but if we run
 + * in a VM this might be a different story.
 + */
 +static u64 tsc_adjust;
 +
 +static u64 art_to_tsc(u64 cycles)
 +{
 + /* FIXME: This needs 128bit math to work proper */
 + return tsc_adjust + (cycles * tsc_numerator) / tsc_denominator;

Yeah, its really unfortunate its given as a divisor and not a shift.
That said I think, at least on the initial hardware, its 2, so:

return mul_u64_u32_shr(cycles, tsc_numerator, 1);

Should do, given that TSC_ADJUST had better be 0.

 +}



 + * get_correlated_timestamp - Get a correlated timestamp
 + *
 + * Reads a timestamp from a device and correlates it to system time
 + */
 +int get_correlated_timestamp(struct correlated_ts *crt,
 +  struct correlated_cs *crs)
 +{
 + struct timekeeper *tk = tk_core.timekeeper;
 + unsigned long seq;
 + cycles_t cycles;
 + ktime_t base;
 + s64 nsecs;
 + int ret;
 +
 + do {
 + seq = read_seqcount_begin(tk_core.seq);
 + /*
 +  * Verify that the correlated clocksoure is related to
 +  * the currently installed timekeeper clocksoure
 +  */
 + if (tk-tkr_mono.clock != crs-related_cs)
 + return -ENODEV;
 +
 + /*
 +  * Try to get a timestamp from the device.
 +  */
 + ret = crt-get_ts(crt);
 + if (ret)
 + return ret;
 +
 + /*
 +  * Convert the timestamp to timekeeper clock cycles
 +  */
 + cycles = crs-convert(crs, crt-system_ts);
 +
 + /* Convert to clock realtime */
 + base = ktime_add(tk-tkr_mono.base, 
 tk_core.timekeeper.offs_real);
 + nsecs = timekeeping_convert_to_ns(tk-tkr_mono, cycles);
 + crt-system_real = ktime_add_ns(base, nsecs);
 +
 + /* Convert to clock raw monotonic */
 + base = tk-tkr_raw.base;
 + nsecs = timekeeping_convert_to_ns(tk-tkr_raw, cycles);
 + crt-system_raw = ktime_add_ns(base, nsecs);
 +
 + } while (read_seqcount_retry(tk_core.seq, seq));
 + return 0;
 +}

This is still fuzzy, right? The hardware ART timestamp could be from
_before_ the NTP adjust; or the NTP adjust could happen while we do this
conversion and we'll take the retry loop.

In both cases, the resulting value is computed using a different slope
than was in effect at the time of the stamp. With the end result being
slightly off from what it should be.

With the PTP case the ART timestamp is very recent, so the fuzz is
smallest, but its still there.

Any other ART users (buffered ETH frames) the delay will only get
bigger.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] Add functions producing system time given a backing counter value

[Peter Zijlstra]

On Wed, Jul 29, 2015 at 12:23:27PM +0200, Thomas Gleixner wrote:
 +/*
 + * CHECKME: Do we need the adjust value? It should be 0, but if we run
 + * in a VM this might be a different story.
 + */
 +static u64 tsc_adjust;

Urgh, VMs have !0 values here?
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] Add functions producing system time given a backing counter value

[Thomas Gleixner]

On Mon, 27 Jul 2015, John Stultz wrote:
 On Mon, Jul 27, 2015 at 8:44 PM, John Stultz john.stu...@linaro.org wrote:
  On Mon, Jul 27, 2015 at 5:46 PM, Christopher Hall
  christopher.s.h...@intel.com wrote:
  * counter_to_rawmono64
  * counter_to_mono64
  * counter_to_realtime64
 
  Enables drivers to translate a captured system clock counter to system
  time. This is useful for network and audio devices that capture timestamps
  in terms of both the system clock and device clock.
 
  Huh.  So for counter_to_realtime64  mono64, this seems to ignore the
  fact that the multiplier is constantly adjusted and corrected. So that
  calling the function twice with the same counter value may result in
  different returned values.
 
  I've not yet groked the whole patchset, but it seems like there needs
  to be some mechanism that ensures the counter value is captured and
  used in the same (or at least close) interval that the timekeeper data
  is valid for.
 
 
 So reading through. It looks like you only use art_to_realtime(), right?
 
 So again, since CLOCK_MONOTONIC and CLOCK_REALTIME are constaly being
 frequency adjusted, it might be best to construct this delta in the
 following way.
 
 
 Add counter_to_rawmono64(), which should be able to safely calculate
 the corresponding CLOCK_MONOTONIC_RAW time from any given cycle value.
 
 Use getnstime_raw_and_real() to get a immediate snapshot of current
 MONOTONIC_RAW  and REALTIME clocks.
 
 Then calculate the delta between the snapshotted counter raw time, and
 the current raw time.  Then apply that offset to the current realtime.
 
 The larger the raw-time delta, the larger the possible realtime error.
 But I think that will be as good as it gets.

I think that's still not the right approach. The whole purpose of this
is to get a precise snapshot of 

   - PTP time from the ETH device
and
   - current system time

Right now this does

  ktime_get_real();
  read_ptp_time();

which is obviously not precise.

The new hardware allows you to latch PTP time and ART time atomically
in the ETH device and read them out.

ART is the base clock of the TSC where

TSC = K + (ART * n) / d;

So for this to work proper, we need a function which converts ART to
TSC. This is obviously x86/TSC specific code.

Now on the PTP side we need a callback provided by the device driver
to get the snapshot of the PTP and the ART.

So the proposed implementation merily calls that callback from the PTP
ioctl and then tries to do a magic CLOCK_REALTIME conversion of the
ART value. But that's just wrong as it does not guarantee a proper
correlation to the core timekeeping.

So what we really need is a function in the timekeeper core which gets
the PTP/ART timestamps from the device under the timekeeper sequence
counter and converts to clock realtime and raw monotonic.

That function is then called from the PTP ioctl.

Anything else is just 'lets hope it works and is precise enough'
voodoo.

Something like the below untested patch should be all we need for PTP
to be as precise as possible.

I don't know whether we need functionality to convert arbitrary
timestamps at all, but if we really need them then they are going to
be pretty simple and explicitely not precise for anything else than
clock monotonic raw. But that's a different story.

Lets concentrate on PTP first and talk about the other stuff once we
settled the use case which actually has a precision requirement.

Thanks,

tglx

-
Subject: ptp: Get sync timestamps
From: Thomas Gleixner t...@linutronix.de
Date: Wed, 29 Jul 2015 10:52:06 +0200

The ART stuff wants to be splitted out.

Not-Signed-off-by: Thomas Gleixner t...@linutronix.de
---
 arch/x86/kernel/tsc.c   |   27 ++
 include/linux/clocksource.h |   30 
 include/linux/timekeeping.h |4 ++
 kernel/time/timekeeping.c   |   63 
 4 files changed, 124 insertions(+)

Index: linux/arch/x86/kernel/tsc.c
===
--- linux.orig/arch/x86/kernel/tsc.c
+++ linux/arch/x86/kernel/tsc.c
@@ -1059,6 +1059,23 @@ int unsynchronized_tsc(void)
return 0;
 }
 
+static u32 tsc_numerator;
+static u32 tsc_denominator;
+/*
+ * CHECKME: Do we need the adjust value? It should be 0, but if we run
+ * in a VM this might be a different story.
+ */
+static u64 tsc_adjust;
+
+static u64 art_to_tsc(u64 cycles)
+{
+   /* FIXME: This needs 128bit math to work proper */
+   return tsc_adjust + (cycles * tsc_numerator) / tsc_denominator;
+}
+
+struct correlated_cs art_timestamper = {
+   .convert= art_to_tsc,
+};
 
 static void tsc_refine_calibration_work(struct work_struct *work);
 static DECLARE_DELAYED_WORK(tsc_irqwork, tsc_refine_calibration_work);
@@ -1129,6 +1146,16 @@ static void tsc_refine_calibration_work(
(unsigned long)tsc_khz / 1000,
(unsigned 

Re: [PATCH 1/5] Add functions producing system time given a backing counter value

[Peter Zijlstra]

On Wed, Jul 29, 2015 at 01:48:41PM +0200, Richard Cochran wrote:
 On Wed, Jul 29, 2015 at 12:49:44PM +0200, Peter Zijlstra wrote:
  This is still fuzzy, right? The hardware ART timestamp could be from
  _before_ the NTP adjust; or the NTP adjust could happen while we do this
  conversion and we'll take the retry loop.
 
 In the original series, yes.
  
  In both cases, the resulting value is computed using a different slope
  than was in effect at the time of the stamp. With the end result being
  slightly off from what it should be.
 
 In Thomas' patch the get_ts() is meant to read fresh pairs of time
 stamps from within the loop.

Ah, indeed, I missed that. Yes if that's possible we get it right.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] Add functions producing system time given a backing counter value

[Thomas Gleixner]

On Wed, 29 Jul 2015, Peter Zijlstra wrote:
 On Wed, Jul 29, 2015 at 12:19:06PM +0200, Thomas Gleixner wrote:
  I don't know whether we need functionality to convert arbitrary
  timestamps at all, but if we really need them then they are going to
  be pretty simple and explicitely not precise for anything else than
  clock monotonic raw. But that's a different story.
 
 I think we need that too, and agreed, given NTP anything other than
 MONO_RAW is going to be fuzzy at best.

Yes, but that's a trivial case.
 
  +static u64 art_to_tsc(u64 cycles)
  +{
  +   /* FIXME: This needs 128bit math to work proper */
  +   return tsc_adjust + (cycles * tsc_numerator) / tsc_denominator;
 
 Yeah, its really unfortunate its given as a divisor and not a shift.
 That said I think, at least on the initial hardware, its 2, so:
 
   return mul_u64_u32_shr(cycles, tsc_numerator, 1);
 
 Should do, given that TSC_ADJUST had better be 0.

I don't trust BIOS folks :)

+   u64 tmp, res = tsc_adjust;
+
+   res += (cycles / tsc_denominator) * tsc_numerator;
+   tmp = (cycles % tsc_denominator) * tsc_numerator;
+   res += tmp / tsc_denominator;
+   return res;

That's what I have in my final patch.
 
  +   do {
  +   seq = read_seqcount_begin(tk_core.seq);
  +   /*
  +* Verify that the correlated clocksoure is related to
  +* the currently installed timekeeper clocksoure
  +*/
  +   if (tk-tkr_mono.clock != crs-related_cs)
  +   return -ENODEV;
  +
  +   /*
  +* Try to get a timestamp from the device.
  +*/
  +   ret = crt-get_ts(crt);
  +   if (ret)
  +   return ret;
  +
  +   /*
  +* Convert the timestamp to timekeeper clock cycles
  +*/
  +   cycles = crs-convert(crs, crt-system_ts);
  +
  +   /* Convert to clock realtime */
  +   base = ktime_add(tk-tkr_mono.base, 
  tk_core.timekeeper.offs_real);
  +   nsecs = timekeeping_convert_to_ns(tk-tkr_mono, cycles);
  +   crt-system_real = ktime_add_ns(base, nsecs);
  +
  +   /* Convert to clock raw monotonic */
  +   base = tk-tkr_raw.base;
  +   nsecs = timekeeping_convert_to_ns(tk-tkr_raw, cycles);
  +   crt-system_raw = ktime_add_ns(base, nsecs);
  +
  +   } while (read_seqcount_retry(tk_core.seq, seq));
  +   return 0;
  +}
 
 This is still fuzzy, right? The hardware ART timestamp could be from
 _before_ the NTP adjust; or the NTP adjust could happen while we do this
 conversion and we'll take the retry loop.

I read the timestamp pair in the loop, so its always consistent.
 
 Any other ART users (buffered ETH frames) the delay will only get
 bigger.

That's a different story and we really can only convert this to
monotonic raw.

Thanks,

tglx


--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/5] Add functions producing system time given a backing counter value

[Thomas Gleixner]

On Wed, 29 Jul 2015, Hall, Christopher S wrote:
 
  -Original Message-
  From: John Stultz [mailto:john.stu...@linaro.org]
  Sent: Monday, July 27, 2015 8:44 PM
  To: Hall, Christopher S
  Cc: Thomas Gleixner; Richard Cochran; Ingo Molnar; Kirsher, Jeffrey T;
  Ronciak, John; H. Peter Anvin; x...@kernel.org; lkml;
  netdev@vger.kernel.org
  Subject: Re: [PATCH 1/5] Add functions producing system time given a
  backing counter value
  
  On Mon, Jul 27, 2015 at 5:46 PM, Christopher Hall
  christopher.s.h...@intel.com wrote:
   * counter_to_rawmono64
   * counter_to_mono64
   * counter_to_realtime64
  
   Enables drivers to translate a captured system clock counter to system
   time. This is useful for network and audio devices that capture
  timestamps
   in terms of both the system clock and device clock.
  
  Huh.  So for counter_to_realtime64  mono64, this seems to ignore the
  fact that the multiplier is constantly adjusted and corrected. So that
  calling the function twice with the same counter value may result in
  different returned values.
  
  I've not yet groked the whole patchset, but it seems like there needs
  to be some mechanism that ensures the counter value is captured and
  used in the same (or at least close) interval that the timekeeper data
  is valid for.
 
 The ART (and derived TSC) values are always in the past.  There's no
 chance that we could exceed the interval.  I don't think any similar
 usage would be a problem either.
 
 Are you suggesting that, for completeness, this be enforced by the
 conversion function?
 
 I do a check here to make sure that the current counter value isn't before
 the beginning of the current interval:
 
 timekeeping_get_delta()
 ...
if (cycle_now  tkr-cycle_last 
tkr-cycle_last - cycle_now  ROLLOVER_THRESHOLD)
 return -EAGAIN;
 
 If tkr-cycle_last - cycle_now is large, the assumption is that
 rollover occurred.  Otherwise, the caller should re-read the counter
 so that it falls within the current interval.  In my normal use
 testing, re-read never occurred.

Sure that never happens, because your rollover value is 2  39 for
whatever reasons.

So on a 1GHz machine that is (2  39) / 1e9 ~= 1099.51 seconds.

Oh well.

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] Add functions producing system time given a backing counter value

[Richard Cochran]

On Wed, Jul 29, 2015 at 12:23:27PM +0200, Thomas Gleixner wrote:
  Something like the below untested patch should be all we need for PTP
  to be as precise as possible.

Yes, that is as good as it can be.  The code protects against
concurrent NTP adjustments, and the PTP driver will have to block
changes to its clock during the ioctl.

  I don't know whether we need functionality to convert arbitrary
  timestamps at all, but if we really need them then they are going to
  be pretty simple and explicitely not precise for anything else than
  clock monotonic raw. But that's a different story.
  
  Lets concentrate on PTP first and talk about the other stuff once we
  settled the use case which actually has a precision requirement.

The PTP ioctl only needs the REALTIME value, and so the MONO-RAW bit
could be dropped for now.

Thanks,
Richard



--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/5] Add functions producing system time given a backing counter value

[Hall, Christopher S]

 -Original Message-
 From: John Stultz [mailto:john.stu...@linaro.org]
 Sent: Monday, July 27, 2015 8:44 PM
 To: Hall, Christopher S
 Cc: Thomas Gleixner; Richard Cochran; Ingo Molnar; Kirsher, Jeffrey T;
 Ronciak, John; H. Peter Anvin; x...@kernel.org; lkml;
 netdev@vger.kernel.org
 Subject: Re: [PATCH 1/5] Add functions producing system time given a
 backing counter value
 
 On Mon, Jul 27, 2015 at 5:46 PM, Christopher Hall
 christopher.s.h...@intel.com wrote:
  * counter_to_rawmono64
  * counter_to_mono64
  * counter_to_realtime64
 
  Enables drivers to translate a captured system clock counter to system
  time. This is useful for network and audio devices that capture
 timestamps
  in terms of both the system clock and device clock.
 
 Huh.  So for counter_to_realtime64  mono64, this seems to ignore the
 fact that the multiplier is constantly adjusted and corrected. So that
 calling the function twice with the same counter value may result in
 different returned values.
 
 I've not yet groked the whole patchset, but it seems like there needs
 to be some mechanism that ensures the counter value is captured and
 used in the same (or at least close) interval that the timekeeper data
 is valid for.

The ART (and derived TSC) values are always in the past.  There's no
chance that we could exceed the interval.  I don't think any similar
usage would be a problem either.

Are you suggesting that, for completeness, this be enforced by the
conversion function?

I do a check here to make sure that the current counter value isn't before
the beginning of the current interval:

timekeeping_get_delta()
...
   if (cycle_now  tkr-cycle_last 
   tkr-cycle_last - cycle_now  ROLLOVER_THRESHOLD)
return -EAGAIN;

If tkr-cycle_last - cycle_now is large, the assumption is that
rollover occurred.  Otherwise, the caller should re-read the counter
so that it falls within the current interval.  In my normal use
testing, re-read never occurred.

Thanks for your input.

Chris

 
 thanks
 -john
N�r��yb�X��ǧv�^�)޺{.n�+���z�^�)w*jg����ݢj/���z�ޖ��2�ޙ�)ߡ�a�����G���h��j:+v���w��٥

Re: [PATCH 1/5] Add functions producing system time given a backing counter value

[John Stultz]

On Mon, Jul 27, 2015 at 8:44 PM, John Stultz john.stu...@linaro.org wrote:
 On Mon, Jul 27, 2015 at 5:46 PM, Christopher Hall
 christopher.s.h...@intel.com wrote:
 * counter_to_rawmono64
 * counter_to_mono64
 * counter_to_realtime64

 Enables drivers to translate a captured system clock counter to system
 time. This is useful for network and audio devices that capture timestamps
 in terms of both the system clock and device clock.

 Huh.  So for counter_to_realtime64  mono64, this seems to ignore the
 fact that the multiplier is constantly adjusted and corrected. So that
 calling the function twice with the same counter value may result in
 different returned values.

 I've not yet groked the whole patchset, but it seems like there needs
 to be some mechanism that ensures the counter value is captured and
 used in the same (or at least close) interval that the timekeeper data
 is valid for.


So reading through. It looks like you only use art_to_realtime(), right?

So again, since CLOCK_MONOTONIC and CLOCK_REALTIME are constaly being
frequency adjusted, it might be best to construct this delta in the
following way.


Add counter_to_rawmono64(), which should be able to safely calculate
the corresponding CLOCK_MONOTONIC_RAW time from any given cycle value.

Use getnstime_raw_and_real() to get a immediate snapshot of current
MONOTONIC_RAW  and REALTIME clocks.

Then calculate the delta between the snapshotted counter raw time, and
the current raw time.  Then apply that offset to the current realtime.

The larger the raw-time delta, the larger the possible realtime error.
But I think that will be as good as it gets.

This should simplify the interfaces you're adding to the timekeeping core.

thanks
-john
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] Add functions producing system time given a backing counter value

[John Stultz]

On Mon, Jul 27, 2015 at 5:46 PM, Christopher Hall
christopher.s.h...@intel.com wrote:
 * counter_to_rawmono64
 * counter_to_mono64
 * counter_to_realtime64

 Enables drivers to translate a captured system clock counter to system
 time. This is useful for network and audio devices that capture timestamps
 in terms of both the system clock and device clock.

Huh.  So for counter_to_realtime64  mono64, this seems to ignore the
fact that the multiplier is constantly adjusted and corrected. So that
calling the function twice with the same counter value may result in
different returned values.

I've not yet groked the whole patchset, but it seems like there needs
to be some mechanism that ensures the counter value is captured and
used in the same (or at least close) interval that the timekeeper data
is valid for.

thanks
-john
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html