Skip to content

Commit

Permalink
Btrfs: fix deadlock due to unsubmitted
Browse files Browse the repository at this point in the history
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 <miaox@cn.fujitsu.com>
Cc: Josef Bacik <jbacik@fusionio.com>
Signed-off-by: Josef Bacik <jbacik@fusionio.com>
  • Loading branch information
Miao Xie authored and Josef Bacik committed Feb 20, 2013
1 parent 4a7d0f6 commit 0934856
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 41 deletions.
3 changes: 2 additions & 1 deletion fs/btrfs/extent-tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -4748,7 +4748,8 @@ void btrfs_delalloc_release_metadata(struct inode *inode, u64 num_bytes)
spin_lock(&BTRFS_I(inode)->lock);
dropped = drop_outstanding_extent(inode);

to_free = calc_csum_metadata_size(inode, num_bytes, 0);
if (num_bytes)
to_free = calc_csum_metadata_size(inode, num_bytes, 0);
spin_unlock(&BTRFS_I(inode)->lock);
if (dropped > 0)
to_free += btrfs_calc_trans_metadata_size(root, dropped);
Expand Down
81 changes: 41 additions & 40 deletions fs/btrfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -6059,16 +6059,15 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
u64 len = bh_result->b_size;
struct btrfs_trans_handle *trans;
int unlock_bits = EXTENT_LOCKED;
int ret;
int ret = 0;

if (create) {
ret = btrfs_delalloc_reserve_space(inode, len);
if (ret)
return ret;
spin_lock(&BTRFS_I(inode)->lock);
BTRFS_I(inode)->outstanding_extents++;
spin_unlock(&BTRFS_I(inode)->lock);
unlock_bits |= EXTENT_DELALLOC | EXTENT_DIRTY;
} else {
} else
len = min_t(u64, len, root->sectorsize);
}

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

if (create) {
ret = set_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
lockend, EXTENT_DELALLOC, NULL,
&cached_state, GFP_NOFS);
if (ret)
goto unlock_err;
}

em = btrfs_get_extent(inode, NULL, 0, start, len, 0);
if (IS_ERR(em)) {
ret = PTR_ERR(em);
Expand Down Expand Up @@ -6119,7 +6110,6 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
if (!create && (em->block_start == EXTENT_MAP_HOLE ||
test_bit(EXTENT_FLAG_PREALLOC, &em->flags))) {
free_extent_map(em);
ret = 0;
goto unlock_err;
}

Expand Down Expand Up @@ -6217,6 +6207,11 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
*/
if (start + len > i_size_read(inode))
i_size_write(inode, start + len);

ret = set_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
lockstart + len - 1, EXTENT_DELALLOC, NULL,
&cached_state, GFP_NOFS);
BUG_ON(ret);
}

/*
Expand All @@ -6225,24 +6220,9 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
* aren't using if there is any left over space.
*/
if (lockstart < lockend) {
if (create && len < lockend - lockstart) {
clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
lockstart + len - 1,
unlock_bits | EXTENT_DEFRAG, 1, 0,
&cached_state, GFP_NOFS);
/*
* Beside unlock, we also need to cleanup reserved space
* for the left range by attaching EXTENT_DO_ACCOUNTING.
*/
clear_extent_bit(&BTRFS_I(inode)->io_tree,
lockstart + len, lockend,
unlock_bits | EXTENT_DO_ACCOUNTING |
EXTENT_DEFRAG, 1, 0, NULL, GFP_NOFS);
} else {
clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
lockend, unlock_bits, 1, 0,
&cached_state, GFP_NOFS);
}
clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
lockend, unlock_bits, 1, 0,
&cached_state, GFP_NOFS);
} else {
free_extent_state(cached_state);
}
Expand All @@ -6252,9 +6232,6 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock,
return 0;

unlock_err:
if (create)
unlock_bits |= EXTENT_DO_ACCOUNTING;

clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart, lockend,
unlock_bits, 1, 0, &cached_state, GFP_NOFS);
return ret;
Expand Down Expand Up @@ -6692,15 +6669,39 @@ static ssize_t btrfs_direct_IO(int rw, struct kiocb *iocb,
{
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
size_t count = 0;
ssize_t ret;

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

return __blockdev_direct_IO(rw, iocb, inode,
BTRFS_I(inode)->root->fs_info->fs_devices->latest_bdev,
iov, offset, nr_segs, btrfs_get_blocks_direct, NULL,
btrfs_submit_direct, 0);
if (rw & WRITE) {
count = iov_length(iov, nr_segs);
ret = btrfs_delalloc_reserve_space(inode, count);
if (ret)
return ret;
}

ret = __blockdev_direct_IO(rw, iocb, inode,
BTRFS_I(inode)->root->fs_info->fs_devices->latest_bdev,
iov, offset, nr_segs, btrfs_get_blocks_direct, NULL,
btrfs_submit_direct, 0);

if (rw & WRITE) {
if (ret < 0 && ret != -EIOCBQUEUED)
btrfs_delalloc_release_space(inode, count);
else if (ret > 0 && (size_t)ret < count) {
spin_lock(&BTRFS_I(inode)->lock);
BTRFS_I(inode)->outstanding_extents++;
spin_unlock(&BTRFS_I(inode)->lock);
btrfs_delalloc_release_space(inode,
count - (size_t)ret);
}
btrfs_delalloc_release_metadata(inode, 0);
}

return ret;
}

#define BTRFS_FIEMAP_FLAGS (FIEMAP_FLAG_SYNC)
Expand Down

0 comments on commit 0934856

Please sign in to comment.