From 44e08c45cc14e6190a424be8d450070c8e508fad Mon Sep 17 00:00:00 2001 From: Dave Chinner Date: Sat, 2 Jan 2010 02:39:40 +0000 Subject: xfs: Don't flush stale inodes 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. Signed-off-by: Dave Chinner Signed-off-by: Alex Elder --- fs/xfs/xfs_inode.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'fs/xfs/xfs_inode.c') diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index ce278b3ae7f..391d36b0e68 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -2841,10 +2841,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; } -- cgit v1.2.3-70-g09d2 From 4b6a46882cca8349e8942e2650c33b11bc571c92 Mon Sep 17 00:00:00 2001 From: Dave Chinner Date: Mon, 11 Jan 2010 11:45:21 +0000 Subject: xfs: fix stale inode flush avoidance When reclaiming stale inodes, we need to guarantee that inodes are unpinned before returning with a "clean" status. If we don't we can reclaim inodes that are pinned, leading to use after free in the transaction subsystem as transactions complete. Signed-off-by: Dave Chinner Reviewed-by: Christoph Hellwig Signed-off-by: Alex Elder --- fs/xfs/xfs_inode.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) (limited to 'fs/xfs/xfs_inode.c') diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index 391d36b0e68..ef77fd88c8e 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -2842,13 +2842,9 @@ xfs_iflush( /* * 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. + * do nothing. */ - if (xfs_inode_clean(ip) || xfs_iflags_test(ip, XFS_ISTALE)) { + if (xfs_inode_clean(ip)) { xfs_ifunlock(ip); return 0; } @@ -2871,6 +2867,19 @@ xfs_iflush( } xfs_iunpin_wait(ip); + /* + * For stale inodes 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. We have to check this after ensuring the inode is + * unpinned so that it is safe to reclaim the stale inode after the + * flush call. + */ + if (xfs_iflags_test(ip, XFS_ISTALE)) { + xfs_ifunlock(ip); + return 0; + } + /* * This may have been unpinned because the filesystem is shutting * down forcibly. If that's the case we must not write this inode -- cgit v1.2.3-70-g09d2