[PATCH] xfs: don't look forever in xfs_inode_ag_walk during async inode flushes

[PATCH] xfs: don't look forever in xfs_inode_ag_walk during async inode flushes

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

When the underlying inode buffer is locked and xfs_sync_inode_attr()
is doing a non-blocking flush, xfs_iflush() can return EAGAIN. When
this happenѕ, clear the error rather than returning it to
xfs_inode_ag_walk(), as returning EAGAIN will result in the AG walk
delaying for a short while and trying again. This can result in
background walks getting stuck on the one AG until inode buffer is
unlocked by some other means.

This behaviour was noticed when analysing event traces followed by
code inspection and verification of the fix via further traces.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/linux-2.6/xfs_sync.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_sync.c b/fs/xfs/linux-2.6/xfs_sync.c
index c240d46..456bc31 100644
--- a/fs/xfs/linux-2.6/xfs_sync.c
+++ b/fs/xfs/linux-2.6/xfs_sync.c
@@ -267,6 +267,16 @@ xfs_sync_inode_attr(
 
 	error = xfs_iflush(ip, flags);
 
+	/*
+	 * We don't want to try again on non-blocking flushes that can't run
+	 * again immediately. If an inode really must be written, then that's
+	 * what the SYNC_WAIT flag is for.
+	 */
+	if (error == EAGAIN) {
+		ASSERT(!(flags & SYNC_WAIT));
+		error = 0;
+	}
+
  out_unlock:
 	xfs_iunlock(ip, XFS_ILOCK_SHARED);
 	return error;
-- 
1.7.2.3

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

Re: [PATCH] xfs: don't look forever in xfs_inode_ag_walk during async inode flushes

[Christoph Hellwig]

On Thu, Apr 07, 2011 at 04:28:05PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> When the underlying inode buffer is locked and xfs_sync_inode_attr()
> is doing a non-blocking flush, xfs_iflush() can return EAGAIN. When
> this happen??, clear the error rather than returning it to
> xfs_inode_ag_walk(), as returning EAGAIN will result in the AG walk
> delaying for a short while and trying again. This can result in
> background walks getting stuck on the one AG until inode buffer is
> unlocked by some other means.
> 
> This behaviour was noticed when analysing event traces followed by
> code inspection and verification of the fix via further traces.

It's a bit sad that we have two callers now that need to explicitly
ignore the EAGAIN and only one that actually cares, but until we
get rid of VFS-dirty inodes and ->write_inode it looks like we
can't do anything about it.


Reviewed-by: Christoph Hellwig <hch@lst.de>

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

Re: [PATCH] xfs: don't look forever in xfs_inode_ag_walk during async inode flushes

[Alex Elder]

On Thu, 2011-04-07 at 16:28 +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> When the underlying inode buffer is locked and xfs_sync_inode_attr()
> is doing a non-blocking flush, xfs_iflush() can return EAGAIN. When
> this happenѕ, clear the error rather than returning it to
> xfs_inode_ag_walk(), as returning EAGAIN will result in the AG walk
> delaying for a short while and trying again. This can result in
> background walks getting stuck on the one AG until inode buffer is
> unlocked by some other means.
> 
> This behaviour was noticed when analysing event traces followed by
> code inspection and verification of the fix via further traces.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


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