[PATCH 0/3] Cleanup ADJ_SETOFFSET patch

[PATCH 0/3] Cleanup ADJ_SETOFFSET patch

[John Stultz]

I was looking to queue Richard's ADJ_SETOFFSET patch to see
if it could be merged into -tip for 2.6.38, but on second 
glance I noticed the ugly local_irq_disable bits as well
as the fact that adding the offset uses a gettime/add/settime
pattern which will also add a small error as the action isn't
atomic.

So I implemented a timekeeping function to add a fixed offset:
timekeeping_inject_offset(), and reworked Richard's code to
make use of it.

Richard: Any objection here? Mind testing this with the rest
of your patch queue?

Thomas: Any comments? Does this seem reasonable for 2.6.38? 
Should I fold my cleanups into Richard's patch?

thanks
-john


CC: Richard Cochran <richard.cochran@omicron.at>
CC: Thomas Gleixner <tglx@linutronix.de>

John Stultz (2):
  time: Introduce timekeeping_inject_offset
  ntp: Change ADJ_SETOFFSET implementation to use
    timekeeping_inject_offset

Richard Cochran (1):
  ntp: add ADJ_SETOFFSET mode bit

 include/linux/time.h      |    1 +
 include/linux/timex.h     |    3 ++-
 kernel/time/ntp.c         |   13 +++++++++++++
 kernel/time/timekeeping.c |   36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 1 deletions(-)

-- 
1.7.3.2.146.gca209

[PATCH 1/3] time: Introduce timekeeping_inject_offset

[John Stultz]

This adds a kernel-internal timekeeping interface to add or subtract
a fixed amount from CLOCK_REALTIME. This makes it so kernel users or
interfaces trying to do so do not have to read the time, then add an
offset and then call settimeofday(), which adds some extra error in
comparision to just simply adding the offset in the kernel timekeeping
core.

CC: Thomas Gleixner <tglx@linutronix.de>
CC: Richard Cochran <richard.cochran@omicron.at>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/linux/time.h      |    1 +
 kernel/time/timekeeping.c |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/include/linux/time.h b/include/linux/time.h
index 9f15ac7..b402134 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -166,6 +166,7 @@ extern int timekeeping_valid_for_hres(void);
 extern u64 timekeeping_max_deferment(void);
 extern void update_wall_time(void);
 extern void timekeeping_leap_insert(int leapsecond);
+extern int timekeeping_inject_offset(struct timespec *ts);
 
 struct tms;
 extern void do_sys_times(struct tms *);
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 49010d8..77e79b3 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -340,6 +340,42 @@ int do_settimeofday(struct timespec *tv)
 
 EXPORT_SYMBOL(do_settimeofday);
 
+
+/**
+ * timekeeping_inject_offset - Adds or subtracts from the current time.
+ * @tv:		pointer to the timespec variable containing the offset
+ *
+ * Adds or subtracts an offset value from the current time.
+ */
+int timekeeping_inject_offset(struct timespec *ts)
+{
+	unsigned long flags;
+
+	if ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC)
+		return -EINVAL;
+
+	write_seqlock_irqsave(&xtime_lock, flags);
+
+	timekeeping_forward_now();
+
+	xtime = timespec_add(xtime, *ts);
+	wall_to_monotonic = timespec_sub(wall_to_monotonic, *ts);
+
+	timekeeper.ntp_error = 0;
+	ntp_clear();
+
+	update_vsyscall(&xtime, &wall_to_monotonic, timekeeper.clock,
+				timekeeper.mult);
+
+	write_sequnlock_irqrestore(&xtime_lock, flags);
+
+	/* signal hrtimers about time change */
+	clock_was_set();
+
+	return 0;
+}
+EXPORT_SYMBOL(timekeeping_inject_offset);
+
 /**
  * change_clocksource - Swaps clocksources if a new one is available
  *
-- 
1.7.3.2.146.gca209

[PATCH 2/3] ntp: add ADJ_SETOFFSET mode bit

[John Stultz]

From: Richard Cochran <richardcochran@gmail.com>

This patch adds a new mode bit into the timex structure. When set, the bit
instructs the kernel to add the given time value to the current time.

CC: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <880d82bb8120f73973db27e0c48e949014b1a106.1292512461.git.richard.cochran@omicron.at>
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/linux/timex.h |    3 ++-
 kernel/time/ntp.c     |   26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/include/linux/timex.h b/include/linux/timex.h
index 32d852f..82d4b24 100644
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -73,7 +73,7 @@ struct timex {
 	long tolerance;		/* clock frequency tolerance (ppm)
 				 * (read only)
 				 */
