xfs: clean up inode locking for RENAME_WHITEOUT

When doing RENAME_WHITEOUT, we now have to lock 5 inodes into the
rename transaction. This means we need to update
xfs_sort_for_rename() and xfs_lock_inodes() to handle up to 5
inodes. Because of the vagaries of rename, this means we could have
anywhere between 3 and 5 inodes locked into the transaction....

While xfs_lock_inodes() does not need anything other than an assert
telling us we are passing more inodes that we ever thought we should
see, it could do with a logic rework to remove all the indenting.
This is not a functional change - it just makes the code a lot
easier to read.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
This commit is contained in:
Dave Chinner 2015-03-25 14:03:32 +11:00 committed by Dave Chinner
parent 444a702231
commit 95afcf5c7b

View File

@ -329,15 +329,14 @@ xfs_lock_inumorder(int lock_mode, int subclass)
} }
/* /*
* The following routine will lock n inodes in exclusive mode. * The following routine will lock n inodes in exclusive mode. We assume the
* We assume the caller calls us with the inodes in i_ino order. * caller calls us with the inodes in i_ino order.
* *
* We need to detect deadlock where an inode that we lock * We need to detect deadlock where an inode that we lock is in the AIL and we
* is in the AIL and we start waiting for another inode that is locked * start waiting for another inode that is locked by a thread in a long running
* by a thread in a long running transaction (such as truncate). This can * transaction (such as truncate). This can result in deadlock since the long
* result in deadlock since the long running trans might need to wait * running trans might need to wait for the inode we just locked in order to
* for the inode we just locked in order to push the tail and free space * push the tail and free space in the log.
* in the log.
*/ */
void void
xfs_lock_inodes( xfs_lock_inodes(
@ -348,30 +347,27 @@ xfs_lock_inodes(
int attempts = 0, i, j, try_lock; int attempts = 0, i, j, try_lock;
xfs_log_item_t *lp; xfs_log_item_t *lp;
ASSERT(ips && (inodes >= 2)); /* we need at least two */ /* currently supports between 2 and 5 inodes */
ASSERT(ips && inodes >= 2 && inodes <= 5);
try_lock = 0; try_lock = 0;
i = 0; i = 0;
again: again:
for (; i < inodes; i++) { for (; i < inodes; i++) {
ASSERT(ips[i]); ASSERT(ips[i]);
if (i && (ips[i] == ips[i-1])) /* Already locked */ if (i && (ips[i] == ips[i - 1])) /* Already locked */
continue; continue;
/* /*
* If try_lock is not set yet, make sure all locked inodes * If try_lock is not set yet, make sure all locked inodes are
* are not in the AIL. * not in the AIL. If any are, set try_lock to be used later.
* If any are, set try_lock to be used later.
*/ */
if (!try_lock) { if (!try_lock) {
for (j = (i - 1); j >= 0 && !try_lock; j--) { for (j = (i - 1); j >= 0 && !try_lock; j--) {
lp = (xfs_log_item_t *)ips[j]->i_itemp; lp = (xfs_log_item_t *)ips[j]->i_itemp;
if (lp && (lp->li_flags & XFS_LI_IN_AIL)) { if (lp && (lp->li_flags & XFS_LI_IN_AIL))
try_lock++; try_lock++;
}
} }
} }
@ -381,51 +377,42 @@ again:
* we can't get any, we must release all we have * we can't get any, we must release all we have
* and try again. * and try again.
*/ */
if (!try_lock) {
if (try_lock) {
/* try_lock must be 0 if i is 0. */
/*
* try_lock means we have an inode locked
* that is in the AIL.
*/
ASSERT(i != 0);
if (!xfs_ilock_nowait(ips[i], xfs_lock_inumorder(lock_mode, i))) {
attempts++;
/*
* Unlock all previous guys and try again.
* xfs_iunlock will try to push the tail
* if the inode is in the AIL.
*/
for(j = i - 1; j >= 0; j--) {
/*
* Check to see if we've already
* unlocked this one.
* Not the first one going back,
* and the inode ptr is the same.
*/
if ((j != (i - 1)) && ips[j] ==
ips[j+1])
continue;
xfs_iunlock(ips[j], lock_mode);
}
if ((attempts % 5) == 0) {
delay(1); /* Don't just spin the CPU */
#ifdef DEBUG
xfs_lock_delays++;
#endif
}
i = 0;
try_lock = 0;
goto again;
}
} else {
xfs_ilock(ips[i], xfs_lock_inumorder(lock_mode, i)); xfs_ilock(ips[i], xfs_lock_inumorder(lock_mode, i));
continue;
} }
/* try_lock means we have an inode locked that is in the AIL. */
ASSERT(i != 0);
if (xfs_ilock_nowait(ips[i], xfs_lock_inumorder(lock_mode, i)))
continue;
/*
* Unlock all previous guys and try again. xfs_iunlock will try
* to push the tail if the inode is in the AIL.
*/
attempts++;
for (j = i - 1; j >= 0; j--) {
/*
* Check to see if we've already unlocked this one. Not
* the first one going back, and the inode ptr is the
* same.
*/
if (j != (i - 1) && ips[j] == ips[j + 1])
continue;
xfs_iunlock(ips[j], lock_mode);
}
if ((attempts % 5) == 0) {
delay(1); /* Don't just spin the CPU */
#ifdef DEBUG
xfs_lock_delays++;
#endif
}
i = 0;
try_lock = 0;
goto again;
} }
#ifdef DEBUG #ifdef DEBUG
@ -2615,19 +2602,22 @@ xfs_remove(
/* /*
* Enter all inodes for a rename transaction into a sorted array. * Enter all inodes for a rename transaction into a sorted array.
*/ */
#define __XFS_SORT_INODES 5
STATIC void STATIC void
xfs_sort_for_rename( xfs_sort_for_rename(
xfs_inode_t *dp1, /* in: old (source) directory inode */ struct xfs_inode *dp1, /* in: old (source) directory inode */
xfs_inode_t *dp2, /* in: new (target) directory inode */ struct xfs_inode *dp2, /* in: new (target) directory inode */
xfs_inode_t *ip1, /* in: inode of old entry */ struct xfs_inode *ip1, /* in: inode of old entry */
xfs_inode_t *ip2, /* in: inode of new entry, if it struct xfs_inode *ip2, /* in: inode of new entry */
already exists, NULL otherwise. */ struct xfs_inode *wip, /* in: whiteout inode */
xfs_inode_t **i_tab,/* out: array of inode returned, sorted */ struct xfs_inode **i_tab,/* out: sorted array of inodes */
int *num_inodes) /* out: number of inodes in array */ int *num_inodes) /* in/out: inodes in array */
{ {
xfs_inode_t *temp;
int i, j; int i, j;
ASSERT(*num_inodes == __XFS_SORT_INODES);
memset(i_tab, 0, *num_inodes * sizeof(struct xfs_inode *));
/* /*
* i_tab contains a list of pointers to inodes. We initialize * i_tab contains a list of pointers to inodes. We initialize
* the table here & we'll sort it. We will then use it to * the table here & we'll sort it. We will then use it to
@ -2635,25 +2625,24 @@ xfs_sort_for_rename(
* *
* Note that the table may contain duplicates. e.g., dp1 == dp2. * Note that the table may contain duplicates. e.g., dp1 == dp2.
*/ */
i_tab[0] = dp1; i = 0;
i_tab[1] = dp2; i_tab[i++] = dp1;
i_tab[2] = ip1; i_tab[i++] = dp2;
if (ip2) { i_tab[i++] = ip1;
*num_inodes = 4; if (ip2)
i_tab[3] = ip2; i_tab[i++] = ip2;
} else { if (wip)
*num_inodes = 3; i_tab[i++] = wip;
i_tab[3] = NULL; *num_inodes = i;
}
/* /*
* Sort the elements via bubble sort. (Remember, there are at * Sort the elements via bubble sort. (Remember, there are at
* most 4 elements to sort, so this is adequate.) * most 5 elements to sort, so this is adequate.)
*/ */
for (i = 0; i < *num_inodes; i++) { for (i = 0; i < *num_inodes; i++) {
for (j = 1; j < *num_inodes; j++) { for (j = 1; j < *num_inodes; j++) {
if (i_tab[j]->i_ino < i_tab[j-1]->i_ino) { if (i_tab[j]->i_ino < i_tab[j-1]->i_ino) {
temp = i_tab[j]; struct xfs_inode *temp = i_tab[j];
i_tab[j] = i_tab[j-1]; i_tab[j] = i_tab[j-1];
i_tab[j-1] = temp; i_tab[j-1] = temp;
} }
@ -2801,16 +2790,16 @@ xfs_rename(
xfs_fsblock_t first_block; xfs_fsblock_t first_block;
int cancel_flags; int cancel_flags;
int committed; int committed;
xfs_inode_t *inodes[4]; xfs_inode_t *inodes[__XFS_SORT_INODES];
int num_inodes = __XFS_SORT_INODES;
int spaceres; int spaceres;
int num_inodes;
trace_xfs_rename(src_dp, target_dp, src_name, target_name); trace_xfs_rename(src_dp, target_dp, src_name, target_name);
new_parent = (src_dp != target_dp); new_parent = (src_dp != target_dp);
src_is_directory = S_ISDIR(src_ip->i_d.di_mode); src_is_directory = S_ISDIR(src_ip->i_d.di_mode);
xfs_sort_for_rename(src_dp, target_dp, src_ip, target_ip, xfs_sort_for_rename(src_dp, target_dp, src_ip, target_ip, NULL,
inodes, &num_inodes); inodes, &num_inodes);
xfs_bmap_init(&free_list, &first_block); xfs_bmap_init(&free_list, &first_block);