We have a bugreport which blames div64_u64() on 32bit platforms.
However, the code obviously doesn't even try to pretend it can do
the 64bit division precisely. If there is something in the high
word of divisor, div64_u64() just shifts both arguments and throws
out the low bits.
Add a small comment to avoid the confusion.
Signed-off-by: Oleg Nesterov <[email protected]>
---
lib/div64.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/lib/div64.c
+++ b/lib/div64.c
@@ -77,7 +77,10 @@ s64 div_s64_rem(s64 dividend, s32 diviso
EXPORT_SYMBOL(div_s64_rem);
#endif
-/* 64bit divisor, dividend and result. dynamic precision */
+/*
+ * 64bit divisor, dividend and result. Dynamic precision, unless
+ * divisor fits in u32 result is not exactly correct.
+ */
#ifndef div64_u64
u64 div64_u64(u64 dividend, u64 divisor)
{
On Mon, 2 Aug 2010 18:09:51 +0200
Oleg Nesterov <[email protected]> wrote:
> We have a bugreport which blames div64_u64() on 32bit platforms.
>
> However, the code obviously doesn't even try to pretend it can do
> the 64bit division precisely. If there is something in the high
> word of divisor, div64_u64() just shifts both arguments and throws
> out the low bits.
>
> Add a small comment to avoid the confusion.
>
> Signed-off-by: Oleg Nesterov <[email protected]>
> ---
>
> lib/div64.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> --- a/lib/div64.c
> +++ b/lib/div64.c
> @@ -77,7 +77,10 @@ s64 div_s64_rem(s64 dividend, s32 diviso
> EXPORT_SYMBOL(div_s64_rem);
> #endif
>
> -/* 64bit divisor, dividend and result. dynamic precision */
> +/*
> + * 64bit divisor, dividend and result. Dynamic precision, unless
> + * divisor fits in u32 result is not exactly correct.
> + */
> #ifndef div64_u64
> u64 div64_u64(u64 dividend, u64 divisor)
> {
Well that was a bit lazy of us - I wonder how hard it is to fix.
At present people will test their code on 64-bit only to find out later
that it doesn't work correctly on 32-bit. Bad. Perhaps we should
similarly break the 64-bit version :)
Andrew,
On Tue, 2010-08-03 at 15:28 -0700, Andrew Morton wrote:
> Well that was a bit lazy of us - I wonder how hard it is to fix.
>
> At present people will test their code on 64-bit only to find out
> later
> that it doesn't work correctly on 32-bit. Bad. Perhaps we should
> similarly break the 64-bit version :)
I just talked to Brian. He's got an implementation that does the
division correctly. He's going to clean it up and send you a patch so
that it will work without losing precision on both 32b and 64b. It is
based upon public domain code from http://www.hackersdelight.org and
has been proven to be correct in all cases.
-ben
> On Mon, 2 Aug 2010 18:09:51 +0200
>
> Oleg Nesterov <[email protected]> wrote:
> > We have a bugreport which blames div64_u64() on 32bit platforms.
> >
> > However, the code obviously doesn't even try to pretend it can do
> > the 64bit division precisely. If there is something in the high
> > word of divisor, div64_u64() just shifts both arguments and throws
> > out the low bits.
>
> Well that was a bit lazy of us - I wonder how hard it is to fix.
>
> At present people will test their code on 64-bit only to find out later
> that it doesn't work correctly on 32-bit. Bad. Perhaps we should
> similarly break the 64-bit version :)
Here's an even crazier idea, let's just fix the 32-bit version. :)
The attached patch fully implements div64_u64() such that it will return
precisely the right quotient even when the divisor exceeds 32-bits. The
patch also adds a div64_s64() function to fully support signed 64-bit
division.
Because this fix is non-obvious I have also included a unsigned and signed
regression test to verify the correctness of the patch. Using a vanilla
2.6.35 kernel the unsigned regression tests fails on 32-bit platforms. With
the proposed patch applied both the unsigned and signed tests pass.
--
Thanks,
Brian