public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Dike <jdike@addtoit.com>
To: Thomas Gleixner <tglx@linutronix.de>, linux-kernel@vger.kernel.org
Cc: "Christopher S. Aker" <caker@theshore.net>,
	user-mode-linux-devel@lists.sourceforge.net
Subject: non-scalar ktime addition and subtraction broken
Date: Thu, 1 Jun 2006 23:08:25 -0400	[thread overview]
Message-ID: <20060602030825.GA8006@ccure.user-mode-linux.org> (raw)

The use of 64-bit additions and subtractions on something which is
nominally a struct containing 32-bit second and nanosecond field is
broken when a negative time is involved.  When the structure is
treated as a 64-bit integer, the increment of the upper 32 bits that's
part of two's-complement subtraction is lost.  This leaves the end
result off by one second.

This manifested itself with sleeps inside UML lasting about 1 second
shorter than expected.

The patch below is more a problem statement than a real fix.  People
thought about performance, and I don't know what this does to that
work.

I'm not sure why the hrtimer.c part is needed - I had done that before
tracking down the ktime_add problem.  I see short sleeps without it,
so it is needed somehow.

The ktime_sub piece was done for completeness - UML compiles and boots
with no apparent ill effects, but it's otherwise untested.

As an aside, I fail to see how it can be correct for ktime_sub to add
NSEC_PER_SEC to something without compensating somewhere else for it.

Andrew - please don't drop this into -mm without an OK from Thomas or
someone else who's familiar with this code :-)

				Jeff

Index: linux-2.6.17-mm/include/linux/ktime.h
===================================================================
--- linux-2.6.17-mm.orig/include/linux/ktime.h	2006-06-01 22:15:44.000000000 -0400
+++ linux-2.6.17-mm/include/linux/ktime.h	2006-06-01 23:00:32.000000000 -0400
@@ -148,7 +148,8 @@ static inline ktime_t ktime_sub(const kt
 {
 	ktime_t res;
 
-	res.tv64 = lhs.tv64 - rhs.tv64;
+	res.tv.sec = lhs.tv.sec - rhs.tv.sec;
+	res.tv.nsec = lhs.tv.nsec - rhs.tv.nsec;
 	if (res.tv.nsec < 0)
 		res.tv.nsec += NSEC_PER_SEC;
 
@@ -167,7 +168,8 @@ static inline ktime_t ktime_add(const kt
 {
 	ktime_t res;
 
-	res.tv64 = add1.tv64 + add2.tv64;
+	res.tv.sec = add1.tv.sec + add2.tv.sec;
+	res.tv.nsec = add1.tv.nsec + add2.tv.nsec;
 	/*
 	 * performance trick: the (u32) -NSEC gives 0x00000000Fxxxxxxx
 	 * so we subtract NSEC_PER_SEC and add 1 to the upper 32 bit.
Index: linux-2.6.17-mm/kernel/hrtimer.c
===================================================================
--- linux-2.6.17-mm.orig/kernel/hrtimer.c	2006-06-01 22:15:44.000000000 -0400
+++ linux-2.6.17-mm/kernel/hrtimer.c	2006-06-01 22:57:24.000000000 -0400
@@ -627,7 +627,8 @@ static inline void run_hrtimer_queue(str
 		int restart;
 
 		timer = rb_entry(node, struct hrtimer, node);
-		if (base->softirq_time.tv64 <= timer->expires.tv64)
+		if(ktime_to_ns(base->softirq_time) <=
+		   ktime_to_ns(timer->expires))
 			break;
 
 		fn = timer->function;

             reply	other threads:[~2006-06-02  3:08 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-06-02  3:08 Jeff Dike [this message]
2006-06-02  6:54 ` non-scalar ktime addition and subtraction broken Thomas Gleixner
2006-06-02 15:19   ` [uml-devel] " Jeff Dike
2006-06-02 18:28     ` [uml-devel] " Blaisorblade
2006-06-02 21:34       ` Jeff Dike
2006-06-04 15:04         ` Blaisorblade

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=20060602030825.GA8006@ccure.user-mode-linux.org \
    --to=jdike@addtoit.com \
    --cc=caker@theshore.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=user-mode-linux-devel@lists.sourceforge.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox