[PATCH] xfs: remove custom do_div implementations

[PATCH] xfs: remove custom do_div implementations

[Eric Sandeen]

Long ago, all this gunk was added with a lament about problems
with gcc's do_div, and a fun recommendation in the changelog:

 egcs-2.91.66 is the recommended compiler version for building XFS.

All this special stuff was needed to work around an old gcc bug,
apparently, and it's been there ever since.

There should be no need for this anymore, so remove it.

Remove the special 32-bit xfs_do_mod as well; just let the
kernel's do_div() handle all this.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

I'm currently running through xfstests on a 32-bit box with
no issues, so going ahead & sending it out.

Darrick, please queue this for -rc7.  (I KID!)  ;)

diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 592fdf7..044fb0e 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -212,88 +212,6 @@ static inline kgid_t xfs_gid_to_kgid(__uint32_t gid)
 #define xfs_sort(a,n,s,fn)	sort(a,n,s,fn,NULL)
 #define xfs_stack_trace()	dump_stack()
 
-
-/* Move the kernel do_div definition off to one side */
-
-#if defined __i386__
-/* For ia32 we need to pull some tricks to get past various versions
- * of the compiler which do not like us using do_div in the middle
- * of large functions.
- */
-static inline __u32 xfs_do_div(void *a, __u32 b, int n)
-{
-	__u32	mod;
-
-	switch (n) {
-		case 4:
-			mod = *(__u32 *)a % b;
-			*(__u32 *)a = *(__u32 *)a / b;
-			return mod;
-		case 8:
-			{
-			unsigned long __upper, __low, __high, __mod;
-			__u64	c = *(__u64 *)a;
-			__upper = __high = c >> 32;
-			__low = c;
-			if (__high) {
-				__upper = __high % (b);
-				__high = __high / (b);
-			}
-			asm("divl %2":"=a" (__low), "=d" (__mod):"rm" (b), "0" (__low), "1" (__upper));
-			asm("":"=A" (c):"a" (__low),"d" (__high));
-			*(__u64 *)a = c;
-			return __mod;
-			}
-	}
-
-	/* NOTREACHED */
-	return 0;
-}
-
-/* Side effect free 64 bit mod operation */
-static inline __u32 xfs_do_mod(void *a, __u32 b, int n)
-{
-	switch (n) {
-		case 4:
-			return *(__u32 *)a % b;
-		case 8:
-			{
-			unsigned long __upper, __low, __high, __mod;
-			__u64	c = *(__u64 *)a;
-			__upper = __high = c >> 32;
-			__low = c;
-			if (__high) {
-				__upper = __high % (b);
-				__high = __high / (b);
-			}
-			asm("divl %2":"=a" (__low), "=d" (__mod):"rm" (b), "0" (__low), "1" (__upper));
-			asm("":"=A" (c):"a" (__low),"d" (__high));
-			return __mod;
-			}
-	}
-
-	/* NOTREACHED */
-	return 0;
-}
-#else
-static inline __u32 xfs_do_div(void *a, __u32 b, int n)
-{
-	__u32	mod;
-
-	switch (n) {
-		case 4:
-			mod = *(__u32 *)a % b;
-			*(__u32 *)a = *(__u32 *)a / b;
-			return mod;
-		case 8:
-			mod = do_div(*(__u64 *)a, b);
-			return mod;
-	}
-
-	/* NOTREACHED */
-	return 0;
-}
-
 /* Side effect free 64 bit mod operation */
 static inline __u32 xfs_do_mod(void *a, __u32 b, int n)
 {
@@ -310,10 +228,7 @@ static inline __u32 xfs_do_mod(void *a, __u32 b, int n)
 	/* NOTREACHED */
 	return 0;
 }
-#endif
 
-#undef do_div
-#define do_div(a, b)	xfs_do_div(&(a), (b), sizeof(a))
 #define do_mod(a, b)	xfs_do_mod(&(a), (b), sizeof(a))
 
 static inline __uint64_t roundup_64(__uint64_t x, __uint32_t y)

