lkml.org 
[lkml]   [2006]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subjectnon-scalar ktime addition and subtraction broken
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;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2006-06-02 05:11    [W:0.818 / U:0.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site