| Message ID | 20210412033451.215379-1-chenhuacai@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: Remove unused and erroneous div64.h | expand |
On Mon, 12 Apr 2021, Huacai Chen wrote: > 5, As Nikolaus, Yunqiang Su and Yanjie Zhou said, the MIPS-specific > __div64_32() sometimes produces wrong results, which makes 32bit > kernel boot fails. > > I have tried to fix theses errors but I have failed with the last one. I think this is a weak argument for removal, isn't it? Maciej
Hi, > Am 12.04.2021 um 05:34 schrieb Huacai Chen <chenhuacai@kernel.org>: > > There are many errors in div64.h caused by commit c21004cd5b4cb7d479514 > ("MIPS: Rewrite <asm/div64.h> to work with gcc 4.4.0."): > > 1, Only 32bit kernel need __div64_32(), but the above commit makes it > depend on 64bit kernel by mistake. So div64.h is unused in fact. > > 2, asm-generic/div64.h should be included after __div64_32() definition. > > 3, __n should be initialized as *n before use (and "*__n >> 32" should > be "__n >> 32") in __div64_32() definition. > > 4, linux/types.h should be included at the first place, otherwise BITS_ > PER_LONG is not defined. > > 5, As Nikolaus, Yunqiang Su and Yanjie Zhou said, the MIPS-specific s/Nikolaus/Nikolaus Schaller/ > __div64_32() sometimes produces wrong results, which makes 32bit > kernel boot fails. > > I have tried to fix theses errors but I have failed with the last one. > Yunqiang's tests show that the MIPS-specific __div64_32() has no obvious > advantage than the C version, so I just simply remove div64.h. This version works on both, alpha400/jz4730 and ci20/jz4780 (both 32 bit). I have cross-checked the tree for *div64* files and only found as mips specific or lib files: ./arch/mips/include/generated/asm/div64.h -> #include <asm-generic/div64.h> ./include/asm-generic/div64.h ./lib/math/div64.c So there were no remains from earlier compile runs. And since it seems to have no advantage over the C version, I agree that we can remove the special code instead of wasting time to fix it. > > Fixes: c21004cd5b4cb7d479514 ("MIPS: Rewrite <asm/div64.h> to work with gcc 4.4.0.") > Cc: stable@vger.kernel.org Tested-by: H. Nikolaus Schaller <hns@goldelico.com> # CI20/jz4780 and Alpha400/jz4730 BR and thanks, Nikolaus Schaller > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > --- > arch/mips/include/asm/div64.h | 68 ----------------------------------- > 1 file changed, 68 deletions(-) > delete mode 100644 arch/mips/include/asm/div64.h > > diff --git a/arch/mips/include/asm/div64.h b/arch/mips/include/asm/div64.h > deleted file mode 100644 > index dc5ea5736440..000000000000 > --- a/arch/mips/include/asm/div64.h > +++ /dev/null > @@ -1,68 +0,0 @@ > -/* > - * Copyright (C) 2000, 2004 Maciej W. Rozycki > - * Copyright (C) 2003, 07 Ralf Baechle (ralf@linux-mips.org) > - * > - * This file is subject to the terms and conditions of the GNU General Public > - * License. See the file "COPYING" in the main directory of this archive > - * for more details. > - */ > -#ifndef __ASM_DIV64_H > -#define __ASM_DIV64_H > - > -#include <asm-generic/div64.h> > - > -#if BITS_PER_LONG == 64 > - > -#include <linux/types.h> > - > -/* > - * No traps on overflows for any of these... > - */ > - > -#define __div64_32(n, base) \ > -({ \ > - unsigned long __cf, __tmp, __tmp2, __i; \ > - unsigned long __quot32, __mod32; \ > - unsigned long __high, __low; \ > - unsigned long long __n; \ > - \ > - __high = *__n >> 32; \ > - __low = __n; \ > - __asm__( \ > - " .set push \n" \ > - " .set noat \n" \ > - " .set noreorder \n" \ > - " move %2, $0 \n" \ > - " move %3, $0 \n" \ > - " b 1f \n" \ > - " li %4, 0x21 \n" \ > - "0: \n" \ > - " sll $1, %0, 0x1 \n" \ > - " srl %3, %0, 0x1f \n" \ > - " or %0, $1, %5 \n" \ > - " sll %1, %1, 0x1 \n" \ > - " sll %2, %2, 0x1 \n" \ > - "1: \n" \ > - " bnez %3, 2f \n" \ > - " sltu %5, %0, %z6 \n" \ > - " bnez %5, 3f \n" \ > - "2: \n" \ > - " addiu %4, %4, -1 \n" \ > - " subu %0, %0, %z6 \n" \ > - " addiu %2, %2, 1 \n" \ > - "3: \n" \ > - " bnez %4, 0b\n\t" \ > - " srl %5, %1, 0x1f\n\t" \ > - " .set pop" \ > - : "=&r" (__mod32), "=&r" (__tmp), \ > - "=&r" (__quot32), "=&r" (__cf), \ > - "=&r" (__i), "=&r" (__tmp2) \ > - : "Jr" (base), "0" (__high), "1" (__low)); \ > - \ > - (__n) = __quot32; \ > - __mod32; \ > -}) > - > -#endif /* BITS_PER_LONG == 64 */ > - > -#endif /* __ASM_DIV64_H */ > -- > 2.27.0 >
Hi, Maciej, On Mon, Apr 12, 2021 at 10:06 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote: > > On Mon, 12 Apr 2021, Huacai Chen wrote: > > > 5, As Nikolaus, Yunqiang Su and Yanjie Zhou said, the MIPS-specific > > __div64_32() sometimes produces wrong results, which makes 32bit > > kernel boot fails. > > > > I have tried to fix theses errors but I have failed with the last one. > > I think this is a weak argument for removal, isn't it? Yes ,it is weak, but I'm not able to fix it, could you please help me? Huacai > > Maciej
On Thu, 15 Apr 2021, Huacai Chen wrote: > > I think this is a weak argument for removal, isn't it? > Yes ,it is weak, but I'm not able to fix it, could you please help me? First of all you need to assign the quotient to `*n' rather than `__n', which is a temporary only. Otherwise it's discarded, so no surprise the piece does not work. Also this piece assumes the quotient will fit in 32 bits (which should be obvious from the name and data type of the relevant temporary if not the asm itself), which is what the initial division of the high part was for before commit c21004cd5b4c and which the `do_div' wrapper does not arrange for. Said commit is really broken indeed as it mustn't have dropped the initial division and instead it should have only amended the asm for the removal of the `h' constraint (easy fix). Maciej
diff --git a/arch/mips/include/asm/div64.h b/arch/mips/include/asm/div64.h deleted file mode 100644 index dc5ea5736440..000000000000 --- a/arch/mips/include/asm/div64.h +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright (C) 2000, 2004 Maciej W. Rozycki - * Copyright (C) 2003, 07 Ralf Baechle (ralf@linux-mips.org) - * - * This file is subject to the terms and conditions of the GNU General Public - * License. See the file "COPYING" in the main directory of this archive - * for more details. - */ -#ifndef __ASM_DIV64_H -#define __ASM_DIV64_H - -#include <asm-generic/div64.h> - -#if BITS_PER_LONG == 64 - -#include <linux/types.h> - -/* - * No traps on overflows for any of these... - */ - -#define __div64_32(n, base) \ -({ \ - unsigned long __cf, __tmp, __tmp2, __i; \ - unsigned long __quot32, __mod32; \ - unsigned long __high, __low; \ - unsigned long long __n; \ - \ - __high = *__n >> 32; \ - __low = __n; \ - __asm__( \ - " .set push \n" \ - " .set noat \n" \ - " .set noreorder \n" \ - " move %2, $0 \n" \ - " move %3, $0 \n" \ - " b 1f \n" \ - " li %4, 0x21 \n" \ - "0: \n" \ - " sll $1, %0, 0x1 \n" \ - " srl %3, %0, 0x1f \n" \ - " or %0, $1, %5 \n" \ - " sll %1, %1, 0x1 \n" \ - " sll %2, %2, 0x1 \n" \ - "1: \n" \ - " bnez %3, 2f \n" \ - " sltu %5, %0, %z6 \n" \ - " bnez %5, 3f \n" \ - "2: \n" \ - " addiu %4, %4, -1 \n" \ - " subu %0, %0, %z6 \n" \ - " addiu %2, %2, 1 \n" \ - "3: \n" \ - " bnez %4, 0b\n\t" \ - " srl %5, %1, 0x1f\n\t" \ - " .set pop" \ - : "=&r" (__mod32), "=&r" (__tmp), \ - "=&r" (__quot32), "=&r" (__cf), \ - "=&r" (__i), "=&r" (__tmp2) \ - : "Jr" (base), "0" (__high), "1" (__low)); \ - \ - (__n) = __quot32; \ - __mod32; \ -}) - -#endif /* BITS_PER_LONG == 64 */ - -#endif /* __ASM_DIV64_H */
There are many errors in div64.h caused by commit c21004cd5b4cb7d479514 ("MIPS: Rewrite <asm/div64.h> to work with gcc 4.4.0."): 1, Only 32bit kernel need __div64_32(), but the above commit makes it depend on 64bit kernel by mistake. So div64.h is unused in fact. 2, asm-generic/div64.h should be included after __div64_32() definition. 3, __n should be initialized as *n before use (and "*__n >> 32" should be "__n >> 32") in __div64_32() definition. 4, linux/types.h should be included at the first place, otherwise BITS_ PER_LONG is not defined. 5, As Nikolaus, Yunqiang Su and Yanjie Zhou said, the MIPS-specific __div64_32() sometimes produces wrong results, which makes 32bit kernel boot fails. I have tried to fix theses errors but I have failed with the last one. Yunqiang's tests show that the MIPS-specific __div64_32() has no obvious advantage than the C version, so I just simply remove div64.h. Fixes: c21004cd5b4cb7d479514 ("MIPS: Rewrite <asm/div64.h> to work with gcc 4.4.0.") Cc: stable@vger.kernel.org Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> --- arch/mips/include/asm/div64.h | 68 ----------------------------------- 1 file changed, 68 deletions(-) delete mode 100644 arch/mips/include/asm/div64.h