xfs
[Top] [All Lists]

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

To: David Chinner <dgc@xxxxxxx>
Subject: Re: [RFC][PATCH 1/1] XFS: annotate all on-disk structures with __ondisk
From: "Josef 'Jeff' Sipek" <jeffpc@xxxxxxxxxxxxxx>
Date: Tue, 18 Mar 2008 01:28:16 -0400
Cc: Eric Sandeen <sandeen@xxxxxxxxxxx>, xfs@xxxxxxxxxxx, tes@xxxxxxx
In-reply-to: <20080318040903.GU155407@xxxxxxx>
References: <20080317202853.GC16500@xxxxxxxxxxxxxx> <1205800745-9217-1-git-send-email-jeffpc@xxxxxxxxxxxxxx> <47DF3861.6020308@xxxxxxxxxxx> <20080318040903.GU155407@xxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.16 (2007-06-11)
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


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