[patch] time_after_eq fix

[patch] time_after_eq fix

[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.

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 <coywolf@lovecn.org>
---

 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
--- 2.6.12-rc4-mm2/include/linux/jiffies.h	2005-03-03 17:12:13.000000000 +0800
+++ 2.6.12-rc4-mm2-cy/include/linux/jiffies.h	2005-05-19 05:32:52.000000000 +0800
@@ -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)
 
 /*

Re: [patch] time_after_eq fix

[Chris Friesen]

Coywolf Qi Hunt wrote:
> 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).

> -	 ((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

Re: [patch] time_after_eq fix

[Linus Torvalds]

On Thu, 19 May 2005, Coywolf Qi Hunt wrote:
> 
> 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?

> -	 ((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

Re: [patch] time_after_eq fix

[George Anzinger]

Coywolf Qi Hunt wrote:
> 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
-- 
> 
> 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 <coywolf@lovecn.org>
> ---
> 
>  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
> --- 2.6.12-rc4-mm2/include/linux/jiffies.h	2005-03-03 17:12:13.000000000 +0800
> +++ 2.6.12-rc4-mm2-cy/include/linux/jiffies.h	2005-05-19 05:32:52.000000000 +0800
> @@ -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 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/
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/