xfs
[Top] [All Lists]

xfs: fix locking for inode cache radix tree tag updates

To: xfs@xxxxxxxxxxx
Subject: xfs: fix locking for inode cache radix tree tag updates
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Christoph Hellwig <hch@xxxxxx>
Date: Mon, 1 Mar 2010 06:30:31 -0500
Sender: Christoph Hellwig <hch@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.19 (2009-01-05)
The radix-tree code requires it's users to serialize tag updates against
other updates to the tree.  While XFS protects tag updates against each
other it does not serialize them against updates of the tree contents,
which can lead to tag corruption.  Fix the inode cache to always take
pag_ici_lock in exclusive mode when updating radix tree tags.

Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Reported-by: Patrick Schreurs <patrick@xxxxxxxxxxxxxxxx>
Tested-by: Patrick Schreurs <patrick@xxxxxxxxxxxxxxxx>

Index: xfs/fs/xfs/linux-2.6/xfs_sync.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_sync.c        2010-02-10 13:08:41.000000000 
+0100
+++ xfs/fs/xfs/linux-2.6/xfs_sync.c     2010-02-10 15:53:28.739570272 +0100
@@ -687,12 +687,12 @@ xfs_inode_set_reclaim_tag(
        struct xfs_perag *pag;
 
        pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
-       read_lock(&pag->pag_ici_lock);
+       write_lock(&pag->pag_ici_lock);
        spin_lock(&ip->i_flags_lock);
        __xfs_inode_set_reclaim_tag(pag, ip);
        __xfs_iflags_set(ip, XFS_IRECLAIMABLE);
        spin_unlock(&ip->i_flags_lock);
-       read_unlock(&pag->pag_ici_lock);
+       write_unlock(&pag->pag_ici_lock);
        xfs_perag_put(pag);
 }
 
Index: xfs/fs/xfs/xfs_iget.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iget.c  2010-02-04 17:28:35.000000000 +0100
+++ xfs/fs/xfs/xfs_iget.c       2010-02-10 15:53:55.504284758 +0100
@@ -190,13 +190,12 @@ xfs_iget_cache_hit(
                trace_xfs_iget_reclaim(ip);
 
                /*
-                * We need to set XFS_INEW atomically with clearing the
-                * reclaimable tag so that we do have an indicator of the
-                * inode still being initialized.
+                * We need to set XFS_IRECLAIM to prevent xfs_reclaim_inode
+                * from stomping over us while we recycle the inode.  We can't
+                * clear the radix tree reclaimable tag yet as it requires
+                * pag_ici_lock to be helt exclusive.
                 */
-               ip->i_flags |= XFS_INEW;
-               ip->i_flags &= ~XFS_IRECLAIMABLE;
-               __xfs_inode_clear_reclaim_tag(mp, pag, ip);
+               ip->i_flags |= XFS_IRECLAIM;
 
                spin_unlock(&ip->i_flags_lock);
                read_unlock(&pag->pag_ici_lock);
@@ -216,7 +215,15 @@ xfs_iget_cache_hit(
                        trace_xfs_iget_reclaim(ip);
                        goto out_error;
                }
+
+               write_lock(&pag->pag_ici_lock);
+               spin_lock(&ip->i_flags_lock);
+               ip->i_flags &= ~(XFS_IRECLAIMABLE | XFS_IRECLAIM);
+               ip->i_flags |= XFS_INEW;
+               __xfs_inode_clear_reclaim_tag(mp, pag, ip);
                inode->i_state = I_NEW;
+               spin_unlock(&ip->i_flags_lock);
+               write_unlock(&pag->pag_ici_lock);
        } else {
                /* If the VFS inode is being torn down, pause and try again. */
                if (!igrab(inode)) {

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