[PATCH 1/2] Make x86_64 udelay() round up instead of down.

[PATCH 1/2] Make x86_64 udelay() round up instead of down.

[Paolo 'Blaisorblade' Giarrusso]

From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>

Port two patches from i386 to x86_64 delay.c to make sure all rounding is done
upward instead of downward.

There is no sign in commit messages that the mismatch was done on purpose, and
"delay() guarantees sleeping at least for the specified time" is still a valid
rule IMHO.

The original x86 patches are both from pre-GIT era, i.e.:

"[PATCH] round up  in __udelay()" in commit
54c7e1f5cc6771ff644d7bc21a2b829308bd126f

"[PATCH] add 1 in __const_udelay()" in commit
42c77a9801b8877d8b90f65f75db758822a0bccc

(both commits are from converted BK repository to x86_64).

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
---

 arch/x86_64/lib/delay.c    |    4 ++--
 include/asm-x86_64/delay.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86_64/lib/delay.c b/arch/x86_64/lib/delay.c
index 50be909..7514df0 100644
--- a/arch/x86_64/lib/delay.c
+++ b/arch/x86_64/lib/delay.c
@@ -40,13 +40,13 @@ EXPORT_SYMBOL(__delay);
 
 inline void __const_udelay(unsigned long xloops)
 {
-	__delay((xloops * HZ * cpu_data[raw_smp_processor_id()].loops_per_jiffy) >> 32);
+	__delay((xloops * HZ * cpu_data[raw_smp_processor_id()].loops_per_jiffy) >> 32 + 1);
 }
 EXPORT_SYMBOL(__const_udelay);
 
 void __udelay(unsigned long usecs)
 {
-	__const_udelay(usecs * 0x000010c6);  /* 2**32 / 1000000 */
+	__const_udelay(usecs * 0x000010c7);  /* 2**32 / 1000000 (rounded up) */
 }
 EXPORT_SYMBOL(__udelay);
 
diff --git a/include/asm-x86_64/delay.h b/include/asm-x86_64/delay.h
index 65f64ac..40146f6 100644
--- a/include/asm-x86_64/delay.h
+++ b/include/asm-x86_64/delay.h
@@ -16,7 +16,7 @@ extern void __const_udelay(unsigned long
 extern void __delay(unsigned long loops);
 
 #define udelay(n) (__builtin_constant_p(n) ? \
-	((n) > 20000 ? __bad_udelay() : __const_udelay((n) * 0x10c6ul)) : \
+	((n) > 20000 ? __bad_udelay() : __const_udelay((n) * 0x10c7ul)) : \
 	__udelay(n))
 
 #define ndelay(n) (__builtin_constant_p(n) ? \
Chiacchiera con i tuoi amici in tempo reale! 
 http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com 

[PATCH 2/2] i386, x86_64: comment magic constants in delay.h

[Paolo 'Blaisorblade' Giarrusso]

From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>

For both i386 and x86_64, copy from arch/$ARCH/lib/delay.c comments about the
used magic constants, plus a few other niceties.

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
---

 include/asm-i386/delay.h   |    5 ++++-
 include/asm-x86_64/delay.h |    5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/asm-i386/delay.h b/include/asm-i386/delay.h
index b1c7650..9ae5e37 100644
--- a/include/asm-i386/delay.h
+++ b/include/asm-i386/delay.h
@@ -7,6 +7,7 @@ #define _I386_DELAY_H
  * Delay routines calling functions in arch/i386/lib/delay.c
  */
  
+/* Undefined functions to get compile-time errors */
 extern void __bad_udelay(void);
 extern void __bad_ndelay(void);
 
@@ -15,10 +16,12 @@ extern void __ndelay(unsigned long nsecs
 extern void __const_udelay(unsigned long usecs);
 extern void __delay(unsigned long loops);
 
+/* 0x10c7 is 2**32 / 1000000 (rounded up) */
 #define udelay(n) (__builtin_constant_p(n) ? \
 	((n) > 20000 ? __bad_udelay() : __const_udelay((n) * 0x10c7ul)) : \
 	__udelay(n))
