new 64 bit math link fail in xfs in linux-next today

new 64 bit math link fail in xfs in linux-next today

[Paul Gortmaker]

Hi Dave,

Not sure if this has been reported yet, but this new failure showed
up in the xtensa link of today's linux-next builds.

http://kisskb.ellerman.id.au/kisskb/buildresult/10938384/

Paul.

Re: new 64 bit math link fail in xfs in linux-next today

[Mark Tinguely]

On 04/16/14 11:09, Paul Gortmaker wrote:
> Hi Dave,
>
> Not sure if this has been reported yet, but this new failure showed
> up in the xtensa link of today's linux-next builds.
>
> http://kisskb.ellerman.id.au/kisskb/buildresult/10938384/
>
> Paul.
>

Try this works on i386. sorry if my mailer wraps the line:

The roundup in commit 68c1fb5d should be a roundup_64()
because it is desired to round a 64 bit type by an integer
and that will result in a 64 bit value. On 32 bit machines
using roundup() in this case will result in the error:

  ERROR: "__divdi3" [fs/xfs/xfs.ko] undefined!

Signed-off-by: Mark Tinguely <tinguely@sgi.com>
---
  fs/xfs/xfs_aops.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/fs/xfs/xfs_aops.c
===================================================================
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1365,7 +1365,7 @@ __xfs_get_blocks(
  		if (offset < i_size_read(inode) &&
  		    offset + mapping_size >= i_size_read(inode)) {
  			/* limit mapping to block that spans EOF */
-			mapping_size = roundup(i_size_read(inode) - offset,
+			mapping_size = roundup_64(i_size_read(inode) - offset,
  					       1 << inode->i_blkbits);
  		}
  		if (mapping_size > LONG_MAX)

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: new 64 bit math link fail in xfs in linux-next today

[Paul Gortmaker]

On 14-04-16 04:59 PM, Mark Tinguely wrote:
> On 04/16/14 11:09, Paul Gortmaker wrote:
>> Hi Dave,
>>
>> Not sure if this has been reported yet, but this new failure showed
>> up in the xtensa link of today's linux-next builds.
>>
>> http://kisskb.ellerman.id.au/kisskb/buildresult/10938384/
>>
>> Paul.
>>
> 
> Try this works on i386. sorry if my mailer wraps the line:

Yep, also works for xtensa too; feel free to add a reported-by
and/or tested-by from me if you want.

Thanks,
Paul.
--

> 
> The roundup in commit 68c1fb5d should be a roundup_64()
> because it is desired to round a 64 bit type by an integer
> and that will result in a 64 bit value. On 32 bit machines
> using roundup() in this case will result in the error:
> 
>   ERROR: "__divdi3" [fs/xfs/xfs.ko] undefined!
> 
> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
> ---
>   fs/xfs/xfs_aops.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: b/fs/xfs/xfs_aops.c
> ===================================================================
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -1365,7 +1365,7 @@ __xfs_get_blocks(
>   		if (offset < i_size_read(inode) &&
>   		    offset + mapping_size >= i_size_read(inode)) {
>   			/* limit mapping to block that spans EOF */
> -			mapping_size = roundup(i_size_read(inode) - offset,
> +			mapping_size = roundup_64(i_size_read(inode) - offset,
>   					       1 << inode->i_blkbits);
>   		}
>   		if (mapping_size > LONG_MAX)
> 

Re: new 64 bit math link fail in xfs in linux-next today

[Dave Chinner]

On Wed, Apr 16, 2014 at 12:09:37PM -0400, Paul Gortmaker wrote:
> Hi Dave,
> 
> Not sure if this has been reported yet, but this new failure showed
> up in the xtensa link of today's linux-next builds.
> 
> http://kisskb.ellerman.id.au/kisskb/buildresult/10938384/

fs/built-in.o: In function `__xfs_get_blocks':
/home/dave/src/build/x86-64/xfsdev/fs/xfs/xfs_aops.c:1368: undefined reference to `__divdi3'

Fmeh. The hunk that throws the error:

@@ -1354,6 +1362,12 @@ __xfs_get_blocks(
                ASSERT(mapping_size > 0);
                if (mapping_size > size)
                        mapping_size = size;
+               if (offset < i_size_read(inode) &&
+                   offset + mapping_size >= i_size_read(inode)) {
+                       /* limit mapping to block that spans EOF */
+                       mapping_size = roundup(i_size_read(inode) - offset,
+                                              1 << inode->i_blkbits);
+               }
                if (mapping_size > LONG_MAX)
                        mapping_size = LONG_MAX;

You'd think a generic function like roundup() would handle that sort
of 64/32 bit type problem, yes? But it doesn't, despite what the
comment implies:

/* The `const' in roundup() prevents gcc-3.3 from calling __divdi3 */
#define roundup(x, y) (                                 \
{                                                       \
        const typeof(y) __y = y;                        \
        (((x) + (__y - 1)) / __y) * __y;                \
}                                                       \
)

It doesn't even have type checking that might warn you of impending
problems...

<sigh>

/me looks around in the XFS code for a bit

... and finds roundup_64() in fs/xfs/xfs_linux.h. That uses do_div()
to avoid this problem. And it's a static inline function, so it has
type checking, too.

And so this hunk fixes the problem:

@@ -1365,7 +1365,7 @@ __xfs_get_blocks(
 		if (offset < i_size_read(inode) &&
 		    offset + mapping_size >= i_size_read(inode)) {
 			/* limit mapping to block that spans EOF */
-                       mapping_size = roundup(i_size_read(inode) - offset,
+                       mapping_size = roundup_64(i_size_read(inode) - offset,
 					       1 << inode->i_blkbits);
 		}
 		if (mapping_size > LONG_MAX)

Can you test it, please, and I'll push out an updated branch
hopefully in time for today's linux-next build....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com