From: "Josef 'Jeff' Sipek" <jeffpc@josefsipek.net>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: xfs-oss <xfs@oss.sgi.com>
Subject: Re: [PATCH] fix dir2 shortform structures on ARM old ABI
Date: Sat, 15 Mar 2008 00:27:03 -0400 [thread overview]
Message-ID: <20080315042703.GA28242@josefsipek.net> (raw)
In-Reply-To: <47DB4F4F.8030407@sandeen.net>
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@sandeen.net>
> >>
> >> ---
> >>
> >> 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
next prev parent reply other threads:[~2008-03-15 5:08 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-03-15 3:24 [PATCH] fix dir2 shortform structures on ARM old ABI Eric Sandeen
2008-03-15 4:17 ` Josef 'Jeff' Sipek
2008-03-15 4:23 ` Eric Sandeen
2008-03-15 4:27 ` Josef 'Jeff' Sipek [this message]
2008-03-15 4:33 ` Eric Sandeen
2008-03-15 4:51 ` Josef 'Jeff' Sipek
2008-03-17 18:32 ` Eric Sandeen
2008-03-17 19:53 ` Josef 'Jeff' Sipek
2008-03-17 20:04 ` Eric Sandeen
2008-03-17 20:28 ` Josef 'Jeff' Sipek
2008-03-18 0:39 ` [RFC][PATCH 1/1] XFS: annotate all on-disk structures with __ondisk Josef 'Jeff' Sipek
2008-03-18 3:34 ` Eric Sandeen
2008-03-18 4:09 ` David Chinner
2008-03-18 5:28 ` Josef 'Jeff' Sipek
2008-03-17 23:35 ` [PATCH] fix dir2 shortform structures on ARM old ABI Timothy Shimmin
2008-03-17 23:42 ` Josef 'Jeff' Sipek
2008-03-18 4:31 ` Timothy Shimmin
[not found] ` <20080315043622.GA11547@puku.stupidest.org>
2008-03-15 4:45 ` Eric Sandeen
2008-03-20 3:02 ` Eric Sandeen
2008-05-05 7:38 ` Barry Naujok
2008-06-06 14:15 ` Eric Sandeen
2008-04-09 19:53 ` Eric Sandeen
2008-04-23 0:58 ` Eric Sandeen
2008-05-02 20:55 ` Eric Sandeen
2008-05-05 7:08 ` David Chinner
2008-05-05 13:07 ` Eric Sandeen
2008-05-06 4:21 ` Timothy Shimmin
2008-05-06 13:43 ` Eric Sandeen
2008-06-05 5:38 ` Eric Sandeen
2008-06-05 5:46 ` Mark Goodwin
2008-06-05 5:49 ` Eric Sandeen
2008-06-05 6:02 ` Mark Goodwin
2008-06-05 6:04 ` Mark Goodwin
2008-06-05 6:06 ` Eric Sandeen
2008-06-05 5:49 ` Chris Wedgwood
2008-06-05 5:52 ` Eric Sandeen
2008-06-05 6:34 ` Eric Sandeen
-- strict thread matches above, loose matches on Subject: below --
2008-03-18 23:31 Andre Draszik
2008-03-19 3:18 ` Eric Sandeen
2008-03-19 3:40 ` Eric Sandeen
2008-03-19 5:11 ` Eric Sandeen
2008-03-19 5:31 ` Eric Sandeen
2008-03-20 0:35 ` Timothy Shimmin
2008-03-18 23:49 Andre Draszik
2008-03-19 0:23 ` Josef 'Jeff' Sipek
2008-03-19 11:21 ` Luca Olivetti
2008-03-19 12:53 ` Eric Sandeen
2008-03-19 14:11 ` Luca Olivetti
[not found] ` <47E11F4A.3090205@ventoso.org>
[not found] ` <47E12D20.4010901@sandeen.net>
2008-03-19 15:34 ` Luca Olivetti
2008-03-19 15:40 ` Eric Sandeen
[not found] ` <47E14FF1.2020100@sandeen.net>
2008-03-19 18:24 ` Luca Olivetti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080315042703.GA28242@josefsipek.net \
--to=jeffpc@josefsipek.net \
--cc=sandeen@sandeen.net \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox