Re: [PATCH 05/10] xfs: do flush inodes from background inode reclaim

[Dave Chinner <david@fromorbit.com>]

On Tue, Mar 27, 2012 at 12:44:05PM -0400, Christoph Hellwig wrote:
> We already flush dirty inodes throug the AIL regularly, there is no reason
> to have second thread compete with it and disturb the I/O pattern.  We still
> do write inodes when doing a synchronous reclaim from the shrinker or during
> unmount for now.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

I think the subject line should say "don't" rather than "do".

....
> -
> -	/*
> -	 * When we have to flush an inode but don't have SYNC_WAIT set, we
> -	 * flush the inode out using a delwri buffer and wait for the next
> -	 * call into reclaim to find it in a clean state instead of waiting for
> -	 * it now. We also don't return errors here - if the error is transient
> -	 * then the next reclaim pass will flush the inode, and if the error
> -	 * is permanent then the next sync reclaim will reclaim the inode and
> -	 * pass on the error.
> -	 */
> -	if (error && error != EAGAIN && !XFS_FORCED_SHUTDOWN(ip->i_mount)) {
> -		xfs_warn(ip->i_mount,
> -			"inode 0x%llx background reclaim flush failed with %d",
> -			(long long)ip->i_ino, error);
> -	}
> -out:
> -	xfs_iflags_clear(ip, XFS_IRECLAIM);
> -	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> -	/*
> -	 * We could return EAGAIN here to make reclaim rescan the inode tree in
> -	 * a short while. However, this just burns CPU time scanning the tree
> -	 * waiting for IO to complete and xfssyncd never goes back to the idle
> -	 * state. Instead, return 0 to let the next scheduled background reclaim
> -	 * attempt to reclaim the inode again.
> -	 */
> -	return 0;

Getting rid of this mess is great. Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

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