xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 05/10] xfs: do flush inodes from background inode reclaim
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 13 Apr 2012 20:14:59 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20120327164645.389070852@xxxxxxxxxxxxxxxxxxxxxx>
References: <20120327164400.967415009@xxxxxxxxxxxxxxxxxxxxxx> <20120327164645.389070852@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxx>

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@xxxxxxxxxx>
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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