diff options
| author | Dave Chinner <dchinner@redhat.com> | 2014-08-04 13:29:32 +1000 |
|---|---|---|
| committer | Ben Hutchings <ben@decadent.org.uk> | 2016-06-15 21:29:32 +0100 |
| commit | 9c457aeadcd3379f06da88647e294cfb5f3ec1da (patch) | |
| tree | 9b6a3b7cdec9f880c8c9eb0b8a96f4ecd8a98805 /fs/xfs | |
| parent | 11e2f8dc61c5efca6eb37dfb40ede38c35c0d5be (diff) | |
| download | linux-stable-9c457aeadcd3379f06da88647e294cfb5f3ec1da.tar.gz linux-stable-9c457aeadcd3379f06da88647e294cfb5f3ec1da.tar.bz2 linux-stable-9c457aeadcd3379f06da88647e294cfb5f3ec1da.zip | |
xfs: fix swapext ilock deadlock
commit 812176832169c77b4bacddd01edc3e55340263fd upstream.
xfs_swap_extents() holds the ilock over a call to
filemap_write_and_wait(), which can then try to write data and take
the ilock. That causes a self-deadlock.
Fix the deadlock and clean up the code by separating the locking
appropriately. Add a lockflags variable to track what locks we are
holding as we gain and drop them and cleanup the error handling to
always use "out_unlock" with the lockflags variable.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Diffstat (limited to 'fs/xfs')
| -rw-r--r-- | fs/xfs/xfs_bmap_util.c | 33 |
1 files changed, 18 insertions, 15 deletions
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c index 64731ef3324d..cc7383e17f6c 100644 --- a/fs/xfs/xfs_bmap_util.c +++ b/fs/xfs/xfs_bmap_util.c @@ -1633,6 +1633,7 @@ xfs_swap_extents( int aforkblks = 0; int taforkblks = 0; __uint64_t tmp; + int lock_flags; tempifp = kmem_alloc(sizeof(xfs_ifork_t), KM_MAYFAIL); if (!tempifp) { @@ -1641,13 +1642,13 @@ xfs_swap_extents( } /* - * we have to do two separate lock calls here to keep lockdep - * happy. If we try to get all the locks in one call, lock will - * report false positives when we drop the ILOCK and regain them - * below. + * Lock up the inodes against other IO and truncate to begin with. + * Then we can ensure the inodes are flushed and have no page cache + * safely. Once we have done this we can take the ilocks and do the rest + * of the checks. */ + lock_flags = XFS_IOLOCK_EXCL; xfs_lock_two_inodes(ip, tip, XFS_IOLOCK_EXCL); - xfs_lock_two_inodes(ip, tip, XFS_ILOCK_EXCL); /* Verify that both files have the same format */ if ((ip->i_d.di_mode & S_IFMT) != (tip->i_d.di_mode & S_IFMT)) { @@ -1666,6 +1667,9 @@ xfs_swap_extents( goto out_unlock; truncate_pagecache_range(VFS_I(tip), 0, -1); + xfs_lock_two_inodes(ip, tip, XFS_ILOCK_EXCL); + lock_flags |= XFS_ILOCK_EXCL; + /* Verify O_DIRECT for ftmp */ if (VN_CACHED(VFS_I(tip)) != 0) { error = XFS_ERROR(EINVAL); @@ -1720,6 +1724,7 @@ xfs_swap_extents( xfs_iunlock(ip, XFS_ILOCK_EXCL); xfs_iunlock(tip, XFS_ILOCK_EXCL); + lock_flags &= ~XFS_ILOCK_EXCL; /* * There is a race condition here since we gave up the @@ -1732,13 +1737,11 @@ xfs_swap_extents( tp = xfs_trans_alloc(mp, XFS_TRANS_SWAPEXT); error = xfs_trans_reserve(tp, &M_RES(mp)->tr_ichange, 0, 0); - if (error) { - xfs_iunlock(ip, XFS_IOLOCK_EXCL); - xfs_iunlock(tip, XFS_IOLOCK_EXCL); - xfs_trans_cancel(tp, 0); - goto out; - } + if (error) + goto out_trans_cancel; + xfs_lock_two_inodes(ip, tip, XFS_ILOCK_EXCL); + lock_flags |= XFS_ILOCK_EXCL; /* * Count the number of extended attribute blocks @@ -1757,8 +1760,8 @@ xfs_swap_extents( goto out_trans_cancel; } - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); - xfs_trans_ijoin(tp, tip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); + xfs_trans_ijoin(tp, ip, lock_flags); + xfs_trans_ijoin(tp, tip, lock_flags); /* * Before we've swapped the forks, lets set the owners of the forks @@ -1887,8 +1890,8 @@ out: return error; out_unlock: - xfs_iunlock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); - xfs_iunlock(tip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); + xfs_iunlock(ip, lock_flags); + xfs_iunlock(tip, lock_flags); goto out; out_trans_cancel: |