From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
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 <[email protected]>
---
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
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
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 <[email protected]>
---
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
Hi!
> From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
>
> 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
Hi Pavel!
On Wednesday 01 November 2006 17:30, Pavel Machek wrote:
> Hi!
>
> > From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
> >
> > 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
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
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 <[email protected]>
---
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))
From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
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 <[email protected]>
---
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) ? \
On Fri, 17 Nov 2006 20:30:47 +0100
"Paolo 'Blaisorblade' Giarrusso" <[email protected]> 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.
On Friday 17 November 2006 23:00, Andrew Morton wrote:
> On Fri, 17 Nov 2006 20:30:47 +0100
>
> "Paolo 'Blaisorblade' Giarrusso" <[email protected]> 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