xfs
[Top] [All Lists]

Re: [PATCH] fix dir2 shortform structures on ARM old ABI

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH] fix dir2 shortform structures on ARM old ABI
From: "Josef 'Jeff' Sipek" <jeffpc@xxxxxxxxxxxxxx>
Date: Sat, 15 Mar 2008 00:27:03 -0400
Cc: xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <47DB4F4F.8030407@xxxxxxxxxxx>
References: <47DB4181.7040603@xxxxxxxxxxx> <20080315041722.GA25621@xxxxxxxxxxxxxx> <47DB4F4F.8030407@xxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.16 (2007-06-11)
On Fri, Mar 14, 2008 at 11:23:43PM -0500, Eric Sandeen wrote:
> Josef 'Jeff' Sipek wrote:
> > On Fri, Mar 14, 2008 at 10:24:49PM -0500, Eric Sandeen wrote:
> >> This should fix the longstanding issues with xfs and old ABI
> >> arm boxes, which lead to various asserts and xfs shutdowns,
> >> and for which an (incorrect) patch has been floating around
> >> for years.  (Said patch made ARM internally consistent, but
> >> altered the normal xfs on-disk format such that it looked
> >> corrupted on other architectures):
> >> http://lists.arm.linux.org.uk/lurker/message/20040311.002034.5ecf21a2.html
> > ...
> >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
> >>
> >> ---
> >>
> >> Index: linux-2.6.24/fs/xfs/linux-2.6/xfs_linux.h
> >> ===================================================================
> >> --- linux-2.6.24.orig/fs/xfs/linux-2.6/xfs_linux.h
> >> +++ linux-2.6.24/fs/xfs/linux-2.6/xfs_linux.h
> >> @@ -300,4 +300,11 @@ static inline __uint64_t howmany_64(__ui
> >>    return x;
> >>  }
> >>  
> >> +/* ARM old ABI has some weird alignment/padding */
> >> +#if defined(__arm__) && !defined(__ARM_EABI__)
> >> +#define __arch_pack __attribute__((packed))
> >> +#else
> >> +#define __arch_pack
> >> +#endif
> > 
> > Shouldn't this be unconditional? Just because it ends up being ok on x86
> > doesn't mean that it won't break some time later on...(do we want another
> > bad_features2 incident?)
> 
> I think that packing structures when they don't need to be can actually
> be harmful, efficiency-wise.  I read a nice explanation of this....
> which I can't find now.

Agreed. For in-memory only structures it makes sense to let the compiler do
whatever is the best, but for structures that are on-disk, you really have
no choice, you have to have the same layout in memory - which frequently
means packed. Unless I missed it, the structs you modified are on-disk, and
therefore _must_ be the way the docs say - which happens to be packed.

Josef 'Jeff' Sipek.

-- 
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like
that.
                - Linus Torvalds


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