[Top] [All Lists]

Re: [PATCH 2/2 v2] xfs: log all dirty inodes in xfs_fs_sync_fs

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/2 v2] xfs: log all dirty inodes in xfs_fs_sync_fs
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 21 Dec 2011 08:21:06 +1100
Cc: xfs@xxxxxxxxxxx, Paul Anderson <pha@xxxxxxxxx>, Sean Thomas Caron <scaron@xxxxxxxxx>
In-reply-to: <20111220200841.GA2788@xxxxxxxxxxxxx>
References: <20111218154936.GA17626@xxxxxxxxxxxxx> <20111218155015.GC17626@xxxxxxxxxxxxx> <20111220200841.GA2788@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Dec 20, 2011 at 03:08:41PM -0500, Christoph Hellwig wrote:
> Since Linux 2.6.36 the writeback code has introduces various measures for
> live lock prevention during sync().  Unfortunately some of these are
> actively harmful for the XFS model, where the inode gets marked dirty for
> metadata from the data I/O handler.
> The older_than_this checks that are now more strictly enforced since
>     writeback: avoid livelocking WB_SYNC_ALL writeback
> by only calling into __writeback_inodes_sb and thus only sampling the
> current cut off time once.  But on a slow enough devices the previous
> asynchronous sync pass might not have fully completed yet, and thus XFS
> might mark metadata dirty only after that sampling of the cut off time for
> the blocking pass already happened.  I have not myself reproduced this
> myself on a real system, but by introducing artificial delay into the
> XFS I/O completion workqueues it can be reproduced easily.
> Fix this by iterating over all XFS inodes in ->sync_fs and log all that
> are dirty.  This might log inode that only got redirtied after the
> previous pass, but given how cheap delayed logging of inodes is it
> isn't a major concern for performance.
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Looks OK now.

Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>

Dave Chinner

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