[XFS] remove manual lookup from xfs_rename and simplify locking

->rename already gets the target inode passed if it exits. Pass it down to
xfs_rename so that we can avoid looking it up again. Also simplify locking
as the first lock section in xfs_rename can go away now: the isdir is an
invariant over the lifetime of the inode, and new_parent and the nlink
check are namespace topology protected by i_mutex in the VFS. The projid
check needs to move into the second lock section anyway to not be racy.

Also kill the now unused xfs_dir_lookup_int and remove the now-unused
first_locked argumet to xfs_lock_inodes.

SGI-PV: 976035
SGI-Modid: xfs-linux-melb:xfs-kern:30903a

Signed-off-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Lachlan McIlroy <lachlan@sgi.com>

1 files changed, 45 insertions, 140 deletions

diff --git a/fs/xfs/xfs_rename.c b/fs/xfs/xfs_rename.c
index ee371890d85d..6a141427f68a 100644
--- a/fs/xfs/xfs_rename.c
+++ b/fs/xfs/xfs_rename.c
@@ -55,85 +55,32 @@ xfs_rename_unlock4(
 
         xfs_iunlock(i_tab[0], lock_mode);
         for (i = 1; i < 4; i++) {
-                if (i_tab[i] == NULL) {
+                if (i_tab[i] == NULL)
                         break;
-                }
+
                 /*
                  * Watch out for duplicate entries in the table.
                  */
-                if (i_tab[i] != i_tab[i-1]) {
+                if (i_tab[i] != i_tab[i-1])
                         xfs_iunlock(i_tab[i], lock_mode);
-                }
         }
 }
 
-#ifdef DEBUG
-int xfs_rename_skip, xfs_rename_nskip;
-#endif
-
 /*
- * The following routine will acquire the locks required for a rename
- * operation. The code understands the semantics of renames and will
- * validate that name1 exists under dp1 & that name2 may or may not
- * exist under dp2.
- *
- * We are renaming dp1/name1 to dp2/name2.
- *
- * Return ENOENT if dp1 does not exist, other lookup errors, or 0 for success.
+ * Enter all inodes for a rename transaction into a sorted array.
  */
