xfs
[Top] [All Lists]

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

To: "Dave Chinner" <david@xxxxxxxxxxxxx>
Subject: RE: [PATCH] XFS: Don't flush stale inodes
From: "Alex Elder" <aelder@xxxxxxx>
Date: Fri, 8 Jan 2010 14:59:40 -0600
Cc: <xfs@xxxxxxxxxxx>
In-reply-to: <1262399980-19277-1-git-send-email-david@xxxxxxxxxxxxx>
Thread-index: AcqLV7bI0nnFfSqfTh2fVEXLp9a9CwFTY/kg
Thread-topic: [PATCH] XFS: Don't flush stale inodes
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@xxxxxxxxxxxxx>

Reviewed-by: Alex Elder <aelder@xxxxxxx>

> ---
>  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;
>       }

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