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: Mon, 17 Mar 2008 16:28:53 -0400 [thread overview]
Message-ID: <20080317202853.GC16500@josefsipek.net> (raw)
In-Reply-To: <47DECEBD.10604@sandeen.net>
On Mon, Mar 17, 2008 at 03:04:13PM -0500, Eric Sandeen wrote:
> Josef 'Jeff' Sipek wrote:
...
> Oh, I wasn't trying to blame you or our patch specifically, just wanted
> to highlight what I consider to be the bad idea of giving gcc a bunch of
> directives that IMHO we don't need.
Right. And yes, just plopping __attribute__((packed)) to the end of each
on-disk structure is a really bad idea - it actually make xfsqa fail :)
But living on the edge, without telling gcc what exactly we want from it is
an even worse idea! Take sb_bad_features2...that fiasco that's going to stay
with the XFS on-disk format for a long time to come, would have never
happened if the structures were properly packed/padded to begin with.
> > Second, I need to find out whether all the affected structures are always
> > aligned on some boundary (probably 4 or 8 byte). If there indeed is some
> > alignment, there might be a way to reduce those 15k extra lines to something
> > a whole lot less - I hope.
>
> To what end? What are you trying to fix? If it's not reduced to 0 then
Not packing the structures is all fine if you have one compiler, one OS, and
one architecture to care about. That worked fine when XFS ran on IRIX on
MIPS, but Linux runs on so many different ABIs that you are asking for
trouble by not packing. I can't imagine the number of supported ABIs not
growing. Packing on as-needed basis (which you suggested with your patch) is
rather messy...
1) new ABIs have to checked
2) you'll end up with a rat's nest of #ifdefs to make __arch_pack do the
right thing
Really, we need a way to tell gcc: "hey, gcc, we know what we're doing -
just trust us; don't pad, and don't worry about the alignment". packed gets
you the no-padding, and as I mentioned in my previous reply, align(X) will
hopefully take care of the alignment. Then, gcc should generate nice code to
access members that are on proper boundaries (AFAIK virtually all, if not
all, the struct members in XFS fall into this category), and slightly worse
code for few places that might exist.
The problem you saw in ia64, is because gcc generated the "worst" case code
for all the struct accesses. I _think_ that can be fixed to near-0, if not 0
on all but the few ABIs (e.g., old arm).
Josef 'Jeff' Sipek.
--
The box said "Windows XP or better required". So I installed Linux.
next prev parent reply other threads:[~2008-03-17 23:43 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
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 [this message]
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=20080317202853.GC16500@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