-STATIC int
-xfs_lock_for_rename(
+STATIC void
+xfs_sort_for_rename(
         xfs_inode_t     *dp1,   /* in: old (source) directory inode */
         xfs_inode_t     *dp2,   /* in: new (target) directory inode */
         xfs_inode_t     *ip1,   /* in: inode of old entry */
-        struct xfs_name *name2, /* in: new entry name */
-        xfs_inode_t     **ipp2, /* out: inode of new entry, if it
+        xfs_inode_t     *ip2,   /* in: inode of new entry, if it
                                    already exists, NULL otherwise. */
         xfs_inode_t     **i_tab,/* out: array of inode returned, sorted */
         int             *num_inodes)  /* out: number of inodes in array */
 {
-        xfs_inode_t             *ip2 = NULL;
         xfs_inode_t             *temp;
-        xfs_ino_t               inum1, inum2;
-        int                     error;
         int                     i, j;
-        uint                    lock_mode;
-        int                     diff_dirs = (dp1 != dp2);
-
-        /*
-         * First, find out the current inums of the entries so that we
-         * can determine the initial locking order.  We'll have to
-         * sanity check stuff after all the locks have been acquired
-         * to see if we still have the right inodes, directories, etc.
-         */
-        lock_mode = xfs_ilock_map_shared(dp1);
-        IHOLD(ip1);
-        xfs_itrace_ref(ip1);
-
-        inum1 = ip1->i_ino;
-
-        /*
-         * Unlock dp1 and lock dp2 if they are different.
-         */
-        if (diff_dirs) {
-                xfs_iunlock_map_shared(dp1, lock_mode);
-                lock_mode = xfs_ilock_map_shared(dp2);
-        }
-
-        error = xfs_dir_lookup_int(dp2, lock_mode, name2, &inum2, &ip2);
-        if (error == ENOENT) {          /* target does not need to exist. */
-                inum2 = 0;
-        } else if (error) {
-                /*
-                 * If dp2 and dp1 are the same, the next line unlocks dp1.
-                 * Got it?
-                 */
-                xfs_iunlock_map_shared(dp2, lock_mode);
-                IRELE (ip1);
-                return error;
-        } else {
-                xfs_itrace_ref(ip2);
-        }
 
         /*
          * i_tab contains a list of pointers to inodes.  We initialize
@@ -145,21 +92,20 @@ xfs_lock_for_rename(
         i_tab[0] = dp1;
         i_tab[1] = dp2;
         i_tab[2] = ip1;
-        if (inum2 == 0) {
-                *num_inodes = 3;
-                i_tab[3] = NULL;
-        } else {
+        if (ip2) {
                 *num_inodes = 4;
                 i_tab[3] = ip2;
+        } else {
+                *num_inodes = 3;
+                i_tab[3] = NULL;
         }
-        *ipp2 = i_tab[3];
 
         /*
          * Sort the elements via bubble sort.  (Remember, there are at
          * most 4 elements to sort, so this is adequate.)
          */
-        for (i=0; i < *num_inodes; i++) {
-                for (j=1; j < *num_inodes; j++) {
+        for (i = 0; i < *num_inodes; i++) {
+                for (j = 1; j < *num_inodes; j++) {
                         if (i_tab[j]->i_ino < i_tab[j-1]->i_ino) {
                                 temp = i_tab[j];
                                 i_tab[j] = i_tab[j-1];
@@ -167,30 +113,6 @@ xfs_lock_for_rename(
                         }
                 }
         }
-
-        /*
-         * We have dp2 locked. If it isn't first, unlock it.
-         * If it is first, tell xfs_lock_inodes so it can skip it
-         * when locking. if dp1 == dp2, xfs_lock_inodes will skip both
-         * since they are equal. xfs_lock_inodes needs all these inodes
-         * so that it can unlock and retry if there might be a dead-lock
-         * potential with the log.
-         */
-
-        if (i_tab[0] == dp2 && lock_mode == XFS_ILOCK_SHARED) {
-#ifdef DEBUG
-                xfs_rename_skip++;
-#endif
-                xfs_lock_inodes(i_tab, *num_inodes, 1, XFS_ILOCK_SHARED);
-        } else {
-#ifdef DEBUG
-                xfs_rename_nskip++;
-#endif
-                xfs_iunlock_map_shared(dp2, lock_mode);
-                xfs_lock_inodes(i_tab, *num_inodes, 0, XFS_ILOCK_SHARED);
-        }
-
-        return 0;
 }
 
 /*
@@ -202,10 +124,10 @@ xfs_rename(
         struct xfs_name *src_name,
         xfs_inode_t     *src_ip,
         xfs_inode_t     *target_dp,
-        struct xfs_name *target_name)
+        struct xfs_name *target_name,
+        xfs_inode_t     *target_ip)
 {
-        xfs_trans_t     *tp;
-        xfs_inode_t     *target_ip;
+        xfs_trans_t     *tp = NULL;
         xfs_mount_t     *mp = src_dp->i_mount;
         int             new_parent;             /* moving to a new dir */
         int             src_is_directory;       /* src_name is a directory */
@@ -230,64 +152,31 @@ xfs_rename(
                                         target_dp, DM_RIGHT_NULL,
                                         src_name->name, target_name->name,
                                         0, 0, 0);
-                if (error) {
+                if (error)
                         return error;
-                }
         }
         /* Return through std_return after this point. */
 
-        /*
-         * Lock all the participating inodes. Depending upon whether
-         * the target_name exists in the target directory, and
-         * whether the target directory is the same as the source
-         * directory, we can lock from 2 to 4 inodes.
-         * xfs_lock_for_rename() will return ENOENT if src_name
-         * does not exist in the source directory.
-         */
-        tp = NULL;
-        error = xfs_lock_for_rename(src_dp, target_dp, src_ip, target_name,
-                                        &target_ip, inodes, &num_inodes);
-        if (error) {
-                /*
-                 * We have nothing locked, no inode references, and
-                 * no transaction, so just get out.
-                 */
-                goto std_return;
-        }
-
-        ASSERT(src_ip != NULL);
+        new_parent = (src_dp != target_dp);
+        src_is_directory = ((src_ip->i_d.di_mode & S_IFMT) == S_IFDIR);
 
-        if ((src_ip->i_d.di_mode & S_IFMT) == S_IFDIR) {
+        if (src_is_directory) {
                 /*
                  * Check for link count overflow on target_dp
                  */
-                if (target_ip == NULL && (src_dp != target_dp) &&
+                if (target_ip == NULL && new_parent &&
                     target_dp->i_d.di_nlink >= XFS_MAXLINK) {
                         error = XFS_ERROR(EMLINK);
-                        xfs_rename_unlock4(inodes, XFS_ILOCK_SHARED);
-                        goto rele_return;
+                        goto std_return;
                 }
         }
 
-        /*
-         * If we are using project inheritance, we only allow renames
-         * into our tree when the project IDs are the same; else the
-         * tree quota mechanism would be circumvented.
-         */
-        if (unlikely((target_dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) &&
-                     (target_dp->i_d.di_projid != src_ip->i_d.di_projid))) {
-                error = XFS_ERROR(EXDEV);
-                xfs_rename_unlock4(inodes, XFS_ILOCK_SHARED);
-                goto rele_return;
-        }
-
-        new_parent = (src_dp != target_dp);
-        src_is_directory = ((src_ip->i_d.di_mode & S_IFMT) == S_IFDIR);
+        xfs_sort_for_rename(src_dp, target_dp, src_ip, target_ip,
+                                inodes, &num_inodes);
 
-        /*
-         * Drop the locks on our inodes so that we can start the transaction.
-         */
-        xfs_rename_unlock4(inodes, XFS_ILOCK_SHARED);
+        IHOLD(src_ip);
+        if (target_ip)
+                IHOLD(target_ip);
 
         XFS_BMAP_INIT(&free_list, &first_block);
         tp = xfs_trans_alloc(mp, XFS_TRANS_RENAME);
@@ -314,9 +203,25 @@ xfs_rename(
         }
 
         /*
-         * Reacquire the inode locks we dropped above.
+         * Lock all the participating inodes. Depending upon whether
+         * the target_name exists in the target directory, and
+         * whether the target directory is the same as the source
+         * directory, we can lock from 2 to 4 inodes.
          */
-        xfs_lock_inodes(inodes, num_inodes, 0, XFS_ILOCK_EXCL);
+        xfs_lock_inodes(inodes, num_inodes, XFS_ILOCK_EXCL);
+
+        /*
+         * If we are using project inheritance, we only allow renames
+         * into our tree when the project IDs are the same; else the
+         * tree quota mechanism would be circumvented.
+         */
+        if (unlikely((target_dp->i_d.di_flags & XFS_DIFLAG_PROJINHERIT) &&
+                     (target_dp->i_d.di_projid != src_ip->i_d.di_projid))) {
+                error = XFS_ERROR(EXDEV);
+                xfs_rename_unlock4(inodes, XFS_ILOCK_SHARED);
+                xfs_trans_cancel(tp, cancel_flags);
+                goto rele_return;
+        }
 
         /*
          * Join all the inodes to the transaction. From this point on,