xfs
[Top] [All Lists]

[PATCH 1/3] xfs: do not take the iolock in xfs_inactive

To: xfs@xxxxxxxxxxx
Subject: [PATCH 1/3] xfs: do not take the iolock in xfs_inactive
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Tue, 25 Aug 2009 14:41:46 -0400
References: <20090825184145.704014101@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: quilt/0.47-1
When xfs_inactive is called we already have the inode torn down and no
one else can have a reference to it.  That means the iolock here is
superflous as all other users require a proper reference to it:

 - xfs_vm_vmap, xfs_vn_fallocate, xfs_read, xfs_write, xfs_splice_read,
   xfs_splice_write and xfs_setattr are all implementations of VFS methods
   that require such a life inode
 - xfs_getbmap and xfs_swap_extents are ioctl sub command for which the
   same is true
 - xfs_truncate_file is only called on quota inodes just return from
   xfs_iget
 - xfs_sync_inode_data does the lock just after an igrab()
 - xfs_filestream_associate and xfs_filestream_new_ag take the iolock
   on the parent inode of an inode which by VFS rules must be referenced

Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Index: xfs/fs/xfs/xfs_vnodeops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_vnodeops.c      2009-08-20 13:51:15.699188904 -0300
+++ xfs/fs/xfs/xfs_vnodeops.c   2009-08-24 11:23:28.798259252 -0300
@@ -869,10 +869,10 @@ xfs_inactive_symlink_rmt(
         * the second transaction.  In the error paths we need it
         * held so the cancel won't rele it, see below.
         */
-       xfs_ilock(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
+       xfs_ilock(ip, XFS_ILOCK_EXCL);
        size = (int)ip->i_d.di_size;
        ip->i_d.di_size = 0;
-       xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
+       xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
        xfs_trans_ihold(tp, ip);
        xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
        /*
@@ -917,7 +917,7 @@ xfs_inactive_symlink_rmt(
         * Mark it dirty so it will be logged and moved forward in the log as
         * part of every commit.
         */
-       xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
+       xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
        xfs_trans_ihold(tp, ip);
        xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
        /*
@@ -977,7 +977,7 @@ xfs_inactive_symlink_rmt(
         * joined to the transaction.
         */
        xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
-       xfs_iunlock(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
+       xfs_iunlock(ip, XFS_ILOCK_EXCL);
        *tpp = NULL;
        return error;
 
@@ -1006,7 +1006,7 @@ xfs_inactive_symlink_local(
                *tpp = NULL;
                return error;
        }
-       xfs_ilock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
+       xfs_ilock(ip, XFS_ILOCK_EXCL);
 
        /*
         * Zero length symlinks _can_ exist.
@@ -1029,7 +1029,6 @@ xfs_inactive_attrs(
        int             error;
        xfs_mount_t     *mp;
 
-       ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
        tp = *tpp;
        mp = ip->i_mount;
        ASSERT(ip->i_d.di_forkoff != 0);
@@ -1051,7 +1050,7 @@ xfs_inactive_attrs(
                goto error_cancel;
 
        xfs_ilock(ip, XFS_ILOCK_EXCL);
-       xfs_trans_ijoin(tp, ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
+       xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
        xfs_trans_ihold(tp, ip);
        xfs_idestroy_fork(ip, XFS_ATTR_FORK);
 
@@ -1065,7 +1064,6 @@ error_cancel:
        xfs_trans_cancel(tp, 0);
 error_unlock:
        *tpp = NULL;
-       xfs_iunlock(ip, XFS_IOLOCK_EXCL);
        return error;
 }
 
@@ -1210,12 +1208,10 @@ xfs_inactive(
                 * will call into the buffer cache and we can't
                 * do that within a transaction.
                 */
-               xfs_ilock(ip, XFS_IOLOCK_EXCL);
 
                error = xfs_itruncate_start(ip, XFS_ITRUNC_DEFINITE, 0);
                if (error) {
                        xfs_trans_cancel(tp, 0);
-                       xfs_iunlock(ip, XFS_IOLOCK_EXCL);
                        return VN_INACTIVE_CACHE;
                }
 
@@ -1227,12 +1223,11 @@ xfs_inactive(
                        /* Don't call itruncate_cleanup */
                        ASSERT(XFS_FORCED_SHUTDOWN(mp));
                        xfs_trans_cancel(tp, 0);
-                       xfs_iunlock(ip, XFS_IOLOCK_EXCL);
                        return VN_INACTIVE_CACHE;
                }
 
                xfs_ilock(ip, XFS_ILOCK_EXCL);
-               xfs_trans_ijoin(tp, ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
+               xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
                xfs_trans_ihold(tp, ip);
 
                /*
@@ -1248,7 +1243,7 @@ xfs_inactive(
                if (error) {
                        xfs_trans_cancel(tp,
                                XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
-                       xfs_iunlock(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
+                       xfs_iunlock(ip, XFS_ILOCK_EXCL);
                        return VN_INACTIVE_CACHE;
                }
        } else if ((ip->i_d.di_mode & S_IFMT) == S_IFLNK) {
@@ -1266,7 +1261,7 @@ xfs_inactive(
                        return VN_INACTIVE_CACHE;
                }
 
-               xfs_trans_ijoin(tp, ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
+               xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
                xfs_trans_ihold(tp, ip);
        } else {
                error = xfs_trans_reserve(tp, 0,
@@ -1279,8 +1274,8 @@ xfs_inactive(
                        return VN_INACTIVE_CACHE;
                }
 
-               xfs_ilock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
-               xfs_trans_ijoin(tp, ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
+               xfs_ilock(ip, XFS_ILOCK_EXCL);
+               xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
                xfs_trans_ihold(tp, ip);
        }
 
@@ -1346,7 +1341,7 @@ xfs_inactive(
         * Release the dquots held by inode, if any.
         */
        xfs_qm_dqdetach(ip);
-       xfs_iunlock(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
+       xfs_iunlock(ip, XFS_ILOCK_EXCL);
 
  out:
        return VN_INACTIVE_CACHE;

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