Skip to content

Commit 0934856

Browse files
Miao XieJosef Bacik
authored andcommitted
Btrfs: fix deadlock due to unsubmitted
The deadlock problem happened when running fsstress(a test program in LTP). Steps to reproduce: # mkfs.btrfs -b 100M <partition> # mount <partition> <mnt> # <Path>/fsstress -p 3 -n 10000000 -d <mnt> The reason is: btrfs_direct_IO() |->do_direct_IO() |->get_page() |->get_blocks() | |->btrfs_delalloc_resereve_space() | |->btrfs_add_ordered_extent() ------- Add a new ordered extent |->dio_send_cur_page(page0) -------------- We didn't submit bio here |->get_page() |->get_blocks() |->btrfs_delalloc_resereve_space() |->flush_space() |->btrfs_start_ordered_extent() |->wait_event() ---------- Wait the completion of the ordered extent that is mentioned above But because we didn't submit the bio that is mentioned above, the ordered extent can not complete, we would wait for its completion forever. There are two methods which can fix this deadlock problem: 1. submit the bio before we invoke get_blocks() 2. reserve the space before we do dio Though the 1st is the simplest way, we need modify the code of VFS, and it is likely to break contiguous requests, and introduce performance regression for the other filesystems. So we have to choose the 2nd way. Signed-off-by: Miao Xie <[email protected]> Cc: Josef Bacik <[email protected]> Signed-off-by: Josef Bacik <[email protected]>
1 parent 4a7d0f6 commit 0934856

File tree

2 files changed

+43
-41
lines changed

2 files changed

+43
-41
lines changed

fs/btrfs/extent-tree.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4748,7 +4748,8 @@ void btrfs_delalloc_release_metadata(struct inode *inode, u64 num_bytes)
47484748
spin_lock(&BTRFS_I(inode)->lock);
47494749
dropped = drop_outstanding_extent(inode);
47504750

4751-
to_free = calc_csum_metadata_size(inode, num_bytes, 0);
4751+
if (num_bytes)
4752+
to_free = calc_csum_metadata_size(inode, num_bytes, 0);
47524753
spin_unlock(&BTRFS_I(inode)->lock);
47534754
if (dropped > 0)
47544755
to_free += btrfs_calc_trans_metadata_size(root, dropped);

fs/btrfs/inode.c

Lines changed: 41 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6059,16 +6059,15 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
60596059
u64 len = bh_result->b_size;
60606060
struct btrfs_trans_handle *trans;
60616061
int unlock_bits = EXTENT_LOCKED;
6062-
int ret;
6062+
int ret = 0;
60636063

60646064
if (create) {
6065-
ret = btrfs_delalloc_reserve_space(inode, len);
6066-
if (ret)
6067-
return ret;
6065+
spin_lock(&BTRFS_I(inode)->lock);
6066+
BTRFS_I(inode)->outstanding_extents++;
6067+
spin_unlock(&BTRFS_I(inode)->lock);
60686068
unlock_bits |= EXTENT_DELALLOC | EXTENT_DIRTY;
6069-
} else {
6069+
} else
60706070
len = min_t(u64, len, root->sectorsize);
6071-
}
60726071

60736072
lockstart = start;
60746073
lockend = start + len - 1;
@@ -6080,14 +6079,6 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
60806079
if (lock_extent_direct(inode, lockstart, lockend, &cached_state, create))
60816080
return -ENOTBLK;
60826081

6083-
if (create) {
6084-
ret = set_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
6085-
lockend, EXTENT_DELALLOC, NULL,
6086-
&cached_state, GFP_NOFS);
6087-
if (ret)
6088-
goto unlock_err;
6089-
}
6090-
60916082
em = btrfs_get_extent(inode, NULL, 0, start, len, 0);
60926083
if (IS_ERR(em)) {
60936084
ret = PTR_ERR(em);
@@ -6119,7 +6110,6 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
61196110
if (!create && (em->block_start == EXTENT_MAP_HOLE ||
61206111
test_bit(EXTENT_FLAG_PREALLOC, &em->flags))) {
61216112
free_extent_map(em);
6122-
ret = 0;
61236113
goto unlock_err;
61246114
}
61256115

@@ -6217,6 +6207,11 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
62176207
*/
62186208
if (start + len > i_size_read(inode))
62196209
i_size_write(inode, start + len);
6210+
6211+
ret = set_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
6212+
lockstart + len - 1, EXTENT_DELALLOC, NULL,
6213+
&cached_state, GFP_NOFS);
6214+
BUG_ON(ret);
62206215
}
62216216

