xfs
[Top] [All Lists]

Re: [PATCH 08/11] xfs: remove the i_size field in struct xfs_inode

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 08/11] xfs: remove the i_size field in struct xfs_inode
From: Ben Myers <bpm@xxxxxxx>
Date: Mon, 16 Jan 2012 13:45:29 -0600
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20120116183201.GA16581@xxxxxxx>
References: <20111218200003.557507716@xxxxxxxxxxxxxxxxxxxxxx> <20111218200132.134835340@xxxxxxxxxxxxxxxxxxxxxx> <20120116183201.GA16581@xxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Mon, Jan 16, 2012 at 12:32:01PM -0600, Ben Myers wrote:
> On Sun, Dec 18, 2011 at 03:00:11PM -0500, Christoph Hellwig wrote:
> > There is no fundamental need to keep an in-memory inode size copy in the XFS
> > inode.  We already have the on-disk value in the dinode, and the separate
> > in-memory copy that we need for regular files only in the XFS inode.
> > 
> > Remove the xfs_inode i_size field and change the XFS_ISIZE macro to use the
> > VFS inode i_size field for regular fields.  Switch code that was directly
                                       files.

I'll fix that up.

-Ben

> > accessing the i_size field in the xfs_inode to XFS_ISIZE, or in cases where
> > we are limited to regular files direct access of the VFS inode i_size field.
> > 
> > This also allows dropping some fairly complicated code in the write path
> > which dealt with keeping the xfs_inode i_size uptodate with the VFS i_size
> > that is getting updated inside ->write_end.
> > 
> > Note that we do not bother resetting the VFS i_size when truncating a file
> > that gets freed to zero as there is point in doing so because the VFS inode
> > is no longer in use at this point.  Just relax the assert in xfs_ifree to
> > only check the on-disk size instead.
> > 
> > Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> 
> This looks good to me too.  The only suggestion I had was that in some
> of these places where we call XFS_ISIZE or i_size_read twice in a row,
> it might be nicer to read them into a local variable and use that.
> Dave's comments were very helpful.
> 
> Reviewed-by: Ben Myers <bpm@xxxxxxx>

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