linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* new 64 bit math link fail in xfs in linux-next today
@ 2014-04-16 16:09 Paul Gortmaker
  2014-04-16 20:59 ` Mark Tinguely
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Paul Gortmaker @ 2014-04-16 16:09 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-next@vger.kernel.org, xfs

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.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: new 64 bit math link fail in xfs in linux-next today
  2014-04-16 16:09 new 64 bit math link fail in xfs in linux-next today Paul Gortmaker
@ 2014-04-16 20:59 ` Mark Tinguely
  2014-04-16 21:45   ` Paul Gortmaker
  2014-04-16 21:51 ` [PATCH] xfs: fix 32bit __divdi3 undefined Mark Tinguely
  2014-04-16 21:57 ` new 64 bit math link fail in xfs in linux-next today Dave Chinner
  2 siblings, 1 reply; 8+ messages in thread
From: Mark Tinguely @ 2014-04-16 20:59 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: linux-next@vger.kernel.org, xfs

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: new 64 bit math link fail in xfs in linux-next today
  2014-04-16 20:59 ` Mark Tinguely
@ 2014-04-16 21:45   ` Paul Gortmaker
  0 siblings, 0 replies; 8+ messages in thread
From: Paul Gortmaker @ 2014-04-16 21:45 UTC (permalink / raw)
  To: Mark Tinguely; +Cc: linux-next@vger.kernel.org, xfs

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)
> 

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] xfs: fix 32bit __divdi3 undefined
  2014-04-16 16:09 new 64 bit math link fail in xfs in linux-next today Paul Gortmaker
  2014-04-16 20:59 ` Mark Tinguely
@ 2014-04-16 21:51 ` Mark Tinguely
  2014-04-16 21:57   ` Eric Sandeen
  2014-04-16 22:10   ` Dave Chinner
  2014-04-16 21:57 ` new 64 bit math link fail in xfs in linux-next today Dave Chinner
  2 siblings, 2 replies; 8+ messages in thread
From: Mark Tinguely @ 2014-04-16 21:51 UTC (permalink / raw)
  To: XFS Filesystem; +Cc: Paul Gortmaker

[-- Attachment #1: xfs-fix-32bit-__divdi3-undefined.patch --]
[-- Type: text/plain, Size: 1157 bytes --]

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!

Reported-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Tested-by: Paul Gortmaker <paul.gortmaker@windriver.com>
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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: fix 32bit __divdi3 undefined
  2014-04-16 21:51 ` [PATCH] xfs: fix 32bit __divdi3 undefined Mark Tinguely
@ 2014-04-16 21:57   ` Eric Sandeen
  2014-04-16 22:10   ` Dave Chinner
  1 sibling, 0 replies; 8+ messages in thread
From: Eric Sandeen @ 2014-04-16 21:57 UTC (permalink / raw)
  To: Mark Tinguely, XFS Filesystem; +Cc: Paul Gortmaker

On 4/16/14, 4:51 PM, Mark Tinguely wrote:
> 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!
> 
> Reported-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> Tested-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> Signed-off-by: Mark Tinguely <tinguely@sgi.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.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
> 

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: new 64 bit math link fail in xfs in linux-next today
  2014-04-16 16:09 new 64 bit math link fail in xfs in linux-next today Paul Gortmaker
  2014-04-16 20:59 ` Mark Tinguely
  2014-04-16 21:51 ` [PATCH] xfs: fix 32bit __divdi3 undefined Mark Tinguely
@ 2014-04-16 21:57 ` Dave Chinner
  2 siblings, 0 replies; 8+ messages in thread
From: Dave Chinner @ 2014-04-16 21:57 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: linux-next@vger.kernel.org, xfs

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: fix 32bit __divdi3 undefined
  2014-04-16 21:51 ` [PATCH] xfs: fix 32bit __divdi3 undefined Mark Tinguely
  2014-04-16 21:57   ` Eric Sandeen
@ 2014-04-16 22:10   ` Dave Chinner
  2014-04-16 22:30     ` Mark Tinguely
  1 sibling, 1 reply; 8+ messages in thread
From: Dave Chinner @ 2014-04-16 22:10 UTC (permalink / raw)
  To: Mark Tinguely; +Cc: Paul Gortmaker, XFS Filesystem

On Wed, Apr 16, 2014 at 04:51:56PM -0500, Mark Tinguely wrote:
> 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!
> 
> Reported-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> Tested-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> 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)

I'm not going to apply this as a patch - I'm going to rebase the
branch with the fix in the original patch so we don't have a bisect
breakage in the branch. Don't worry, I'll add the fact you fixed the
bug into the commit message...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] xfs: fix 32bit __divdi3 undefined
  2014-04-16 22:10   ` Dave Chinner
@ 2014-04-16 22:30     ` Mark Tinguely
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Tinguely @ 2014-04-16 22:30 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Paul Gortmaker, XFS Filesystem

On 04/16/14 17:10, Dave Chinner wrote:
> On Wed, Apr 16, 2014 at 04:51:56PM -0500, Mark Tinguely wrote:
>> 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!
>>
>> Reported-by: Paul Gortmaker<paul.gortmaker@windriver.com>
>> Tested-by: Paul Gortmaker<paul.gortmaker@windriver.com>
>> 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)
>
> I'm not going to apply this as a patch - I'm going to rebase the
> branch with the fix in the original patch so we don't have a bisect
> breakage in the branch. Don't worry, I'll add the fact you fixed the
> bug into the commit message...
>
> Cheers,
>
> Dave.

Makes sense.

--Mark.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-04-16 22:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-16 16:09 new 64 bit math link fail in xfs in linux-next today Paul Gortmaker
2014-04-16 20:59 ` Mark Tinguely
2014-04-16 21:45   ` Paul Gortmaker
2014-04-16 21:51 ` [PATCH] xfs: fix 32bit __divdi3 undefined Mark Tinguely
2014-04-16 21:57   ` Eric Sandeen
2014-04-16 22:10   ` Dave Chinner
2014-04-16 22:30     ` Mark Tinguely
2014-04-16 21:57 ` new 64 bit math link fail in xfs in linux-next today Dave Chinner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).