-	
+
+/* 0x5 is 2**32 / 1000000000 (rounded up) */
 #define ndelay(n) (__builtin_constant_p(n) ? \
 	((n) > 20000 ? __bad_ndelay() : __const_udelay((n) * 5ul)) : \
 	__ndelay(n))
diff --git a/include/asm-x86_64/delay.h b/include/asm-x86_64/delay.h
index 40146f6..c2669f1 100644
--- a/include/asm-x86_64/delay.h
+++ b/include/asm-x86_64/delay.h
@@ -7,18 +7,21 @@ #define _X8664_DELAY_H
  * Delay routines calling functions in arch/x86_64/lib/delay.c
  */
  
+/* Undefined functions to get compile-time errors */
 extern void __bad_udelay(void);
 extern void __bad_ndelay(void);
 
 extern void __udelay(unsigned long usecs);
-extern void __ndelay(unsigned long usecs);
+extern void __ndelay(unsigned long nsecs);
 extern void __const_udelay(unsigned long usecs);
 extern void __delay(unsigned long loops);
 
+/* 0x10c7 is 2**32 / 1000000 (rounded up) */
 #define udelay(n) (__builtin_constant_p(n) ? \
 	((n) > 20000 ? __bad_udelay() : __const_udelay((n) * 0x10c7ul)) : \
 	__udelay(n))
 
+/* 0x5 is 2**32 / 1000000000 (rounded up) */
 #define ndelay(n) (__builtin_constant_p(n) ? \
        ((n) > 20000 ? __bad_ndelay() : __const_udelay((n) * 5ul)) : \
        __ndelay(n))
Chiacchiera con i tuoi amici in tempo reale! 
 http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com 

Re: [PATCH 1/2] Make x86_64 udelay() round up instead of down.

[Pavel Machek]

Hi!

> From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
> 
> Port two patches from i386 to x86_64 delay.c to make sure all rounding is done
> upward instead of downward.
> 
> There is no sign in commit messages that the mismatch was done on purpose, and
> "delay() guarantees sleeping at least for the specified time" is still a valid
> rule IMHO.

> diff --git a/arch/x86_64/lib/delay.c b/arch/x86_64/lib/delay.c
> index 50be909..7514df0 100644
> --- a/arch/x86_64/lib/delay.c
> +++ b/arch/x86_64/lib/delay.c
> @@ -40,13 +40,13 @@ EXPORT_SYMBOL(__delay);
>  
>  inline void __const_udelay(unsigned long xloops)
>  {
> -	__delay((xloops * HZ * cpu_data[raw_smp_processor_id()].loops_per_jiffy) >> 32);
> +	__delay((xloops * HZ * cpu_data[raw_smp_processor_id()].loops_per_jiffy) >> 32 + 1);

Well, if this should be *rounding* up, you should do 

(xloops * HZ * cpu_data[raw_smp_processor_id()].loops_per_jiffy + 0xffffffff) >> 32

, no? Not sure if it matters...

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/2] Make x86_64 udelay() round up instead of down.

[Blaisorblade]

Hi Pavel!

On Wednesday 01 November 2006 17:30, Pavel Machek wrote:
> Hi!
>
> > From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
> >
> > Port two patches from i386 to x86_64 delay.c to make sure all rounding is
> > done upward instead of downward.
> >
> > There is no sign in commit messages that the mismatch was done on
> > purpose, and "delay() guarantees sleeping at least for the specified
> > time" is still a valid rule IMHO.
> >
> > diff --git a/arch/x86_64/lib/delay.c b/arch/x86_64/lib/delay.c
> > index 50be909..7514df0 100644
> > --- a/arch/x86_64/lib/delay.c
> > +++ b/arch/x86_64/lib/delay.c
> > @@ -40,13 +40,13 @@ EXPORT_SYMBOL(__delay);
> >
> >  inline void __const_udelay(unsigned long xloops)
> >  {
> > -	__delay((xloops * HZ *
> > cpu_data[raw_smp_processor_id()].loops_per_jiffy) >> 32);
> > +	__delay((xloops * HZ *
> > cpu_data[raw_smp_processor_id()].loops_per_jiffy) >> 32 + 1);

Btw, that's read as >> (32 + 1). My fault.
> Well, if this should be *rounding* up, you should do
>
> (xloops * HZ * cpu_data[raw_smp_processor_id()].loops_per_jiffy +
> 0xffffffff) >> 32

Indeed, I did it so following i386 code (where fixing this is awkward - doable 
with carry sums or 64-bit sums), but it's better this way.

I'd slightly prefer (for readability) (xloops * HZ * 
cpu_data[raw_smp_processor_id()].loops_per_jiffy +
(1<<32)-1) >> 32, or even