Re: [PATCH] xfs: remove custom do_div implementations

[Brian Foster]

On Fri, Apr 07, 2017 at 04:42:18PM -0500, Eric Sandeen wrote:
> Long ago, all this gunk was added with a lament about problems
> with gcc's do_div, and a fun recommendation in the changelog:
> 
>  egcs-2.91.66 is the recommended compiler version for building XFS.
> 
> All this special stuff was needed to work around an old gcc bug,
> apparently, and it's been there ever since.
> 
> There should be no need for this anymore, so remove it.
> 
> Remove the special 32-bit xfs_do_mod as well; just let the
> kernel's do_div() handle all this.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Looks Ok to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> 
> I'm currently running through xfstests on a 32-bit box with
> no issues, so going ahead & sending it out.
> 
> Darrick, please queue this for -rc7.  (I KID!)  ;)
> 
> diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
> index 592fdf7..044fb0e 100644
> --- a/fs/xfs/xfs_linux.h
> +++ b/fs/xfs/xfs_linux.h
> @@ -212,88 +212,6 @@ static inline kgid_t xfs_gid_to_kgid(__uint32_t gid)
>  #define xfs_sort(a,n,s,fn)	sort(a,n,s,fn,NULL)
>  #define xfs_stack_trace()	dump_stack()
>  
> -
> -/* Move the kernel do_div definition off to one side */
> -
> -#if defined __i386__
> -/* For ia32 we need to pull some tricks to get past various versions
> - * of the compiler which do not like us using do_div in the middle
> - * of large functions.
> - */
> -static inline __u32 xfs_do_div(void *a, __u32 b, int n)
> -{
> -	__u32	mod;
> -
> -	switch (n) {
> -		case 4:
> -			mod = *(__u32 *)a % b;
> -			*(__u32 *)a = *(__u32 *)a / b;
> -			return mod;
> -		case 8:
> -			{
> -			unsigned long __upper, __low, __high, __mod;
> -			__u64	c = *(__u64 *)a;
> -			__upper = __high = c >> 32;
> -			__low = c;
> -			if (__high) {
> -				__upper = __high % (b);
> -				__high = __high / (b);
> -			}
> -			asm("divl %2":"=a" (__low), "=d" (__mod):"rm" (b), "0" (__low), "1" (__upper));
> -			asm("":"=A" (c):"a" (__low),"d" (__high));
> -			*(__u64 *)a = c;
> -			return __mod;
> -			}
> -	}
> -
> -	/* NOTREACHED */
> -	return 0;
> -}
> -
> -/* Side effect free 64 bit mod operation */
> -static inline __u32 xfs_do_mod(void *a, __u32 b, int n)
> -{
> -	switch (n) {
> -		case 4:
> -			return *(__u32 *)a % b;
> -		case 8:
> -			{
> -			unsigned long __upper, __low, __high, __mod;
> -			__u64	c = *(__u64 *)a;
> -			__upper = __high = c >> 32;
> -			__low = c;
> -			if (__high) {
> -				__upper = __high % (b);
> -				__high = __high / (b);
> -			}
> -			asm("divl %2":"=a" (__low), "=d" (__mod):"rm" (b), "0" (__low), "1" (__upper));
> -			asm("":"=A" (c):"a" (__low),"d" (__high));
> -			return __mod;
> -			}
> -	}
> -
> -	/* NOTREACHED */
> -	return 0;
> -}
> -#else
> -static inline __u32 xfs_do_div(void *a, __u32 b, int n)
> -{
> -	__u32	mod;
> -
> -	switch (n) {
> -		case 4:
> -			mod = *(__u32 *)a % b;
> -			*(__u32 *)a = *(__u32 *)a / b;
> -			return mod;
> -		case 8:
> -			mod = do_div(*(__u64 *)a, b);
> -			return mod;
> -	}
> -
> -	/* NOTREACHED */
> -	return 0;
> -}
> -
>  /* Side effect free 64 bit mod operation */
>  static inline __u32 xfs_do_mod(void *a, __u32 b, int n)
>  {
> @@ -310,10 +228,7 @@ static inline __u32 xfs_do_mod(void *a, __u32 b, int n)
>  	/* NOTREACHED */
>  	return 0;
>  }
> -#endif
>  
> -#undef do_div
> -#define do_div(a, b)	xfs_do_div(&(a), (b), sizeof(a))
>  #define do_mod(a, b)	xfs_do_mod(&(a), (b), sizeof(a))
>  
>  static inline __uint64_t roundup_64(__uint64_t x, __uint32_t y)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] xfs: remove custom do_div implementations

