# define do_div(n,base) ({ \
uint32_t __base = (base); \
uint32_t __rem; \
if (likely(((n) >> 32) == 0)) { \
so if we call do_div() on a u32, the compiler emits nasal daemons.
and we do this -- in the antcipatory scheduler:
if (aic->seek_samples) {
aic->seek_mean = aic->seek_total + 128;
do_div(aic->seek_mean, aic->seek_samples);
}
seek_mean is a sector_t so sometimes it's 64-bit on a 32-bit platform.
so we can't avoid calling do_div().
This almost works (the warning is harmless since gcc optimises away the call)
# define do_div(n,base) ({ \
uint32_t __base = (base); \
uint32_t __rem; \
if ((sizeof(n) < 8) || (likely(((n) >> 32) == 0))) { \
__rem = (uint32_t)(n) % __base; \
(n) = (uint32_t)(n) / __base; \
} else \
__rem = __div64_32(&(n), __base); \
__rem; \
})
Better ideas?
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
Matthew Wilcox <[email protected]> wrote:
>
> This almost works (the warning is harmless since gcc optimises away the call)
>
> # define do_div(n,base) ({ \
> uint32_t __base = (base); \
> uint32_t __rem; \
> if ((sizeof(n) < 8) || (likely(((n) >> 32) == 0))) { \
> __rem = (uint32_t)(n) % __base; \
> (n) = (uint32_t)(n) / __base; \
> } else \
> __rem = __div64_32(&(n), __base); \
> __rem; \
> })
Could we just do:
diff -puN include/asm-generic/div64.h~do_div-fix-43 include/asm-generic/div64.h
--- 25/include/asm-generic/div64.h~do_div-fix-43 Fri Jul 11 15:32:33 2003
+++ 25-akpm/include/asm-generic/div64.h Fri Jul 11 15:33:26 2003
@@ -34,7 +34,8 @@
extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor);
-# define do_div(n,base) ({ \
+# define do_div(_n,base) ({ \
+ uint64_t n = _n; \
uint32_t __base = (base); \
uint32_t __rem; \
if (likely(((n) >> 32) == 0)) { \
@@ -42,6 +43,7 @@ extern uint32_t __div64_32(uint64_t *div
(n) = (uint32_t)(n) / __base; \
} else \
__rem = __div64_32(&(n), __base); \
+ _n = n; \
__rem; \
})
_
On Friday July 11, [email protected] wrote:
>
> # define do_div(n,base) ({ \
> uint32_t __base = (base); \
> uint32_t __rem; \
> if (likely(((n) >> 32) == 0)) { \
>
> so if we call do_div() on a u32, the compiler emits nasal daemons.
> and we do this -- in the antcipatory scheduler:
>
> if (aic->seek_samples) {
> aic->seek_mean = aic->seek_total + 128;
> do_div(aic->seek_mean, aic->seek_samples);
> }
>
> seek_mean is a sector_t so sometimes it's 64-bit on a 32-bit platform.
> so we can't avoid calling do_div().
>
> This almost works (the warning is harmless since gcc optimises away the call)
>
> # define do_div(n,base) ({ \
> uint32_t __base = (base); \
> uint32_t __rem; \
> if ((sizeof(n) < 8) || (likely(((n) >> 32) == 0))) { \
> __rem = (uint32_t)(n) % __base; \
> (n) = (uint32_t)(n) / __base; \
> } else \
> __rem = __div64_32(&(n), __base); \
> __rem; \
> })
>
> Better ideas?
sector_div, defined in blkdev.h, is probably what you want.
NeilBrown
--- linux-2.5/drivers/block/as-iosched.c.orig 2003-07-12 10:12:20.000000000 +1000
+++ linux-2.5/drivers/block/as-iosched.c 2003-07-12 10:12:28.000000000 +1000
@@ -837,7 +837,7 @@ static void as_update_iohist(struct as_i
aic->seek_total += 256*seek_dist;
if (aic->seek_samples) {
aic->seek_mean = aic->seek_total + 128;
- do_div(aic->seek_mean, aic->seek_samples);
+ sector_div(aic->seek_mean, aic->seek_samples);
}
aic->seek_samples = (aic->seek_samples>>1)
+ (aic->seek_samples>>2);
On Fri, Jul 11, 2003 at 11:33:59PM +0100, Matthew Wilcox wrote:
> aic->seek_mean = aic->seek_total + 128;
> do_div(aic->seek_mean, aic->seek_samples);
> }
>
> seek_mean is a sector_t so sometimes it's 64-bit on a 32-bit platform.
> so we can't avoid calling do_div().
That's why we have sector_div, never use do_div on a sector_t.
Christoph Hellwig <[email protected]> wrote:
>
> On Fri, Jul 11, 2003 at 11:33:59PM +0100, Matthew Wilcox wrote:
> > aic->seek_mean = aic->seek_total + 128;
> > do_div(aic->seek_mean, aic->seek_samples);
> > }
> >
> > seek_mean is a sector_t so sometimes it's 64-bit on a 32-bit platform.
> > so we can't avoid calling do_div().
>
> That's why we have sector_div, never use do_div on a sector_t.
Thing is, the arith in there can overflow with 32-bit sector_t anyway, so
we need 64-bit quantities regardless of the CONFIG_LBD setting.
On Sun, Jul 13, 2003 at 09:14:35PM +0200, Bernardo Innocenti wrote:
> On Sunday 13 July 2003 19:26, Richard Henderson wrote:
> > On Fri, Jul 11, 2003 at 11:33:59PM +0100, Matthew Wilcox wrote:
> > > Better ideas?
> >
> > if (likely(((n) >> 31 >> 1) == 0)) {
>
> Do we still need to fix this? I've already posted a patch to disallow
> calling do_div() with any divisor that doesn't look like an unsigned
> 64bit interger.
No, I think the combination of sector_div() and your patch makes everything
happy-happy. Thanks!
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
On Sunday 13 July 2003 19:26, Richard Henderson wrote:
> On Fri, Jul 11, 2003 at 11:33:59PM +0100, Matthew Wilcox wrote:
> > Better ideas?
>
> if (likely(((n) >> 31 >> 1) == 0)) {
Do we still need to fix this? I've already posted a patch to disallow
calling do_div() with any divisor that doesn't look like an unsigned
64bit interger.
--
// Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/ http://www.develer.com/
Please don't send Word attachments - http://www.gnu.org/philosophy/no-word-attachments.html
> if (likely(((n) >> 32) == 0)) { \
if (likely((n) <= 0xffffffff)) {
will work with any unsigned type for n.
GCC knows to only look at the high word to make this test.
On Fri, Jul 11, 2003 at 11:33:59PM +0100, Matthew Wilcox wrote:
> Better ideas?
if (likely(((n) >> 31 >> 1) == 0)) {
r~
On Sun, Jul 13, 2003 at 10:26:23AM -0700, Richard Henderson wrote:
> On Fri, Jul 11, 2003 at 11:33:59PM +0100, Matthew Wilcox wrote:
> > Better ideas?
>
> if (likely(((n) >> 31 >> 1) == 0)) {
>
Beware - luckily I don't have to worry about that on ARM (we do our own
thing.) However, with this code:
int foo(unsigned long long n)
{
if (((n) >> 31 >> 1) == 0) {
return 1;
} else {
return 0;
}
}
gcc 3.2.2 on ARM (32-bit) produces some not-very-nice code, consisting of
6 shifts and including placing one register on the stack and completely
ignoring a register which it could freely use instead.
--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
>>>>> "Matthew" == Matthew Wilcox <[email protected]> writes:
Matthew> do_div(aic->seek_mean, aic->seek_samples); }
Matthew> seek_mean is a sector_t so sometimes it's 64-bit on a 32-bit
Matthew> platform. so we can't avoid calling do_div().
Use sector_div() instead! -- it uses a 63/32 bit divide/remainder if
sector_t is 64-bit, and 32/32 if it's 32-bit.
--
Dr Peter Chubb http://www.gelato.unsw.edu.au peterc AT gelato.unsw.edu.au
You are lost in a maze of BitKeeper repositories, all slightly different.