DIV_ROUND_UP(xloops * HZ * cpu_data[raw_smp_processor_id()].loops_per_jiffy, 1 
<< 32)

but kernel.h macros such as that seem to be underused (everything reimplements 
them as min() and max() before 2.6) and should be made more consistent in 
naming:

#define ALIGN(x,a) (((x)+(a)-1UL)&~((a)-1UL))
#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
#define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))

I hope that gcc optimizes well x / (1<<32) to x >> 32, but without assurance I 
won't use DIV_ROUND_UP in timer code.

 (I also thought there was a bug, so I had a deep look and the difference 
between -1 in roundup and -1UL is correct due to sign extension ruining 
bitmasks, even if it seemed a bad mismatch).
-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade

Chiacchiera con i tuoi amici in tempo reale! 
 http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com 

[PATCH 1/2] Make x86_64 udelay() round up instead of down - try2

[Paolo 'Blaisorblade' Giarrusso]

From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>

Port two patches from i386 to x86_64 delay.c to make sure all rounding is done
upward instead of downward.

There is no sign in commit messages that the mismatch was done on purpose, and
"delay() guarantees sleeping at least for the specified time" is still a valid
rule IMHO.

The original x86 patches are both from pre-GIT era, i.e.:

"[PATCH] round up  in __udelay()" in commit
54c7e1f5cc6771ff644d7bc21a2b829308bd126f

"[PATCH] add 1 in __const_udelay()" in commit
42c77a9801b8877d8b90f65f75db758822a0bccc

(both commits are from the BK repository converted to git).

Changes from try1:
* fixed the code, compile tested against warnings;
* now it is a real round up rather than "round down and add 1"

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
---

 arch/x86_64/lib/delay.c    |    4 ++--
 include/asm-x86_64/delay.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86_64/lib/delay.c b/arch/x86_64/lib/delay.c
index 50be909..b488743 100644
--- a/arch/x86_64/lib/delay.c
+++ b/arch/x86_64/lib/delay.c
@@ -40,13 +40,13 @@ EXPORT_SYMBOL(__delay);
 
 inline void __const_udelay(unsigned long xloops)
 {
-	__delay((xloops * HZ * cpu_data[raw_smp_processor_id()].loops_per_jiffy) >> 32);
+	__delay((xloops * HZ * cpu_data[raw_smp_processor_id()].loops_per_jiffy + (1UL << 32) - 1) >> 32);
 }
 EXPORT_SYMBOL(__const_udelay);
 
 void __udelay(unsigned long usecs)
 {
-	__const_udelay(usecs * 0x000010c6);  /* 2**32 / 1000000 */
+	__const_udelay(usecs * 0x000010c7);  /* 2**32 / 1000000 (rounded up) */
 }
 EXPORT_SYMBOL(__udelay);
 
diff --git a/include/asm-x86_64/delay.h b/include/asm-x86_64/delay.h
index 65f64ac..40146f6 100644
--- a/include/asm-x86_64/delay.h
+++ b/include/asm-x86_64/delay.h
@@ -16,7 +16,7 @@ extern void __const_udelay(unsigned long
 extern void __delay(unsigned long loops);
 
 #define udelay(n) (__builtin_constant_p(n) ? \
-	((n) > 20000 ? __bad_udelay() : __const_udelay((n) * 0x10c6ul)) : \
+	((n) > 20000 ? __bad_udelay() : __const_udelay((n) * 0x10c7ul)) : \
 	__udelay(n))
 
 #define ndelay(n) (__builtin_constant_p(n) ? \

Re: [PATCH 1/2] Make x86_64 udelay() round up instead of down - try2

[Andrew Morton]

On Fri, 17 Nov 2006 20:30:47 +0100
"Paolo 'Blaisorblade' Giarrusso" <blaisorblade@yahoo.it> wrote:

