On Tue, Mar 18, 2008 at 03:09:03PM +1100, David Chinner wrote:
> 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"....
Ok. I'll redo the comment for the next version of the patch :)
...
> 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?
Ok, next one will include pahole output. (And yes, pahole is the tool Eric
used.)
> 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.....
Agreed on both accounts.
Josef 'Jeff' Sipek.
--
Intellectuals solve problems; geniuses prevent them
- Albert Einstein
|