Re: [RFC][PATCH 1/1] XFS: annotate all on-disk structures with __ondisk

[David Chinner <dgc@sgi.com>]

On Mon, Mar 17, 2008 at 10:34:57PM -0500, Eric Sandeen wrote:
> Josef 'Jeff' Sipek wrote:
> > Currently, the annotation just forces the structures to be packed, and
> > 4-byte aligned.
> 
> Semantic nitpick: in my definition of "annotation" this is more than
> just an annotation.
> 
> An "__ondisk" annotation, to me, would allow something like sparse to
> verify properly laid out on-disk structures, but would not affect the
> actual runtime code - I think that would be quite useful.  However, this
>  change actually impacts the bytecode; it is a functional change.

Yup - this isn't "annotation"....

> So I really do understand what you're trying to do, despite my
> protestations.  If there is some magical instruction to gcc which:
> 
> a) leaves all current "non-broken" ABIs and gcc implementations'
> bytecode untouched (or at the very least, minimally/trivially modified), and
> 
> b) fixes all possible future ABIs so they will always have things
> perfectly and properly aligned, again w/o undue molestation of the
> resulting bytecode
> 
> then I could probably be convinced.  :)  But this seems like a tall
> order, and would require much scrutiny.
> 
> I'm just very shy of a sweeping change like this, which has a material
> impact on the most common architectures, and does not actually provide,
> as far as I can see, any benefit to them - only risk.
> 
> And for now I'll shut up and let the sgi guys chime in eventually.  :)

I think you iterated my concerns quite well, Eric.

The thing I want to see for any sort of change like this is output
off all the structures and their alignment before the change and their alignment
after the change. On all supported arches. 'pahole' is the tool you used
for that, wasn't it, Eric?

The only arch I would expect to see a change in the structures is on ARM; if
there's anything other than that there there's something wrong. This is going
to require a lot of validation to ensure that it is correct.....

Not to mention performance testing on ia64 given the added overhead in
critical paths.....

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group