62226217
/*
@@ -6225,24 +6220,9 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
62256220
* aren't using if there is any left over space.
62266221
*/
62276222
if (lockstart < lockend) {
6228-
if (create && len < lockend - lockstart) {
6229-
clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
6230-
lockstart + len - 1,
6231-
unlock_bits | EXTENT_DEFRAG, 1, 0,
6232-
&cached_state, GFP_NOFS);
6233-
/*
6234-
* Beside unlock, we also need to cleanup reserved space
6235-
* for the left range by attaching EXTENT_DO_ACCOUNTING.
6236-
*/
6237-
clear_extent_bit(&BTRFS_I(inode)->io_tree,
6238-
lockstart + len, lockend,
6239-
unlock_bits | EXTENT_DO_ACCOUNTING |
6240-
EXTENT_DEFRAG, 1, 0, NULL, GFP_NOFS);
6241-
} else {
6242-
clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
6243-
lockend, unlock_bits, 1, 0,
6244-
&cached_state, GFP_NOFS);
6245-
}
6223+
clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
6224+
lockend, unlock_bits, 1, 0,
6225+
&cached_state, GFP_NOFS);
62466226
} else {
62476227
free_extent_state(cached_state);
62486228
}
@@ -6252,9 +6232,6 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
62526232
return 0;
62536233

62546234
unlock_err:
6255-
if (create)
6256-
unlock_bits |= EXTENT_DO_ACCOUNTING;
6257-
62586235
clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart, lockend,
62596236
unlock_bits, 1, 0, &cached_state, GFP_NOFS);
62606237
return ret;
@@ -6692,15 +6669,39 @@ static ssize_t btrfs_direct_IO(int rw, struct kiocb *iocb,
66926669
{
66936670
struct file *file = iocb->ki_filp;
66946671
struct inode *inode = file->f_mapping->host;
6672+
size_t count = 0;
6673+
ssize_t ret;
66956674

66966675
if (check_direct_IO(BTRFS_I(inode)->root, rw, iocb, iov,
66976676
offset, nr_segs))
66986677
return 0;
66996678

6700-
return __blockdev_direct_IO(rw, iocb, inode,
6701-
BTRFS_I(inode)->root->fs_info->fs_devices->latest_bdev,
6702-
iov, offset, nr_segs, btrfs_get_blocks_direct, NULL,
6703-
btrfs_submit_direct, 0);
6679+
if (rw & WRITE) {
6680+
count = iov_length(iov, nr_segs);
6681+
ret = btrfs_delalloc_reserve_space(inode, count);
6682+
if (ret)
6683+
return ret;
6684+
}
6685+
6686+
ret = __blockdev_direct_IO(rw, iocb, inode,
6687+
BTRFS_I(inode)->root->fs_info->fs_devices->latest_bdev,
6688+
iov, offset, nr_segs, btrfs_get_blocks_direct, NULL,
6689+
btrfs_submit_direct, 0);
6690+
6691+
if (rw & WRITE) {
6692+
if (ret < 0 && ret != -EIOCBQUEUED)
6693+
btrfs_delalloc_release_space(inode, count);
6694+
else if (ret > 0 && (size_t)ret < count) {
6695+
spin_lock(&BTRFS_I(inode)->lock);
6696+
BTRFS_I(inode)->outstanding_extents++;
6697+
spin_unlock(&BTRFS_I(inode)->lock);
6698+
btrfs_delalloc_release_space(inode,
6699+
count - (size_t)ret);
6700+
}
6701+
btrfs_delalloc_release_metadata(inode, 0);
6702+
}
6703+
6704+
return ret;
67046705
}
67056706

67066707
#define BTRFS_FIEMAP_FLAGS (FIEMAP_FLAG_SYNC)

0 commit comments

Comments
 (0)