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

Dave Chinner
2012-04-13T10:15:03Z

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