> Port two patches from i386 to x86_64 delay.c to make sure all rounding is done
> upward instead of downward.

Andi already has a patch in his, tree, only it's different.

ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt-current/patches/make-x86_64-udelay-round-up-instead-of-down.

Re: [PATCH 1/2] Make x86_64 udelay() round up instead of down - try2

[Blaisorblade]

On Friday 17 November 2006 23:00, Andrew Morton wrote:
> On Fri, 17 Nov 2006 20:30:47 +0100
>
> "Paolo 'Blaisorblade' Giarrusso" <blaisorblade@yahoo.it> wrote:
> > Port two patches from i386 to x86_64 delay.c to make sure all rounding is
> > done upward instead of downward.
>
> Andi already has a patch in his, tree, only it's different.
>
> ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt-current/patches/make-x86_64-ud
>elay-round-up-instead-of-down.
Ok, a fixed-up version of what I sent - I implemented Pavel's suggestion, the 
the choice is just a taste matter.
-- 
Inform me of my mistakes, so I can add them to my list!
Paolo Giarrusso, aka Blaisorblade
http://www.user-mode-linux.org/~blaisorblade
Chiacchiera con i tuoi amici in tempo reale! 
 http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com 

[PATCH 2/2] i386, x86_64: comment magic constants in delay.h

[Paolo 'Blaisorblade' Giarrusso]

From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>

For both i386 and x86_64, copy from arch/$ARCH/lib/delay.c comments about the
used magic constants, plus a few other niceties.

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@yahoo.it>
---

 include/asm-i386/delay.h   |    5 ++++-
 include/asm-x86_64/delay.h |    5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/asm-i386/delay.h b/include/asm-i386/delay.h
index b1c7650..9ae5e37 100644
--- a/include/asm-i386/delay.h
+++ b/include/asm-i386/delay.h
@@ -7,6 +7,7 @@
  * Delay routines calling functions in arch/i386/lib/delay.c
  */
  
+/* Undefined functions to get compile-time errors */
 extern void __bad_udelay(void);
 extern void __bad_ndelay(void);
 
@@ -15,10 +16,12 @@ extern void __ndelay(unsigned long nsecs
 extern void __const_udelay(unsigned long usecs);
 extern void __delay(unsigned long loops);
 
+/* 0x10c7 is 2**32 / 1000000 (rounded up) */
 #define udelay(n) (__builtin_constant_p(n) ? \
 	((n) > 20000 ? __bad_udelay() : __const_udelay((n) * 0x10c7ul)) : \
 	__udelay(n))
-	
+
+/* 0x5 is 2**32 / 1000000000 (rounded up) */
 #define ndelay(n) (__builtin_constant_p(n) ? \
 	((n) > 20000 ? __bad_ndelay() : __const_udelay((n) * 5ul)) : \
 	__ndelay(n))
diff --git a/include/asm-x86_64/delay.h b/include/asm-x86_64/delay.h
index 40146f6..c2669f1 100644
--- a/include/asm-x86_64/delay.h
+++ b/include/asm-x86_64/delay.h
@@ -7,18 +7,21 @@
  * Delay routines calling functions in arch/x86_64/lib/delay.c
  */
  
+/* Undefined functions to get compile-time errors */
 extern void __bad_udelay(void);
 extern void __bad_ndelay(void);
 
 extern void __udelay(unsigned long usecs);
-extern void __ndelay(unsigned long usecs);
+extern void __ndelay(unsigned long nsecs);
 extern void __const_udelay(unsigned long usecs);
 extern void __delay(unsigned long loops);
 
+/* 0x10c7 is 2**32 / 1000000 (rounded up) */
 #define udelay(n) (__builtin_constant_p(n) ? \
 	((n) > 20000 ? __bad_udelay() : __const_udelay((n) * 0x10c7ul)) : \
 	__udelay(n))
 
+/* 0x5 is 2**32 / 1000000000 (rounded up) */
 #define ndelay(n) (__builtin_constant_p(n) ? \
        ((n) > 20000 ? __bad_ndelay() : __const_udelay((n) * 5ul)) : \
        __ndelay(n))