All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Stultz <john.stultz@linaro.org>
To: Daniel Kurtz <djkurtz@google.com>
Cc: linux-input@vger.kernel.org,
	"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
	"Arve Hjønnevåg" <arve@android.com>
Subject: Re: [RFC][PATCH] Input: Add infrastrucutre for selecting clockid for event time stamps.
Date: Fri, 06 Jan 2012 11:39:16 -0800	[thread overview]
Message-ID: <1325878756.2290.101.camel@work-vm> (raw)
In-Reply-To: <CAGS+omCB7L7w1NXiaNdBPWPjx4+5soeg8Qq9TLQtmn=b13cSAg@mail.gmail.com>

On Fri, 2012-01-06 at 17:20 +0800, Daniel Kurtz wrote:
> On Fri, Jan 6, 2012 at 10:50 AM, John Stultz <john.stultz@linaro.org> wrote:
> > Here's another revision, incorperating Dmitry's suggestion.
> >
> > As noted by Arve and others, since wall time can jump backwards, it
> > is difficult to use for input because one cannot determine if one
> > event occured before another or for how long a key was pressed.
> >
> > However, the timestamp field is part of the kernel ABI, and cannot
> > be changed without possibly breaking existing users.
> >
> > This patch adds a new IOCTL that allows a clockid to be set in
> > the evdev_client struct that will specify which time base to
> > use for event timestamps (ie: CLOCK_MONOTONIC instead
> > of CLOCK_REALTIME).
> >
> > For now we only support CLOCK_MONOTONIC and CLOCK_REALTIME, but
> > in the future we could support other clockids if appropriate.
> 
> What about CLOCK_MONOTONIC_RAW?

It could be used here, although I'm still not clear on the benefit of
using monotonic raw over just monotonic.

> Last time we discussed, I thought this clock was the most useful for
> use with input devices.  But, you wrote this:
> > So rawmonotonic isn't frequency corrected via NTP, while the monotonic
> > clock is. So if you're calculating intervals, you will get more accurate
> > times (where a second is a second) w/ ktime_get_ts().
> 
> Does this frequency correction involve timestamp jumps?

No. clock monotonic isn't jumped. But its rate may be slightly adjusted
(max of +/- 500ppm), so that it accurately matches the passing of time. 

> If so, of what magnitude?

At worse, assuming some sort of terribly mis-configured or malicious ntp
daemon, if you had two one second intervals that you had measured, they
could actually differ by up to a millisecond. 

> In my experience, input event timestamp intervals are usually on the
> order of a few milliseconds (5-25 ms).  If CLOCK_MONOTONIC experiences
> frequent adjustments near this order of magnitude, I still think
> CLOCK_MONOTONIC_RAW might be a better choice for event timestamps.

So for 5-25 ms intervals, you're looking at a worse case difference of
5-25 us. And again, this isn't likely, as the ntp freq adjustment would
have to go from -500ppm to +500ppm mid-interval. 

So I really suspect there isn't much practical difference between
CLOCK_MONOTONIC and CLOCK_MONOTONIC_RAW for input events. But I'd be
interested to hear if anyone has actually run into such complications.

CLOCK_MONOTONIC_RAW can be useful (especially for providing a stable
baseline when doing time adjustment calculations), but the downside is
that measurements (especially longer measurements) with
CLOCK_MONOTONIC_RAW may not be accurate.

And as it always is with time, everything is relative: even
CLOCK_MONOTONIC_RAW can vary over time, as the crystal driving the
clocksource may slightly fluctuate with temperature.

But again, nothing is keeping CLOCK_MONOTONIC_RAW from being supported
via the proposed ioctl.

> > +       bool timestamp_clkid;
> 
> This isn't bool anymore.

Good catch!

thanks!
-john


  reply	other threads:[~2012-01-06 19:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-06  2:50 [RFC][PATCH] Input: Add infrastrucutre for selecting clockid for event time stamps John Stultz
2012-01-06  9:20 ` Daniel Kurtz
2012-01-06 19:39   ` John Stultz [this message]
  -- strict thread matches above, loose matches on Subject: below --
2012-01-07  3:40 John Stultz
2012-01-07  6:42 ` Daniel Kurtz
2012-01-09 18:04   ` Dmitry Torokhov
     [not found]     ` <CAGS+omDkkH_cVQV4CvF1iz-aSRdNTGcsEghXU3MNBLAgsp7jFA@mail.gmail.com>
2012-01-09 18:48       ` Dmitry Torokhov
2012-01-09 22:49     ` Arve Hjønnevåg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1325878756.2290.101.camel@work-vm \
    --to=john.stultz@linaro.org \
    --cc=arve@android.com \
    --cc=djkurtz@google.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-input@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.