Skip to contentSkip to conversation

time_after_eq fix

PATCH In review Coywolf Qi Hunt 4 participants 4 messages May 18, 2005 – May 19, 2005

lore
Share & tools
lore

Coywolf Qi Hunt

author
#0 lore raw

PATCH 1/1 time_after_eq fix

Hello,

The two macros time_after and time_after_eq were added to do wrapping correctly, but only time_after does it the right way, time_after_eq has been wrong since the very beginning(v2.1.127, 07-Nov-1998). Now this patch fixes it.

And I don't agree with the the original code comment. I don't think this is gcc's fault. If it is "a good compiler" or "a really good compiler", trying to be smarter than human, it wouldn't still be a C compiler. So I'd like it be removed.

Signed-off-by Coywolf Qi Hunt <[email protected]>

---

 jiffies.h |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
include/linux/jiffies.h +2 -4
diff -pruN 2.6.12-rc4-mm2/include/linux/jiffies.h 2.6.12-rc4-mm2-cy/include/linux/jiffies.h
@@ -102,9 +102,7 @@ static inline u64 get_jiffies_64(void)
  *
  * time_after(a,b) returns true if the time a is after time b.
  *
- * Do this with "<0" and ">=0" to only test the sign of the result. A
- * good compiler would generate better code (and a really good compiler
- * wouldn't care). Gcc is currently neither.
+ * Do this with "<0" and "<=0" to only test the sign of the result.
  */
 #define time_after(a,b)		\
 	(typecheck(unsigned long, a) && \

@@ -115,7 +113,7 @@ static inline u64 get_jiffies_64(void)
 #define time_after_eq(a,b)	\
 	(typecheck(unsigned long, a) && \
 	 typecheck(unsigned long, b) && \
-	 ((long)(a) - (long)(b) >= 0))
+	 ((long)(b) - (long)(a) <= 0))
 #define time_before_eq(a,b)	time_after_eq(b,a)
 
 /*
#1 lore raw

include/linux/jiffies.h static inline u64 get_jiffies_64(void) · line 116

Quoted 8 lines · Coywolf Qi Hunt

Hello,

The two macros time_after and time_after_eq were added to do wrapping correctly, but only time_after does it the right way, time_after_eq has been wrong since the very beginning(v2.1.127, 07-Nov-1998).

include/linux/jiffies.h +1 -1
-	 ((long)(a) - (long)(b) >= 0))
+	 ((long)(b) - (long)(a) <= 0))

Why does it matter which way you do it? In what circumstances does your code give a different answer?

Chris

#2 lore raw
In reply to Coywolf Qi Hunt
Quoted 5 lines · Coywolf Qi Hunt

And I don't agree with the the original code comment. I don't think this is gcc's fault. If it is "a good compiler" or "a really good compiler", trying to be smarter than human, it wouldn't still be a C compiler. So I'd like it be removed.

The original comment is correct, and your changed comment is nonsensical, since "<= 0" doesn't actually test the sign of the result like your comment says.

Also, your patch itself seems not very sensible. Why do you think your patch matters?

include/linux/jiffies.h static inline u64 get_jiffies_64(void) · line 116

Quoted 2 lines
include/linux/jiffies.h +1 -1
-	 ((long)(a) - (long)(b) >= 0))
+	 ((long)(b) - (long)(a) <= 0))

Now, tell me why that one line would make any difference, except for the (undefined) case where we don't know and the time is as far behind as it is ahead?

Notice: you switch the order of the subtraction, and you switch the sign of the test. The original code allowed old gcc versions to generate better code. Your new code doesn't.

What am I missing?

Linus

#3 lore raw
In reply to Coywolf Qi Hunt
Quoted 6 lines · Coywolf Qi Hunt

Hello,

The two macros time_after and time_after_eq were added to do wrapping correctly, but only time_after does it the right way, time_after_eq has been wrong since the very beginning(v2.1.127, 07-Nov-1998). Now this patch fixes it.

I may be especially dense today, but could you give an example where your change actually gives a result different from what exists?

george

Quoted 41 lines

And I don't agree with the the original code comment. I don't think this is gcc's fault. If it is "a good compiler" or "a really good compiler", trying to be smarter than human, it wouldn't still be a C compiler. So I'd like it be removed.

Signed-off-by Coywolf Qi Hunt <[email protected]>

---

 jiffies.h |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff -pruN 2.6.12-rc4-mm2/include/linux/jiffies.h 2.6.12-rc4-mm2-cy/include/linux/jiffies.h

include/linux/jiffies.h +2 -5
@@ -102,9 +102,7 @@ static inline u64 get_jiffies_64(void)
  *
  * time_after(a,b) returns true if the time a is after time b.
  *
- * Do this with "<0" and ">=0" to only test the sign of the result. A
- * good compiler would generate better code (and a really good compiler
- * wouldn't care). Gcc is currently neither.
+ * Do this with "<0" and "<=0" to only test the sign of the result.
  */
 #define time_after(a,b)		\
 	(typecheck(unsigned long, a) && \

@@ -115,7 +113,7 @@ static inline u64 get_jiffies_64(void)
 #define time_after_eq(a,b)	\
 	(typecheck(unsigned long, a) && \
 	 typecheck(unsigned long, b) && \
-	 ((long)(a) - (long)(b) >= 0))
+	 ((long)(b) - (long)(a) <= 0))
 #define time_before_eq(a,b)	time_after_eq(b,a)
 
 /*
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 2 lines
George Anzinger   [email protected]
High-res-timers:  http://sourceforge.net/projects/high-res-timers/