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: [uml-devel] 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;
-------------------------------------------------------
All the advantages of Linux Managed Hosting--Without the Cost and Risk!
Fully trained technicians. The highest number of Red Hat certifications in
the hosting industry. Fanatical Support. Click to learn more
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=107521&bid=248729&dat=121642
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
WARNING: multiple messages have this Message-ID (diff)
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;
next reply other threads:[~2006-06-02 3:08 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-06-02 3:08 Jeff Dike [this message]
2006-06-02 3:08 ` non-scalar ktime addition and subtraction broken Jeff Dike
2006-06-02 6:54 ` [uml-devel] " Thomas Gleixner
2006-06-02 6:54 ` Thomas Gleixner
2006-06-02 15:19 ` [uml-devel] " Jeff Dike
2006-06-02 15:19 ` [uml-devel] " Jeff Dike
2006-06-02 18:28 ` [uml-devel] " Blaisorblade
2006-06-02 18:28 ` Blaisorblade
2006-06-02 21:34 ` Jeff Dike
2006-06-02 21:34 ` Jeff Dike
2006-06-04 15:04 ` Blaisorblade
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 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.