[PATCH] MIPS: Remove unused and erroneous div64.h

[PATCH] MIPS: Remove unused and erroneous div64.h

[Huacai Chen]

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

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

Re: [PATCH] MIPS: Remove unused and erroneous div64.h

[Maciej W. Rozycki]

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

Re: [PATCH] MIPS: Remove unused and erroneous div64.h

[Huacai Chen]

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

Re: [PATCH] MIPS: Remove unused and erroneous div64.h

[Maciej W. Rozycki]

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

Re: [PATCH] MIPS: Remove unused and erroneous div64.h

[H. Nikolaus Schaller]

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
>