RE: [PATCH] XFS: Don't flush stale inodes

Alex Elder
2010-01-08T21:01:00Z

Dave Chinner wrote:
> Because inodes remain in cache much longer than inode buffers do
> under memory pressure, we can get the situation where we have stale,
> dirty inodes being reclaimed but the backing storage has been freed.
> Hence we should never, ever flush XFS_ISTALE inodes to disk as
> there is no guarantee that the backing buffer is in cache and
> still marked stale when the flush occurs.

Looks good. Seems like this could be the source of
some pretty gnarly problems.

> Signed-off-by: Dave Chinner <david@fromorbit.com>

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

> ---
> fs/xfs/xfs_inode.c | 10 +++++++---
> 1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c2618db..e5c9953 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2846,10 +2846,14 @@ xfs_iflush(
> mp = ip->i_mount;
>
> /*
> - * If the inode isn't dirty, then just release the inode
> - * flush lock and do nothing.
> + * If the inode isn't dirty, then just release the inode flush lock and
> + * do nothing. Treat stale inodes the same; we cannot rely on the
> + * backing buffer remaining stale in cache for the remaining life of
> + * the stale inode and so xfs_itobp() below may give us a buffer that
> + * no longer contains inodes below. Doing this stale check here also
> + * avoids forcing the log on pinned, stale inodes.
> */
> - if (xfs_inode_clean(ip)) {
> + if (xfs_inode_clean(ip) || xfs_iflags_test(ip, XFS_ISTALE)) {
> xfs_ifunlock(ip);
> return 0;
> }