-	struct timeval time;	/* (read only) */
+	struct timeval time;	/* (read only, except for ADJ_SETOFFSET) */
 	long tick;		/* (modified) usecs between clock ticks */
 
 	long ppsfreq;           /* pps frequency (scaled ppm) (ro) */
@@ -101,6 +101,7 @@ struct timex {
 #define ADJ_ESTERROR		0x0008	/* estimated time error */
 #define ADJ_STATUS		0x0010	/* clock status */
 #define ADJ_TIMECONST		0x0020	/* pll time constant */
+#define ADJ_SETOFFSET		0x0040  /* add 'time' to current time */
 #define ADJ_TAI			0x0080	/* set TAI offset */
 #define ADJ_MICRO		0x1000	/* select microsecond resolution */
 #define ADJ_NANO		0x2000	/* select nanosecond resolution */
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index d232189..e9e3915 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -454,6 +454,7 @@ static inline void process_adjtimex_modes(struct timex *txc, struct timespec *ts
 int do_adjtimex(struct timex *txc)
 {
 	struct timespec ts;
+	ktime_t delta, kt;
 	int result;
 
 	/* Validate the data before disabling interrupts */
@@ -482,8 +483,33 @@ int do_adjtimex(struct timex *txc)
 			hrtimer_cancel(&leap_timer);
 	}
 
+	if (txc->modes & ADJ_SETOFFSET) {
+		/* Validate the delta value. */
+		if (txc->time.tv_sec && txc->time.tv_usec < 0)
+			return -EINVAL;
+
+		if (txc->modes & ADJ_NANO) {
+			struct timespec tmp;
+			tmp.tv_sec  = txc->time.tv_sec;
+			tmp.tv_nsec = txc->time.tv_usec;
+			delta = timespec_to_ktime(tmp);
+		} else
+			delta = timeval_to_ktime(txc->time);
+
+		/* Adding the delta should be an "atomic" operation. */
+		local_irq_disable();
+	}
+
 	getnstimeofday(&ts);
 
+	if (txc->modes & ADJ_SETOFFSET) {
+		kt = timespec_to_ktime(ts);
+		kt = ktime_add(kt, delta);
+		ts = ktime_to_timespec(kt);
+		do_settimeofday(&ts);
+		local_irq_enable();
+	}
+
 	write_seqlock_irq(&xtime_lock);
 
 	if (txc->modes & ADJ_ADJTIME) {
-- 
1.7.3.2.146.gca209

Re: [PATCH 2/3] ntp: add ADJ_SETOFFSET mode bit

[Kuwahara,T.]

On Tue, Dec 28, 2010 at 8:40 AM, John Stultz <john.stultz@linaro.org> wrote:
> From: Richard Cochran <richardcochran@gmail.com>
>
> This patch adds a new mode bit into the timex structure. When set, the
> bit instructs the kernel to add the given time value to the current time.

I came up with this simple solution: "Just use ADJ_OFFSET as usual,
but don't forget to disable the kernel PLL."

Here's my (untested) patch.

---
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index c631168..d492887 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -119,14 +119,21 @@
 	return div_s64(offset64 << (NTP_SCALE_SHIFT - SHIFT_FLL), secs);
 }

-static void ntp_update_offset(long offset)
+static void ntp_update_offset(long offset, struct timespec *ts)
 {
 	s64 freq_adj;
 	s64 offset64;
 	long secs;

-	if (!(time_status & STA_PLL))
+	if (!(time_status & STA_PLL)) {
+		offset64 = (s64)offset;
+		if (!(time_status & STA_NANO))
+			offset64 *= NSEC_PER_USEC;
+
+		set_normalized_timespec(ts, ts->tv_sec, offset64 + ts->tv_nsec);
+
 		return;
+	}

 	if (!(time_status & STA_NANO))
 		offset *= NSEC_PER_USEC;
@@ -430,7 +437,7 @@
 		time_tai = txc->constant;

 	if (txc->modes & ADJ_OFFSET)
-		ntp_update_offset(txc->offset);
+		ntp_update_offset(txc->offset, ts);

 	if (txc->modes & ADJ_TICK)
 		tick_usec = txc->tick;
@@ -526,6 +533,9 @@

 	write_sequnlock_irq(&xtime_lock);

+	if ((txc->modes & ADJ_OFFSET) && !(time_status & STA_PLL))
+		do_settimeofday(&ts);
+
 	txc->time.tv_sec = ts.tv_sec;
 	txc->time.tv_usec = ts.tv_nsec;
 	if (!(time_status & STA_NANO))
--

Re: [PATCH 2/3] ntp: add ADJ_SETOFFSET mode bit

[John Stultz]

On Wed, 2010-12-29 at 05:47 +0900, Kuwahara,T. wrote:
> On Tue, Dec 28, 2010 at 8:40 AM, John Stultz <john.stultz@linaro.org> wrote:
> > From: Richard Cochran <richardcochran@gmail.com>
> >
> > This patch adds a new mode bit into the timex structure. When set, the
> > bit instructs the kernel to add the given time value to the current time.
> 
> I came up with this simple solution: "Just use ADJ_OFFSET as usual,
> but don't forget to disable the kernel PLL."

Again, this seems to be trying to add new functionality into a corner
case of a existing mode.

I don't like this because it makes testing and validating that the code
is correct for its uses difficult. Especially given that a number of
combinations of modes might be set at once. What happens to freq
adjustments done at the same time as an ADJ_OFFSET - STA_PLL?

Personally, I see the adjtimex call as too flexible, as I'd prefer to
have the modes be exclusive (adjusting one thing per call). As it is
now, the kernel doesn't throw out enough invalid to invalid-ish cases.
ie: MOD_NANO|MOD_MICRO: totally cool! We should fix these and make the
input checking more strict, but in my mind moving to mode-numbers rather
then mode-flags would be much nicer (and more extendable).

Richard: Maybe this is a good thing to think about for clock_adjtime? If
we are adding a new syscall, maybe we should make sure we clean up some
of the old syscalls issues? It does add a good bit of complexity, as the
idea of clock_adjtime being a multiplexing adjtimex was nice and simple.
We'd also have to review the mode usage to see if multi-mode adjustments
in a single call are all that common or not.

Kuwahara: Maybe could you maybe better explain your passion against
using a new mode-flag for this new functionality? You seem dead set
against it, but have not expressed your rational well.

thanks
-john

Re: [PATCH 2/3] ntp: add ADJ_SETOFFSET mode bit

[Richard Cochran]

On Mon, Jan 03, 2011 at 12:44:26PM -0800, John Stultz wrote:
> On Wed, 2010-12-29 at 05:47 +0900, Kuwahara,T. wrote:
> > I came up with this simple solution: "Just use ADJ_OFFSET as usual,
> > but don't forget to disable the kernel PLL."
> 
> Again, this seems to be trying to add new functionality into a corner
> case of a existing mode.
> 
> I don't like this because it makes testing and validating that the code
> is correct for its uses difficult. Especially given that a number of
> combinations of modes might be set at once. What happens to freq
> adjustments done at the same time as an ADJ_OFFSET - STA_PLL?

Where are the mode bits and their combinations documented, anyhow?

There are lot of papers on NTP, and I have not read them all, but I
don't recall having seen a precise and succinct decription of the
ntp_adjtime interface anywhere.

Thanks,
Richard

Re: [PATCH 2/3] ntp: add ADJ_SETOFFSET mode bit

[John Stultz]

On Tue, 2011-01-04 at 09:37 +0100, Richard Cochran wrote:
> On Mon, Jan 03, 2011 at 12:44:26PM -0800, John Stultz wrote:
> > On Wed, 2010-12-29 at 05:47 +0900, Kuwahara,T. wrote:
> > > I came up with this simple solution: "Just use ADJ_OFFSET as usual,
> > > but don't forget to disable the kernel PLL."
> > 
> > Again, this seems to be trying to add new functionality into a corner
> > case of a existing mode.
> > 
> > I don't like this because it makes testing and validating that the code
> > is correct for its uses difficult. Especially given that a number of
> > combinations of modes might be set at once. What happens to freq
> > adjustments done at the same time as an ADJ_OFFSET - STA_PLL?
> 
> Where are the mode bits and their combinations documented, anyhow?
> 
> There are lot of papers on NTP, and I have not read them all, but I
> don't recall having seen a precise and succinct decription of the
> ntp_adjtime interface anywhere.

You can find the original ntp_adjtime description here:
Page 22 of:
http://www.eecis.udel.edu/~mills/database/rfc/rfc1589.txt

This pre-dates the nanotime kernel paper, which added MOD_NANO.

thanks
-john

Re: [PATCH 2/3] ntp: add ADJ_SETOFFSET mode bit

[Richard Cochran]

On Tue, Jan 04, 2011 at 08:52:58PM +0000, Kuwahara,T. wrote:
> You can obtain a more recent version of the timex.h and its related documentations from:
> http://www.eecis.udel.edu/~mills/ntp/html/kern.html
> 
> Download "nanokernel.tar.gz" and extract it.

Following the link, I get:

> wget ftp://ftp.udel.edu/pub/ntp/software/nanokernel.tar.gz
--2011-01-05 07:58:39--  ftp://ftp.udel.edu/pub/ntp/software/nanokernel.tar.gz
           => `nanokernel.tar.gz'
Resolving ftp.udel.edu... 128.4.40.10
Connecting to ftp.udel.edu|128.4.40.10|:21... failed: Connection refused.

Re: [PATCH 2/3] ntp: add ADJ_SETOFFSET mode bit

[Richard Cochran]

On Mon, Jan 03, 2011 at 12:44:26PM -0800, John Stultz wrote:
> Richard: Maybe this is a good thing to think about for clock_adjtime? If
> we are adding a new syscall, maybe we should make sure we clean up some
> of the old syscalls issues? It does add a good bit of complexity, as the
> idea of clock_adjtime being a multiplexing adjtimex was nice and simple.
> We'd also have to review the mode usage to see if multi-mode adjustments
> in a single call are all that common or not.

So, is the NTP source code the documentation of the kernel interface?

Thanks,
Richard

Re: [PATCH 2/3] ntp: add ADJ_SETOFFSET mode bit

[John Stultz]

On Tue, 2011-01-04 at 09:40 +0100, Richard Cochran wrote:
> On Mon, Jan 03, 2011 at 12:44:26PM -0800, John Stultz wrote:
> > Richard: Maybe this is a good thing to think about for clock_adjtime? If
> > we are adding a new syscall, maybe we should make sure we clean up some
> > of the old syscalls issues? It does add a good bit of complexity, as the
> > idea of clock_adjtime being a multiplexing adjtimex was nice and simple.
> > We'd also have to review the mode usage to see if multi-mode adjustments
> > in a single call are all that common or not.
> 
> So, is the NTP source code the documentation of the kernel interface?

Yea. adjtimex is a combination of ntp_adjtime and the older adjtime
interfaces. So its not identical to David Mill's design, but it is
compatible.  In fact, it wasn't until somewhat recently that it picked
up the ntpv4 changes and MOD/ADJ_NANO.

By the way, I'm not saying we should switch from using mode flags to
mode numbers for the new interface as I'm not sure if it would confuse
users moving to it (being very similar, but slightly different can be
worse then being totally different). But I figure it warrants some
consideration. We do still have 4 unused bits in the modes flags after
your patch, so this may be a premature worry.

thanks
-john

[PATCH 3/3] ntp: Change ADJ_SETOFFSET implementation to use timekeeping_inject_offset

[John Stultz]

Cleans up the ADJ_SETOFFSET implementation to use
timekeeping_inject_offset which greatly simplifies the code.

CC: Richard Cochran <richard.cochran@omicron.at>
CC: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/ntp.c |   25 ++++++-------------------
 1 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index e9e3915..18d7a0f 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -454,7 +454,6 @@ static inline void process_adjtimex_modes(struct timex *txc, struct timespec *ts
 int do_adjtimex(struct timex *txc)
 {
 	struct timespec ts;
-	ktime_t delta, kt;
 	int result;
 
 	/* Validate the data before disabling interrupts */
@@ -484,32 +483,20 @@ int do_adjtimex(struct timex *txc)
 	}
 
 	if (txc->modes & ADJ_SETOFFSET) {
+		struct timespec delta;
 		/* Validate the delta value. */
 		if (txc->time.tv_sec && txc->time.tv_usec < 0)
 			return -EINVAL;
 
-		if (txc->modes & ADJ_NANO) {
-			struct timespec tmp;
-			tmp.tv_sec  = txc->time.tv_sec;
-			tmp.tv_nsec = txc->time.tv_usec;
-			delta = timespec_to_ktime(tmp);
-		} else
-			delta = timeval_to_ktime(txc->time);
-
-		/* Adding the delta should be an "atomic" operation. */
-		local_irq_disable();
+		delta.tv_sec  = txc->time.tv_sec;
+		delta.tv_nsec = txc->time.tv_usec;
+		if (!(txc->modes & ADJ_NANO))
+			delta.tv_nsec *= 1000;
+		timekeeping_inject_offset(&delta);
 	}
 
 	getnstimeofday(&ts);
 
-	if (txc->modes & ADJ_SETOFFSET) {
-		kt = timespec_to_ktime(ts);
-		kt = ktime_add(kt, delta);
-		ts = ktime_to_timespec(kt);
-		do_settimeofday(&ts);
-		local_irq_enable();
-	}
-
 	write_seqlock_irq(&xtime_lock);
 
 	if (txc->modes & ADJ_ADJTIME) {
-- 
1.7.3.2.146.gca209

Re: [PATCH 0/3] Cleanup ADJ_SETOFFSET patch

[Richard Cochran]

On Mon, Dec 27, 2010 at 03:40:41PM -0800, John Stultz wrote:
> I was looking to queue Richard's ADJ_SETOFFSET patch to see
> if it could be merged into -tip for 2.6.38, but on second 
> glance I noticed the ugly local_irq_disable bits as well
> as the fact that adding the offset uses a gettime/add/settime
> pattern which will also add a small error as the action isn't
> atomic.
> 
> So I implemented a timekeeping function to add a fixed offset:
> timekeeping_inject_offset(), and reworked Richard's code to
> make use of it.

Okay, so you added an optimized version of do_settimeofday() for
jumping the clock. It certainly helps the code in do_adjtimex(), but
it also (nearly) duplicates do_settimeofday(). I guess you will not
mind having to maintain both code paths...

> Richard: Any objection here? Mind testing this with the rest
> of your patch queue?

Well, you have uncovered a problem.

The code I offered was broken to begin with, but I think your patch is
also troubled. The timespec is awkwardly split into seconds and
nanoseconds, and I think that arithmetic using timespecs is not well
defined. Or perhaps only I am confused by it all.

The problem seems to be, how can a timespec have a sign?

The exisiting code seems to assume that a timespec can only have the
sign in one field. In other words, if tv_sec is non-zero, then tv_nsec
must be non-negative. (I added a check for this into my patch).

I took a second look at ktime_add() vs. timespec_add() and discovered
a problems both. Consider the following test code:

static void kt_add(struct timespec now, struct timespec adj)
{
	ktime_t delta, kt;
	struct timespec ts;
	delta = timespec_to_ktime(adj);
	kt = timespec_to_ktime(now);
	kt = ktime_add(kt, delta);
	ts = ktime_to_timespec(kt);
	pr_err("kt add: {%ld,%ld} + {%ld,%ld} = {%ld,%ld}\n",
	       now.tv_sec, now.tv_nsec,
	       adj.tv_sec, adj.tv_nsec,
	       ts.tv_sec, ts.tv_nsec);
}

static void ts_add(struct timespec now, struct timespec adj)
{
	struct timespec ts;
	ts = timespec_add(now, adj);
	pr_err("ts add: {%ld,%ld} + {%ld,%ld} = {%ld,%ld}\n",
	       now.tv_sec, now.tv_nsec,
	       adj.tv_sec, adj.tv_nsec,
	       ts.tv_sec, ts.tv_nsec);
}

There are (at least) four cases to consider:

1. adj > 1.0

   kt add: {2,0} + {1,100} = {3,100}                                               
   ts add: {2,0} + {1,100} = {3,100}                                               

2. adj < -1.0

   kt add: {2,0} + {-1,100} = {1,100}                                              
   ts add: {2,0} + {-1,100} = {1,100}                                              

3. 0 < adj < 1.0

   kt add: {2,0} + {0,100} = {2,100}                                               
   ts add: {2,0} + {0,100} = {2,100}                                               

4. -1.0 < adj < 0

   kt add: {2,0} + {0,-100} = {6,294967196}                                        
   ts add: {2,0} + {0,-100} = {1,999999900}                                        

Case 2 fails for both functions.
Case 4 fails when using ktime_add().

So it seems that I have now way to correctly encode a negative offset
less than -1.0 into a timespec. Perhaps we could specify new rules for
timespecs.

1. Time Values:
   If tv_sec is non-zero, then tv_nsec must be non-negative.

2. Time Deltas:
   The sign of tv_sec and tv_nsec must be the same.

In any case, I would like you help in clarifying all of this...

Thanks,

Richard

Re: [PATCH 0/3] Cleanup ADJ_SETOFFSET patch

[Richard Cochran]

On Tue, Dec 28, 2010 at 12:20:00PM +0100, Richard Cochran wrote:
> The exisiting code seems to assume that a timespec can only have the
> sign in one field. In other words, if tv_sec is non-zero, then tv_nsec
> must be non-negative. (I added a check for this into my patch).

Looking at ktime.h we find:

 * Be especially aware that negative values are represented in a way
 * that the tv.sec field is negative and the tv.nsec field is greater
 * or equal to zero but less than nanoseconds per second. This is the
 * same representation which is used by timespecs.
 *
 *   tv.sec < 0 and 0 >= tv.nsec < NSEC_PER_SEC

So it seems that the time values (or time intervals) in the range
-1 < x < 0 are not even possible, from the point of view of ktime.

> So it seems that I have now way to correctly encode a negative offset

I meant, "no way"---------^

> less than -1.0 into a timespec. Perhaps we could specify new rules for
> timespecs.

> 1. Time Values:
>    If tv_sec is non-zero, then tv_nsec must be non-negative.
> 
> 2. Time Deltas:
>    The sign of tv_sec and tv_nsec must be the same.

For me, the confusion gets worse when you consider the timespec values
delivered by the system for dates before the 1970 epoch. In that case,
the kernel always gives the tv_nsec value as a positive number. The
value makes sense when interpreted as a sum.

Reading consecutive clock values every 0.1 seconds produces, for
example:

{sec, nsec}        sum
-------------------------
{-2, +600000000}  -1.4
{-2, +700000000}  -1.3
{-2, +800000000}  -1.2
{-2, +900000000}  -1.1
{-1,  000000000}  -1.0
{-1, +100000000}  -0.9
{-1, +200000000}  -0.8
{-1, +300000000}  -0.7

If we say that the time value or interval in a timespec is always the
sum of the fields, still it would seem that the ktime code is broken
according to that assumption.

Perhaps someone can clarify?

Thanks,
Richard

Re: [PATCH 0/3] Cleanup ADJ_SETOFFSET patch

[Richard Cochran]

On Tue, Dec 28, 2010 at 01:53:32PM +0100, Richard Cochran wrote:
> {sec, nsec}        sum
> -------------------------
> {-2, +600000000}  -1.4
> {-2, +700000000}  -1.3
> {-2, +800000000}  -1.2
> {-2, +900000000}  -1.1
> {-1,  000000000}  -1.0
> {-1, +100000000}  -0.9
> {-1, +200000000}  -0.8
> {-1, +300000000}  -0.7
> 
> If we say that the time value or interval in a timespec is always the
> sum of the fields, still it would seem that the ktime code is broken
> according to that assumption.

Okay, now I think I get it.

The value of a timespec is the sum of its fields. The tv_nsec field
should always be non-negative.

So, if a user space program wants to jump the clock back by one-tenth
second, it must pass a timespec with value {-1,900000000}.

Right?

Thanks,
Richard