[Christoph Hellwig]

On Fri, Apr 07, 2017 at 04:42:18PM -0500, Eric Sandeen wrote:
> Long ago, all this gunk was added with a lament about problems
> with gcc's do_div, and a fun recommendation in the changelog:
> 
>  egcs-2.91.66 is the recommended compiler version for building XFS.

heh..

Looks fine to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>

(if the tests run fine..)

Re: [PATCH] xfs: remove custom do_div implementations

[Eric Sandeen]

On 4/7/17 4:42 PM, Eric Sandeen wrote:
> Long ago, all this gunk was added with a lament about problems
> with gcc's do_div, and a fun recommendation in the changelog:
> 
>  egcs-2.91.66 is the recommended compiler version for building XFS.
> 
> All this special stuff was needed to work around an old gcc bug,
> apparently, and it's been there ever since.
> 
> There should be no need for this anymore, so remove it.
> 
> Remove the special 32-bit xfs_do_mod as well; just let the
> kernel's do_div() handle all this.

Sooo this makes parisc unhappy, but I don't know why:

  CC [M]  fs/xfs/libxfs/xfs_bmap_btree.o
In file included from ./arch/parisc/include/generated/asm/div64.h:1:0,
                 from ./include/linux/kernel.h:147,
                 from ./arch/parisc/include/asm/bug.h:4,
                 from ./include/linux/bug.h:4,
                 from ./include/linux/mmdebug.h:4,
                 from ./include/linux/gfp.h:4,
                 from ./include/linux/slab.h:14,
                 from fs/xfs/kmem.h:21,
                 from fs/xfs/xfs_linux.h:43,
                 from fs/xfs/xfs.h:32,
                 from fs/xfs/xfs_trace.c:18:
fs/xfs/xfs_mount.h: In function 'xfs_daddr_to_agno':
./include/asm-generic/div64.h:207:28: warning: comparison of distinct pointer types lacks a cast
  (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
                            ^
fs/xfs/xfs_mount.h:315:2: note: in expansion of macro 'do_div'
  do_div(ld, mp->m_sb.sb_agblocks);
  ^
fs/xfs/xfs_mount.h: In function 'xfs_daddr_to_agbno':
./include/asm-generic/div64.h:207:28: warning: comparison of distinct pointer types lacks a cast
  (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
                            ^
fs/xfs/xfs_mount.h:323:25: note: in expansion of macro 'do_div'
  return (xfs_agblock_t) do_div(ld, mp->m_sb.sb_agblocks);
                         ^

ld is an xfs_daddr_t in this call.

so passing an xfs_daddr_t (__s64) to do_div triggers the type check
warning above.  On parisc, we typedef things like this:

typedef __s64 xfs_daddr_t;
__extension__ typedef __signed__ long long __s64;

if # make.cross ARCH=parisc fs/xfs/xfs_bmap_util.i
is to be believed.

On x86_64 the typedefs are the same:

typedef __s64 xfs_daddr_t;
__extension__ typedef __signed__ long long __s64;

so I can't figure out why this is triggering...

-Eric