From a4bfd8ea1b58d8797ec91b18982de243675e63bc Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Mon, 12 Jul 2010 12:38:34 -0700 Subject: [PATCH] Add __divdi3(), remove __udivdi3() kernel dependency Up until now no SPL consumer attempted to perform signed 64-bit division so there was no need to support this. That has now changed so I adding 64-bit division support for 32-bit platforms. The signed implementation is based on the unsigned version. Since the have been several bug reports in the past concerning correct 64-bit division on 32-bit platforms I added some long over due regression tests. Much to my surprise the unsigned 64-bit division regression tests failed. This was surprising because __udivdi3() was implemented by simply calling div64_u64() which is provided by the kernel. This meant that the linux kernels 64-bit division algorithm on 32-bit platforms was flawed. After some investigation this turned out to be exactly the case. Because of this I was forced to abandon the kernel helper and instead to fully implement 64-bit division in the spl. There are several published implementation out there on how to do this properly and I settled on one proposed in the book Hacker's Delight. Their proposed algoritm is freely available without restriction and I have just modified it to be linux kernel friendly. The update implementation now passed all the unsigned and signed regression tests. This should be functional, but not fast, which is good enough for out purposes. If you want fast too I'd strongly suggest you upgrade to a 64-bit platform. I have also reported the kernel bug and we'll see if we can't get it fixed up stream. --- config/spl-build.m4 | 28 ------ configure | 164 ---------------------------------- include/linux/math64_compat.h | 32 +++++++ include/sys/types.h | 1 + module/spl/spl-generic.c | 119 +++++++++++++++++++----- module/splat/splat-generic.c | 133 +++++++++++++++++++++++++++ spl_config.h.in | 6 -- 7 files changed, 262 insertions(+), 221 deletions(-) create mode 100644 include/linux/math64_compat.h diff --git a/config/spl-build.m4 b/config/spl-build.m4 index 7fd403f89..ae4e1f162 100644 --- a/config/spl-build.m4 +++ b/config/spl-build.m4 @@ -53,8 +53,6 @@ AC_DEFUN([SPL_AC_CONFIG_KERNEL], [ SPL_AC_INODE_I_MUTEX SPL_AC_MUTEX_OWNER SPL_AC_MUTEX_LOCK_NESTED - SPL_AC_DIV64_64 - SPL_AC_DIV64_U64 SPL_AC_3ARGS_ON_EACH_CPU SPL_AC_KALLSYMS_LOOKUP_NAME SPL_AC_GET_VMALLOC_INFO @@ -950,32 +948,6 @@ AC_DEFUN([SPL_AC_MUTEX_LOCK_NESTED], [ ]) ]) -dnl # -dnl # 2.6.22 API change, -dnl # First introduced 'div64_64()' in lib/div64.c -dnl -AC_DEFUN([SPL_AC_DIV64_64], [ - SPL_CHECK_SYMBOL_EXPORT( - [div64_64], - [], - [AC_DEFINE(HAVE_DIV64_64, 1, - [div64_64() is available])], - []) -]) - -dnl # -dnl # 2.6.26 API change, -dnl # Renamed 'div64_64()' to 'div64_u64' in lib/div64.c -dnl # -AC_DEFUN([SPL_AC_DIV64_U64], [ - SPL_CHECK_SYMBOL_EXPORT( - [div64_u64], - [], - [AC_DEFINE(HAVE_DIV64_U64, 1, - [div64_u64() is available])], - []) -]) - dnl # dnl # 2.6.27 API change, dnl # on_each_cpu() uses 3 args, no 'retry' argument diff --git a/configure b/configure index 82c086a63..04da5953a 100755 --- a/configure +++ b/configure @@ -13406,88 +13406,6 @@ fi - { $as_echo "$as_me:$LINENO: checking whether symbol div64_64 is exported" >&5 -$as_echo_n "checking whether symbol div64_64 is exported... " >&6; } - grep -q -E '[[:space:]]div64_64[[:space:]]' \ - $LINUX_OBJ/Module*.symvers 2>/dev/null - rc=$? - if test $rc -ne 0; then - export=0 - for file in ; do - grep -q -E "EXPORT_SYMBOL.*(div64_64)" \ - "$LINUX_OBJ/$file" 2>/dev/null - rc=$? - if test $rc -eq 0; then - export=1 - break; - fi - done - if test $export -eq 0; then - { $as_echo "$as_me:$LINENO: result: no" >&5 -$as_echo "no" >&6; } - - else - { $as_echo "$as_me:$LINENO: result: yes" >&5 -$as_echo "yes" >&6; } - -cat >>confdefs.h <<\_ACEOF -#define HAVE_DIV64_64 1 -_ACEOF - - fi - else - { $as_echo "$as_me:$LINENO: result: yes" >&5 -$as_echo "yes" >&6; } - -cat >>confdefs.h <<\_ACEOF -#define HAVE_DIV64_64 1 -_ACEOF - - fi - - - - { $as_echo "$as_me:$LINENO: checking whether symbol div64_u64 is exported" >&5 -$as_echo_n "checking whether symbol div64_u64 is exported... " >&6; } - grep -q -E '[[:space:]]div64_u64[[:space:]]' \ - $LINUX_OBJ/Module*.symvers 2>/dev/null - rc=$? - if test $rc -ne 0; then - export=0 - for file in ; do - grep -q -E "EXPORT_SYMBOL.*(div64_u64)" \ - "$LINUX_OBJ/$file" 2>/dev/null - rc=$? - if test $rc -eq 0; then - export=1 - break; - fi - done - if test $export -eq 0; then - { $as_echo "$as_me:$LINENO: result: no" >&5 -$as_echo "no" >&6; } - - else - { $as_echo "$as_me:$LINENO: result: yes" >&5 -$as_echo "yes" >&6; } - -cat >>confdefs.h <<\_ACEOF -#define HAVE_DIV64_U64 1 -_ACEOF - - fi - else - { $as_echo "$as_me:$LINENO: result: yes" >&5 -$as_echo "yes" >&6; } - -cat >>confdefs.h <<\_ACEOF -#define HAVE_DIV64_U64 1 -_ACEOF - - fi - - - { $as_echo "$as_me:$LINENO: checking whether on_each_cpu() wants 3 args" >&5 $as_echo_n "checking whether on_each_cpu() wants 3 args... " >&6; } @@ -16932,88 +16850,6 @@ fi - { $as_echo "$as_me:$LINENO: checking whether symbol div64_64 is exported" >&5 -$as_echo_n "checking whether symbol div64_64 is exported... " >&6; } - grep -q -E '[[:space:]]div64_64[[:space:]]' \ - $LINUX_OBJ/Module*.symvers 2>/dev/null - rc=$? - if test $rc -ne 0; then - export=0 - for file in ; do - grep -q -E "EXPORT_SYMBOL.*(div64_64)" \ - "$LINUX_OBJ/$file" 2>/dev/null - rc=$? - if test $rc -eq 0; then - export=1 - break; - fi - done - if test $export -eq 0; then - { $as_echo "$as_me:$LINENO: result: no" >&5 -$as_echo "no" >&6; } - - else - { $as_echo "$as_me:$LINENO: result: yes" >&5 -$as_echo "yes" >&6; } - -cat >>confdefs.h <<\_ACEOF -#define HAVE_DIV64_64 1 -_ACEOF - - fi - else - { $as_echo "$as_me:$LINENO: result: yes" >&5 -$as_echo "yes" >&6; } - -cat >>confdefs.h <<\_ACEOF -#define HAVE_DIV64_64 1 -_ACEOF - - fi - - - - { $as_echo "$as_me:$LINENO: checking whether symbol div64_u64 is exported" >&5 -$as_echo_n "checking whether symbol div64_u64 is exported... " >&6; } - grep -q -E '[[:space:]]div64_u64[[:space:]]' \ - $LINUX_OBJ/Module*.symvers 2>/dev/null - rc=$? - if test $rc -ne 0; then - export=0 - for file in ; do - grep -q -E "EXPORT_SYMBOL.*(div64_u64)" \ - "$LINUX_OBJ/$file" 2>/dev/null - rc=$? - if test $rc -eq 0; then - export=1 - break; - fi - done - if test $export -eq 0; then - { $as_echo "$as_me:$LINENO: result: no" >&5 -$as_echo "no" >&6; } - - else - { $as_echo "$as_me:$LINENO: result: yes" >&5 -$as_echo "yes" >&6; } - -cat >>confdefs.h <<\_ACEOF -#define HAVE_DIV64_U64 1 -_ACEOF - - fi - else - { $as_echo "$as_me:$LINENO: result: yes" >&5 -$as_echo "yes" >&6; } - -cat >>confdefs.h <<\_ACEOF -#define HAVE_DIV64_U64 1 -_ACEOF - - fi - - - { $as_echo "$as_me:$LINENO: checking whether on_each_cpu() wants 3 args" >&5 $as_echo_n "checking whether on_each_cpu() wants 3 args... " >&6; } diff --git a/include/linux/math64_compat.h b/include/linux/math64_compat.h new file mode 100644 index 000000000..652082ea2 --- /dev/null +++ b/include/linux/math64_compat.h @@ -0,0 +1,32 @@ +/*****************************************************************************\ + * Copyright (C) 2007-2010 Lawrence Livermore National Security, LLC. + * Copyright (C) 2007 The Regents of the University of California. + * Produced at Lawrence Livermore National Laboratory (cf, DISCLAIMER). + * Written by Brian Behlendorf . + * UCRL-CODE-235197 + * + * This file is part of the SPL, Solaris Porting Layer. + * For details, see . + * + * The SPL is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + * + * The SPL is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + * + * You should have received a copy of the GNU General Public License along + * with the SPL. If not, see . +\*****************************************************************************/ + +#ifndef _SPL_MATH64_COMPAT_H +#define _SPL_MATH64_COMPAT_H + +#ifndef abs64 +#define abs64(x) ({ uint64_t t = (x) >> 63; ((x) ^ t) - t; }) +#endif + +#endif /* _SPL_MATH64_COMPAT_H */ diff --git a/include/sys/types.h b/include/sys/types.h index 256e7b09e..786474baa 100644 --- a/include/sys/types.h +++ b/include/sys/types.h @@ -40,6 +40,7 @@ #include #include #include +#include #ifndef HAVE_UINTPTR_T typedef unsigned long uintptr_t; diff --git a/module/spl/spl-generic.c b/module/spl/spl-generic.c index b9ab7109a..6a3f49ae0 100644 --- a/module/spl/spl-generic.c +++ b/module/spl/spl-generic.c @@ -94,39 +94,111 @@ highbit(unsigned long i) } EXPORT_SYMBOL(highbit); +#if BITS_PER_LONG == 32 /* - * Implementation of 64 bit division for 32-bit machines. + * Support 64/64 => 64 division on a 32-bit platform. While the kernel + * provides a div64_u64() function for this we do not use it because the + * implementation is flawed. There are cases which return incorrect + * results as late as linux-2.6.35. Until this is fixed upstream the + * spl must provide its own implementation. + * + * This implementation is a slightly modified version of the algorithm + * proposed by the book 'Hacker's Delight'. The original source can be + * found here and is available for use without restriction. + * + * http://www.hackersdelight.org/HDcode/newCode/divDouble.c */ -#if BITS_PER_LONG == 32 -uint64_t -__udivdi3(uint64_t dividend, uint64_t divisor) -{ -#if defined(HAVE_DIV64_64) /* 2.6.22 - 2.6.25 API */ - return div64_64(dividend, divisor); -#elif defined(HAVE_DIV64_U64) /* 2.6.26 - 2.6.x API */ - return div64_u64(dividend, divisor); -#else - /* Implementation from 2.6.30 kernel */ - uint32_t high, d; - high = divisor >> 32; - if (high) { - unsigned int shift = fls(high); +/* + * Calculate number of leading of zeros for a 64-bit value. + */ +static int +nlz64(uint64_t x) { + register int n = 0; - d = divisor >> shift; - dividend >>= shift; - } else - d = divisor; + if (x == 0) + return 64; - do_div(dividend, d); + if (x <= 0x00000000FFFFFFFFULL) {n = n + 32; x = x << 32;} + if (x <= 0x0000FFFFFFFFFFFFULL) {n = n + 16; x = x << 16;} + if (x <= 0x00FFFFFFFFFFFFFFULL) {n = n + 8; x = x << 8;} + if (x <= 0x0FFFFFFFFFFFFFFFULL) {n = n + 4; x = x << 4;} + if (x <= 0x3FFFFFFFFFFFFFFFULL) {n = n + 2; x = x << 2;} + if (x <= 0x7FFFFFFFFFFFFFFFULL) {n = n + 1;} + + return n; +} - return dividend; -#endif /* HAVE_DIV64_64, HAVE_DIV64_U64 */ +/* + * Newer kernels have a div_u64() function but we define our own + * to simplify portibility between kernel versions. + */ +static inline uint64_t +__div_u64(uint64_t u, uint32_t v) +{ + (void) do_div(u, v); + return u; +} + +/* + * Implementation of 64-bit unsigned division for 32-bit machines. + * + * First the procedure takes care of the case in which the divisor is a + * 32-bit quantity. There are two subcases: (1) If the left half of the + * dividend is less than the divisor, one execution of do_div() is all that + * is required (overflow is not possible). (2) Otherwise it does two + * divisions, using the grade school method. + */ +uint64_t +__udivdi3(uint64_t u, uint64_t v) +{ + uint64_t u0, u1, v1, q0, q1, k; + int n; + + if (v >> 32 == 0) { // If v < 2**32: + if (u >> 32 < v) { // If u/v cannot overflow, + return __div_u64(u, v); // just do one division. + } else { // If u/v would overflow: + u1 = u >> 32; // Break u into two halves. + u0 = u & 0xFFFFFFFF; + q1 = __div_u64(u1, v); // First quotient digit. + k = u1 - q1 * v; // First remainder, < v. + u0 += (k << 32); + q0 = __div_u64(u0, v); // Seconds quotient digit. + return (q1 << 32) + q0; + } + } else { // If v >= 2**32: + n = nlz64(v); // 0 <= n <= 31. + v1 = (v << n) >> 32; // Normalize divisor, MSB is 1. + u1 = u >> 1; // To ensure no overflow. + q1 = __div_u64(u1, v1); // Get quotient from + q0 = (q1 << n) >> 31; // Undo normalization and + // division of u by 2. + if (q0 != 0) // Make q0 correct or + q0 = q0 - 1; // too small by 1. + if ((u - q0 * v) >= v) + q0 = q0 + 1; // Now q0 is correct. + + return q0; + } } EXPORT_SYMBOL(__udivdi3); /* - * Implementation of 64 bit modulo for 32-bit machines. + * Implementation of 64-bit signed division for 32-bit machines. + */ +int64_t +__divdi3(int64_t u, int64_t v) +{ + int64_t q, t; + q = __udivdi3(abs64(u), abs64(v)); + t = (u ^ v) >> 63; // If u, v have different + return (q ^ t) - t; // signs, negate q. +} +EXPORT_SYMBOL(__divdi3); + +/* + * Implementation of 64-bit unsigned modulo for 32-bit machines. */ uint64_t __umoddi3(uint64_t dividend, uint64_t divisor) @@ -134,6 +206,7 @@ __umoddi3(uint64_t dividend, uint64_t divisor) return (dividend - (divisor * __udivdi3(dividend, divisor))); } EXPORT_SYMBOL(__umoddi3); + #endif /* BITS_PER_LONG */ /* NOTE: The strtoxx behavior is solely based on my reading of the Solaris diff --git a/module/splat/splat-generic.c b/module/splat/splat-generic.c index 8ad6913c0..f9c3c7ec5 100644 --- a/module/splat/splat-generic.c +++ b/module/splat/splat-generic.c @@ -45,6 +45,14 @@ #define SPLAT_GENERIC_TEST4_NAME "ddi_strtoll" #define SPLAT_GENERIC_TEST4_DESC "ddi_strtoll Test" +# define SPLAT_GENERIC_TEST5_ID 0x0d05 +# define SPLAT_GENERIC_TEST5_NAME "udivdi3" +# define SPLAT_GENERIC_TEST5_DESC "Unsigned Div-64 Test" + +# define SPLAT_GENERIC_TEST6_ID 0x0d06 +# define SPLAT_GENERIC_TEST6_NAME "divdi3" +# define SPLAT_GENERIC_TEST6_DESC "Signed Div-64 Test" + #define STR_POS "123456789" #define STR_NEG "-123456789" #define STR_BASE "0xabcdef" @@ -183,6 +191,125 @@ define_splat_generic_test_strtox(l, long); define_splat_generic_test_strtox(ull, unsigned long long); define_splat_generic_test_strtox(ll, long long); +/* + * The entries in the table are used in all combinations and the + * return value is checked to ensure it is range. On 32-bit + * systems __udivdi3 will be invoked for the 64-bit division. + * On 64-bit system the native 64-bit divide will be used so + * __udivdi3 isn't used but we might as well stil run the test. + */ +static int +splat_generic_test_udivdi3(struct file *file, void *arg) +{ + const uint64_t tabu[] = { + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, + 10, 11, 12, 13, 14, 15, 16, 1000, 2003, + 32765, 32766, 32767, 32768, 32769, 32760, + 65533, 65534, 65535, 65536, 65537, 65538, + 0x7ffffffeULL, 0x7fffffffULL, 0x80000000ULL, 0x80000001ULL, + 0x7000000000000000ULL, 0x7000000080000000ULL, 0x7000000080000001ULL, + 0x7fffffffffffffffULL, 0x7fffffff8fffffffULL, 0x7fffffff8ffffff1ULL, + 0x7fffffff00000000ULL, 0x7fffffff80000000ULL, 0x7fffffff00000001ULL, + 0x8000000000000000ULL, 0x8000000080000000ULL, 0x8000000080000001ULL, + 0xc000000000000000ULL, 0xc000000080000000ULL, 0xc000000080000001ULL, + 0xfffffffffffffffdULL, 0xfffffffffffffffeULL, 0xffffffffffffffffULL, + }; + uint64_t uu, vu, qu, ru; + int n, i, j, errors = 0; + + splat_vprint(file, SPLAT_GENERIC_TEST5_NAME, "%s", + "Testing unsigned 64-bit division.\n"); + n = sizeof(tabu) / sizeof(tabu[0]); + for (i = 0; i < n; i++) { + for (j = 1; j < n; j++) { + uu = tabu[i]; + vu = tabu[j]; + qu = uu / vu; /* __udivdi3 */ + ru = uu - qu * vu; + if (qu > uu || ru >= vu) { + splat_vprint(file, SPLAT_GENERIC_TEST5_NAME, + "%016llx/%016llx != %016llx rem %016llx\n", + uu, vu, qu, ru); + errors++; + } + } + } + + if (errors) { + splat_vprint(file, SPLAT_GENERIC_TEST5_NAME, + "Failed %d/%d tests\n", errors, n * (n - 1)); + return -ERANGE; + } + + splat_vprint(file, SPLAT_GENERIC_TEST5_NAME, + "Passed all %d tests\n", n * (n - 1)); + + return 0; +} + +/* + * The entries the table are used in all combinations, with + and - signs + * preceding them. The return value is checked to ensure it is range. + * On 32-bit systems __divdi3 will be invoked for the 64-bit division. + * On 64-bit system the native 64-bit divide will be used so __divdi3 + * isn't used but we might as well stil run the test. + */ +static int +splat_generic_test_divdi3(struct file *file, void *arg) +{ + const int64_t tabs[] = { + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, + 10, 11, 12, 13, 14, 15, 16, 1000, 2003, + 32765, 32766, 32767, 32768, 32769, 32760, + 65533, 65534, 65535, 65536, 65537, 65538, + 0x7ffffffeLL, 0x7fffffffLL, 0x80000000LL, 0x80000001LL, + 0x7000000000000000LL, 0x7000000080000000LL, 0x7000000080000001LL, + 0x7fffffffffffffffLL, 0x7fffffff8fffffffLL, 0x7fffffff8ffffff1LL, + 0x7fffffff00000000LL, 0x7fffffff80000000LL, 0x7fffffff00000001LL, + 0x0123456789abcdefLL, 0x00000000abcdef01LL, 0x0000000012345678LL, +#if BITS_PER_LONG == 32 + 0x8000000000000000LL, 0x8000000080000000LL, 0x8000000080000001LL, +#endif + }; + int64_t u, v, q, r; + int n, i, j, k, errors = 0; + + splat_vprint(file, SPLAT_GENERIC_TEST6_NAME, "%s", + "Testing signed 64-bit division.\n"); + n = sizeof(tabs) / sizeof(tabs[0]); + for (i = 0; i < n; i++) { + for (j = 1; j < n; j++) { + for (k = 0; k <= 3; k++) { + u = (k & 1) ? -tabs[i] : tabs[i]; + v = (k >= 2) ? -tabs[j] : tabs[j]; + + q = u / v; /* __divdi3 */ + r = u - q * v; + if (abs64(q) > abs64(u) || + abs64(r) >= abs64(v) || + (r != 0 && (r ^ u) < 0)) { + splat_vprint(file, + SPLAT_GENERIC_TEST6_NAME, + "%016llx/%016llx != %016llx " + "rem %016llx\n", u, v, q, r); + errors++; + } + } + } + } + + if (errors) { + splat_vprint(file, SPLAT_GENERIC_TEST6_NAME, + "Failed %d/%d tests\n", errors, n * (n - 1)); + return -ERANGE; + } + + splat_vprint(file, SPLAT_GENERIC_TEST6_NAME, + "Passed all %d tests\n", n * (n - 1)); + + return 0; +} + splat_subsystem_t * splat_generic_init(void) { @@ -208,6 +335,10 @@ splat_generic_init(void) SPLAT_GENERIC_TEST3_ID, splat_generic_test_strtoull); SPLAT_TEST_INIT(sub, SPLAT_GENERIC_TEST4_NAME, SPLAT_GENERIC_TEST4_DESC, SPLAT_GENERIC_TEST4_ID, splat_generic_test_strtoll); + SPLAT_TEST_INIT(sub, SPLAT_GENERIC_TEST5_NAME, SPLAT_GENERIC_TEST5_DESC, + SPLAT_GENERIC_TEST5_ID, splat_generic_test_udivdi3); + SPLAT_TEST_INIT(sub, SPLAT_GENERIC_TEST6_NAME, SPLAT_GENERIC_TEST6_DESC, + SPLAT_GENERIC_TEST6_ID, splat_generic_test_divdi3); return sub; } @@ -217,6 +348,8 @@ splat_generic_fini(splat_subsystem_t *sub) { ASSERT(sub); + SPLAT_TEST_FINI(sub, SPLAT_GENERIC_TEST6_ID); + SPLAT_TEST_FINI(sub, SPLAT_GENERIC_TEST5_ID); SPLAT_TEST_FINI(sub, SPLAT_GENERIC_TEST4_ID); SPLAT_TEST_FINI(sub, SPLAT_GENERIC_TEST3_ID); SPLAT_TEST_FINI(sub, SPLAT_GENERIC_TEST2_ID); diff --git a/spl_config.h.in b/spl_config.h.in index 48b610b32..5ed1991f9 100644 --- a/spl_config.h.in +++ b/spl_config.h.in @@ -57,12 +57,6 @@ /* device_create() is available */ #undef HAVE_DEVICE_CREATE -/* div64_64() is available */ -#undef HAVE_DIV64_64 - -/* div64_u64() is available */ -#undef HAVE_DIV64_U64 - /* Define to 1 if you have the header file. */ #undef HAVE_DLFCN_H -- 2.39.5