[PATCH V12 12/14] btrfs: improve inode's outstanding_extents computation
From: Qu Wenruo <hidden>
Date: 2016-06-30 09:19:05
Subsystem:
btrfs file system, filesystems (vfs and infrastructure), the rest · Maintainers:
Chris Mason, David Sterba, Alexander Viro, Christian Brauner, Linus Torvalds
From: Wang Xiaoguang <redacted>
This issue was revealed by modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB,
When modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB, fsstress test often
gets these warnings from btrfs_destroy_inode():
WARN_ON(BTRFS_I(inode)->outstanding_extents);
WARN_ON(BTRFS_I(inode)->reserved_extents);
Simple test program below can reproduce this issue steadily.
Note: you need to modify BTRFS_MAX_EXTENT_SIZE to 64KB to have test,
otherwise there won't be such WARNING.
#include <string.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
int main(void)
{
int fd;
char buf[68 *1024];
memset(buf, 0, 68 * 1024);
fd = open("testfile", O_CREAT | O_EXCL | O_RDWR);
pwrite(fd, buf, 68 * 1024, 64 * 1024);
return;
}
When BTRFS_MAX_EXTENT_SIZE is 64KB, and buffered data range is:
64KB 128K 132KB
|-----------------------------------------------|---------------|
64 + 4KB
1) for above data range, btrfs_delalloc_reserve_metadata() will reserve
metadata and set BTRFS_I(inode)->outstanding_extents to 2.
(68KB + 64KB - 1) / 64KB == 2
Outstanding_extents: 2
2) then btrfs_dirty_page() will be called to dirty pages and set
EXTENT_DELALLOC flag. In this case, btrfs_set_bit_hook() will be called
twice.
The 1st set_bit_hook() call will set DEALLOC flag for the first 64K.
64KB 128KB
|-----------------------------------------------|
64KB DELALLOC
Outstanding_extents: 2
Set_bit_hooks() uses FIRST_DELALLOC flag to avoid re-increase
outstanding_extents counter.
So for 1st set_bit_hooks() call, it won't modify outstanding_extents,
it's still 2.
Then FIRST_DELALLOC flag is *CLEARED*.
3) 2nd btrfs_set_bit_hook() call.
Because FIRST_DELALLOC have been cleared by previous set_bit_hook(),
btrfs_set_bit_hook() will increase BTRFS_I(inode)->outstanding_extents by
one, so now BTRFS_I(inode)->outstanding_extents is 3.
64KB 128KB 132KB
|-----------------------------------------------|----------------|
64K DELALLOC 4K DELALLOC
Outstanding_extents: 3
But the correct outstanding_extents number should be 2, not 3.
The 2nd btrfs_set_bit_hook() call just screwed up this, and leads to the
WARN_ON().
Normally, we can solve it by only increasing outstanding_extents in
set_bit_hook().
But the problem is for delalloc_reserve/release_metadata(), we only have
a 'length' parameter, and calculate in-accurate outstanding_extents.
If we only rely on set_bit_hook() release_metadata() will crew things up
as it will decrease inaccurate number.
So the fix we use is:
1) Increase *INACCURATE* outstanding_extents at delalloc_reserve_meta
Just as a place holder.
2) Increase *accurate* outstanding_extents at set_bit_hooks()
This is the real increaser.
3) Decrease *INACCURATE* outstanding_extents before returning
This makes outstanding_extents to correct value.
For 128M BTRFS_MAX_EXTENT_SIZE, due to limitation of
__btrfs_buffered_write(), each iteration will only handle about 2MB
data.
So btrfs_dirty_pages() won't need to handle cases cross 2 extents.
Cc: Mark Fasheh <redacted>
Cc: Josef Bacik <redacted>
Signed-off-by: Wang Xiaoguang <redacted>
---
fs/btrfs/ctree.h | 2 ++
fs/btrfs/inode.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
fs/btrfs/ioctl.c | 6 ++---
3 files changed, 66 insertions(+), 10 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index a513433..ec6b61b 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h@@ -3094,6 +3094,8 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int delay_iput, int nr); int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end, struct extent_state **cached_state); +int btrfs_set_extent_defrag(struct inode *inode, u64 start, u64 end, + struct extent_state **cached_state); int btrfs_create_subvol_root(struct btrfs_trans_handle *trans, struct btrfs_root *new_root, struct btrfs_root *parent_root,
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b83e322..ff9f8a8 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c@@ -1729,11 +1729,15 @@ static void btrfs_split_extent_hook(struct inode *inode, struct extent_state *orig, u64 split) { u64 size; + struct btrfs_root *root = BTRFS_I(inode)->root; /* not delalloc, ignore it */ if (!(orig->state & EXTENT_DELALLOC)) return; + if (root == root->fs_info->tree_root) + return; + size = orig->end - orig->start + 1; if (size > BTRFS_MAX_EXTENT_SIZE) { u64 num_extents;
@@ -1771,11 +1775,15 @@ static void btrfs_merge_extent_hook(struct inode *inode, { u64 new_size, old_size; u64 num_extents; + struct btrfs_root *root = BTRFS_I(inode)->root; /* not delalloc, ignore it */ if (!(other->state & EXTENT_DELALLOC)) return; + if (root == root->fs_info->tree_root) + return; + if (new->start > other->start) new_size = new->end - other->start + 1; else
@@ -1882,13 +1890,16 @@ static void btrfs_set_bit_hook(struct inode *inode, if (!(state->state & EXTENT_DELALLOC) && (*bits & EXTENT_DELALLOC)) { struct btrfs_root *root = BTRFS_I(inode)->root; u64 len = state->end + 1 - state->start; + u64 num_extents = div64_u64(len + BTRFS_MAX_EXTENT_SIZE - 1, + BTRFS_MAX_EXTENT_SIZE); bool do_list = !btrfs_is_free_space_inode(inode); - if (*bits & EXTENT_FIRST_DELALLOC) { + if (*bits & EXTENT_FIRST_DELALLOC) *bits &= ~EXTENT_FIRST_DELALLOC; - } else { + + if (root != root->fs_info->tree_root) { spin_lock(&BTRFS_I(inode)->lock); - BTRFS_I(inode)->outstanding_extents++; + BTRFS_I(inode)->outstanding_extents += num_extents; spin_unlock(&BTRFS_I(inode)->lock); }
@@ -1936,7 +1947,7 @@ static void btrfs_clear_bit_hook(struct inode *inode, if (*bits & EXTENT_FIRST_DELALLOC) { *bits &= ~EXTENT_FIRST_DELALLOC; - } else if (!(*bits & EXTENT_DO_ACCOUNTING)) { + } else if (!(*bits & EXTENT_DO_ACCOUNTING) && do_list) { spin_lock(&BTRFS_I(inode)->lock); BTRFS_I(inode)->outstanding_extents -= num_extents; spin_unlock(&BTRFS_I(inode)->lock);
@@ -2129,9 +2140,54 @@ static noinline int add_pending_csums(struct btrfs_trans_handle *trans, int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end, struct extent_state **cached_state) { + int ret; + struct btrfs_root *root = BTRFS_I(inode)->root; + u64 num_extents = div64_u64(end - start + BTRFS_MAX_EXTENT_SIZE, + BTRFS_MAX_EXTENT_SIZE); + WARN_ON((end & (PAGE_SIZE - 1)) == 0); - return set_extent_delalloc(&BTRFS_I(inode)->io_tree, start, end, - cached_state); + ret = set_extent_delalloc(&BTRFS_I(inode)->io_tree, start, end, + cached_state); + + /* + * btrfs_delalloc_reserve_metadata() will first add number of + * outstanding extents according to data length, which is inaccurate + * for case like dirtying already dirty pages. + * so here we will decrease such inaccurate numbers, to make + * outstanding_extents only rely on the correct values added by + * set_bit_hook() + * + * Also, we skipped the metadata space reserve for space cache inodes, + * so don't modify the outstanding_extents value. + */ + if (ret == 0 && root != root->fs_info->tree_root) { + spin_lock(&BTRFS_I(inode)->lock); + BTRFS_I(inode)->outstanding_extents -= num_extents; + spin_unlock(&BTRFS_I(inode)->lock); + } + + return ret; +} + +int btrfs_set_extent_defrag(struct inode *inode, u64 start, u64 end, + struct extent_state **cached_state) +{ + int ret; + struct btrfs_root *root = BTRFS_I(inode)->root; + u64 num_extents = div64_u64(end - start + BTRFS_MAX_EXTENT_SIZE, + BTRFS_MAX_EXTENT_SIZE); + + WARN_ON((end & (PAGE_SIZE - 1)) == 0); + ret = set_extent_defrag(&BTRFS_I(inode)->io_tree, start, end, + cached_state); + + if (ret == 0 && root != root->fs_info->tree_root) { + spin_lock(&BTRFS_I(inode)->lock); + BTRFS_I(inode)->outstanding_extents -= num_extents; + spin_unlock(&BTRFS_I(inode)->lock); + } + + return ret; } /* see btrfs_writepage_start_hook for details on why this is required */
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 74e2cef0..b9bc0de 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c@@ -1236,10 +1236,8 @@ again: (page_cnt - i_done) << PAGE_SHIFT); } - - set_extent_defrag(&BTRFS_I(inode)->io_tree, page_start, page_end - 1, - &cached_state); - + btrfs_set_extent_defrag(inode, page_start, + page_end - 1, &cached_state); unlock_extent_cached(&BTRFS_I(inode)->io_tree, page_start, page_end - 1, &cached_state, GFP_NOFS);
--
2.9.0