xfs
[Top] [All Lists]

Re: [Patch] xfs_lowbit64 broken on ia32

To: David Chinner <dgc@xxxxxxx>
Subject: Re: [Patch] xfs_lowbit64 broken on ia32
From: Lachlan McIlroy <lachlan@xxxxxxx>
Date: Wed, 05 Dec 2007 16:37:01 +1100
Cc: xfs-oss <xfs@xxxxxxxxxxx>, xfs-dev <xfs-dev@xxxxxxx>
In-reply-to: <20071205032128.GB115527101@sgi.com>
References: <20071203211620.GN115527101@sgi.com> <475608D8.4070402@sgi.com> <20071205032128.GB115527101@sgi.com>
Reply-to: lachlan@xxxxxxx
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.9 (X11/20071031)
David Chinner wrote:
On Wed, Dec 05, 2007 at 01:11:36PM +1100, Lachlan McIlroy wrote:
David Chinner wrote:
The recent change to the internals of xfs_lowbit64 broke it on
ia32 - the code treats the 64bit value as an unsigned long
which is only 32 bits on ia32 and hence throws away the high 32
bits. 64 bit platforms are not affected.

Tested with a userspace implementation comparing the original
code with the new code.

Signed-off-by: Dave Chinner <dgc@xxxxxxx>
---
fs/xfs/xfs_bit.h |    4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Index: 2.6.x-xfs-new/fs/xfs/xfs_bit.h
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_bit.h 2007-11-02 13:44:45.000000000 +1100
+++ 2.6.x-xfs-new/fs/xfs/xfs_bit.h 2007-12-03 14:43:33.169851481 +1100
@@ -68,8 +68,8 @@ static inline int xfs_lowbit32(__uint32_
/* Get low bit set out of 64-bit argument, -1 if none set */
static inline int xfs_lowbit64(__uint64_t v)
{
- unsigned long t = v;
- return (v) ? find_first_bit(&t, 64) : -1;
+ unsigned long long t = v;
Why create a local copy? Why not just pass v into find_first_bit()?

Because I thought that taking the address of a function parameter was a big no-no because the result is undefined (i.e. platform and compiler dependent)?


Really? Didn't occur to me but then I can't say for sure I've ever used the address of an argument. If we need a local copy then why not use __unit64_t instead of unsigned long long?


<Prev in Thread] Current Thread [Next in Thread>