Re: [PATCH 3/5] xfs: do not require an ioend for new EOF calculation

From: Ben Myers <bpm@sgi.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 3/5] xfs: do not require an ioend for new EOF calculation
Date: Wed, 16 Nov 2011 12:09:12 -0600	[thread overview]
Message-ID: <20111116180912.GF29840@sgi.com> (raw)
In-Reply-To: <20111115201426.862605739@bombadil.infradead.org>

On Tue, Nov 15, 2011 at 03:14:10PM -0500, Christoph Hellwig wrote:
> Replace xfs_ioend_new_eof with a new inline xfs_new_eof helper that
> doesn't require and ioend, and is available also outside of xfs_aops.c.
> 
> Also make the code a bit more clear by using a normal if statement
> instead of a slightly misleading MIN().
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

This looks good to me.  Getting rid of that MIN is much clearer.

Reviewed-by: Ben Myers <bpm@sgi.com>

> ---
>  fs/xfs/xfs_aops.c  |   26 +++++---------------------
>  fs/xfs/xfs_inode.h |   14 ++++++++++++++
>  2 files changed, 19 insertions(+), 21 deletions(-)
> 
> Index: linux-2.6/fs/xfs/xfs_aops.c
> ===================================================================
> --- linux-2.6.orig/fs/xfs/xfs_aops.c	2011-11-08 08:11:44.891887054 +0100
> +++ linux-2.6/fs/xfs/xfs_aops.c	2011-11-08 08:12:31.586400976 +0100
> @@ -99,24 +99,6 @@ xfs_destroy_ioend(
>  }
>  
>  /*
> - * If the end of the current ioend is beyond the current EOF,
> - * return the new EOF value, otherwise zero.
> - */
> -STATIC xfs_fsize_t
> -xfs_ioend_new_eof(
> -	xfs_ioend_t		*ioend)
> -{
> -	xfs_inode_t		*ip = XFS_I(ioend->io_inode);
> -	xfs_fsize_t		isize;
> -	xfs_fsize_t		bsize;
> -
> -	bsize = ioend->io_offset + ioend->io_size;
> -	isize = MAX(ip->i_size, ip->i_new_size);
> -	isize = MIN(isize, bsize);
> -	return isize > ip->i_d.di_size ? isize : 0;
> -}
> -
> -/*
>   * Fast and loose check if this write could update the on-disk inode size.
>   */
>  static inline bool xfs_ioend_is_append(struct xfs_ioend *ioend)
> @@ -140,7 +122,7 @@ xfs_setfilesize(
>  	xfs_fsize_t		isize;
>  
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
> -	isize = xfs_ioend_new_eof(ioend);
> +	isize = xfs_new_eof(ip, ioend->io_offset + ioend->io_size);
>  	if (isize) {
>  		trace_xfs_setfilesize(ip, ioend->io_offset, ioend->io_size);
>  		ip->i_d.di_size = isize;
> @@ -362,6 +344,8 @@ xfs_submit_ioend_bio(
>  	xfs_ioend_t		*ioend,
>  	struct bio		*bio)
>  {
> +	struct xfs_inode	*ip = XFS_I(ioend->io_inode);
> +
>  	atomic_inc(&ioend->io_remaining);
>  	bio->bi_private = ioend;
>  	bio->bi_end_io = xfs_end_bio;
> @@ -370,8 +354,8 @@ xfs_submit_ioend_bio(
>  	 * If the I/O is beyond EOF we mark the inode dirty immediately
>  	 * but don't update the inode size until I/O completion.
>  	 */
> -	if (xfs_ioend_new_eof(ioend))
> -		xfs_mark_inode_dirty(XFS_I(ioend->io_inode));
> +	if (xfs_new_eof(ip, ioend->io_offset + ioend->io_size))
> +		xfs_mark_inode_dirty(ip);
>  
>  	submit_bio(wbc->sync_mode == WB_SYNC_ALL ? WRITE_SYNC : WRITE, bio);
>  }
> Index: linux-2.6/fs/xfs/xfs_inode.h
> ===================================================================
> --- linux-2.6.orig/fs/xfs/xfs_inode.h	2011-11-08 08:02:50.000000000 +0100
> +++ linux-2.6/fs/xfs/xfs_inode.h	2011-11-08 08:13:01.290386996 +0100
> @@ -278,6 +278,20 @@ static inline struct inode *VFS_I(struct
>  }
>  
>  /*
> + * If this I/O goes past the on-disk inode size update it unless it would
> + * be past the current in-core or write in-progress inode size.
> + */
> +static inline xfs_fsize_t
> +xfs_new_eof(struct xfs_inode *ip, xfs_fsize_t new_size)
> +{
> +	xfs_fsize_t i_size = max(ip->i_size, ip->i_new_size);
> +
> +	if (new_size > i_size)
> +		new_size = i_size;
> +	return new_size > ip->i_d.di_size ? new_size : 0;
> +}
> +
> +/*
>   * i_flags helper functions
>   */
>  static inline void
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs