Ooops. Sorry, didn't notice the second "__". This part will work. But rest
of comments remain.
Best regards,
Anton
>+# if defined(CONFIG_X86) || define(CONFIG_ARCH_S390) ||
>defined(CONFIG_MIPS) || defined(CONFIG_M68K)
>+# define __USE_ASM__
>+# endif
>
>I.e. remove the #ifdef __USE_ASM and add CONFIG_M68K dependence.
>
>>+#ifdef __USE_ASM__
>>+#include <asm/div64.h>
>>+#else /* __USE_ASM__ */
>>+static inline int do_div(unsigned long long * n, unsigned long base)
>>+{
>>+ int res = 0;
>>+ unsigned long long t = *n;
>>+ if ( t == (unsigned long)t ){ /* this should handle 64 bit
>>platforms */
>>+ res = ((unsigned long) t) % base;
>>+ t = ((unsigned long) t) / base;
>>+ } else {
>
>This check is silly. It is _way_ more efficient to do:
>
>if (BITS_PER_LONG == 64) {
> res = ((unsigned long)t) % base;
> t = ((unsigned long)t) / base;
>} else {
>
>This actually only compiles one or the other but not both.
>
>>+#ifndef USE_SLOW_64BIT_DIVIDES
>
>This is stupid. If someone knows in advance they are only doing a division
>by 8 or 16 they would not be using do_div in the first place, they would
>be using a shift or even just normal division sign which gcc is supposed
>to optimize to a shift itself (assuming division by constant).
>
>Please remove this. People using do_div() are using it for a reason.
>
>>+ switch (base) {
>>+ case 8:
>>+ res = ((unsigned long) t & 0x7);
>>+ t = t >> 3;
>>+ break;
>>+ case 16:
>>+ res = ((unsigned long) t & 0xf);
>>+ t = t >> 4;
>>+ break;
>>+ default:
>>+ panic("do_div called with 64 bit arg and
>>unsupported base\n", base);
>
>Anyway, why do you randomly pick 8 and 16? You could do any power of two
>via shift by simply doing something along these lines:
>
>if (!(base & (base - 1))) {
> res = t & (base - 1);
> t >>= ffs(base) - 1;
>} else
> panic("blah");
>
>But I still think this is a really stupid thing to do.
>
>>+ }
>>+#else /* USE_SLOW_64BIT_DIVIDES */
>>+ /*
>>+ * Nasty ugly generic C 64 bit divide on 32 bit machine
>>+ * No one seems to want to take credit for this
>>+ */
>>+ unsigned long low, low2, high;
>>+ low = (t) & 0xffffffff;
>>+ high = (t) >> 32;
>
>Look at asm-parisc/div64.h which has an optimized version of this for t
>actually being 32bit.
>
>>+ res = high % base;
>>+ high = high / base;
>>+ low2 = low >> 16;
>>+ low2 += res << 16;
>>+ res = low2 % base;
>>+ low2 = low2 / base;
>>+ low = low & 0xffff;
>>+ low += res << 16;
>>+ res = low % base;
>>+ low = low / base;
>>+ t = low + ((long long)low2 << 16) +
>>+ ((long long) high << 32);
>>+#endif /* USE_SLOW_64BIT_DIVIDES */
>>+ }
>>+ *n = t;
>>+ return res;
>>+}
>>+
>>+
>>+#endif /* __USE_ASM__ */
>>+
>>+#endif
>[snip]
>
>I think the patch is generally a good idea but it has to be done right...
>
>Best regards,
>
>Anton
>
>
>--
> "I've not lost my mind. It's backed up on tape somewhere." - Unknown
>--
>Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
>Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/
>ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/
>
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
-- "I've not lost my mind. It's backed up on tape somewhere." - Unknown-- Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @) Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/ ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/