xfs
[Top] [All Lists]

Re: [PATCH 01/11] xfs: remove xfs_itruncate_data

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 01/11] xfs: remove xfs_itruncate_data
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 14 Dec 2011 08:23:30 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20111208155917.662964294@xxxxxxxxxxxxxxxxxxxxxx>
References: <20111208155755.323930705@xxxxxxxxxxxxxxxxxxxxxx> <20111208155917.662964294@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Dec 08, 2011 at 10:57:56AM -0500, Christoph Hellwig wrote:
> This wrapper isn't overly useful, not to say rather confusing.
> 
> Around the call to xfs_itruncate_extents it does:
> 
>  - add tracing
>  - add a few asserts in debug builds
>  - conditionally update the inode size in two places
>  - log the inode
> 
> Both the tracing and the inode logging can be moved to xfs_itruncate_extents
> as they are useful for the attribute fork as well - in fact the attr code
> already does an equivalent xfs_trans_log_inode call just after calling
> xfs_itruncate_extents.  The conditional size updates are a mess, and there
> was no reason to do them in two places anyway, as the first one was
> conditional on the inode having extents - but without extents we
> xfs_itruncate_extents would be a no-op and the placement wouldn't matter
> anyway.  Instead move the size assignments and the asserts that make sense
> to the callers that want it.
> 
> As a side effect of this clean up xfs_setattr_size by introducing variables
> for the old and new inode size, and moving the size updates into a common
> place.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

I can't see anything wrong with this, and it hasn't produced any
failures in my testing, so it looks OK.

Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>

-- 
Dave Chinner
david@xxxxxxxxxxxxx

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