Skip to content

Commit

Permalink
Btrfs: Fix some data=ordered related data corruptions
Browse files Browse the repository at this point in the history
Stress testing was showing data checksum errors, most of which were caused
by a lookup bug in the extent_map tree.  The tree was caching the last
pointer returned, and searches would check the last pointer first.

But, search callers also expect the search to return the very first
matching extent in the range, which wasn't always true with the last
pointer usage.

For now, the code to cache the last return value is just removed.  It is
easy to fix, but I think lookups are rare enough that it isn't required anymore.

This commit also replaces do_sync_mapping_range with a local copy of the
related functions.

Signed-off-by: Chris Mason <chris.mason@oracle.com>
  • Loading branch information
Chris Mason committed Sep 25, 2008
1 parent a61e6f2 commit f421950
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 83 deletions.
2 changes: 2 additions & 0 deletions fs/btrfs/ctree.h
Original file line number Diff line number Diff line change
Expand Up @@ -1590,6 +1590,8 @@ int btrfs_csum_truncate(struct btrfs_trans_handle *trans,
struct btrfs_root *root, struct btrfs_path *path,
u64 isize);
/* inode.c */
int btrfs_writepages(struct address_space *mapping,
struct writeback_control *wbc);
int btrfs_create_subvol_root(struct btrfs_root *new_root,
struct btrfs_trans_handle *trans, u64 new_dirid,
struct btrfs_block_group_cache *block_group);
Expand Down
20 changes: 0 additions & 20 deletions fs/btrfs/extent_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ void extent_io_tree_init(struct extent_io_tree *tree,
spin_lock_init(&tree->lock);
spin_lock_init(&tree->buffer_lock);
tree->mapping = mapping;
tree->last = NULL;
}
EXPORT_SYMBOL(extent_io_tree_init);

Expand Down Expand Up @@ -173,12 +172,6 @@ static struct rb_node *__etree_search(struct extent_io_tree *tree, u64 offset,
struct tree_entry *entry;
struct tree_entry *prev_entry = NULL;

if (tree->last) {
struct extent_state *state;
state = tree->last;
if (state->start <= offset && offset <= state->end)
return &tree->last->rb_node;
}
while(n) {
entry = rb_entry(n, struct tree_entry, rb_node);
prev = n;
Expand All @@ -189,7 +182,6 @@ static struct rb_node *__etree_search(struct extent_io_tree *tree, u64 offset,
else if (offset > entry->end)
n = n->rb_right;
else {
tree->last = rb_entry(n, struct extent_state, rb_node);
return n;
}
}
Expand Down Expand Up @@ -223,10 +215,6 @@ static inline struct rb_node *tree_search(struct extent_io_tree *tree,

ret = __etree_search(tree, offset, &prev, NULL);
if (!ret) {
if (prev) {
tree->last = rb_entry(prev, struct extent_state,
rb_node);
}
return prev;
}
return ret;
Expand Down Expand Up @@ -301,8 +289,6 @@ static int merge_state(struct extent_io_tree *tree,
other->state == state->state) {
state->start = other->start;
other->tree = NULL;
if (tree->last == other)
tree->last = state;
rb_erase(&other->rb_node, &tree->state);
free_extent_state(other);
}
Expand All @@ -314,8 +300,6 @@ static int merge_state(struct extent_io_tree *tree,
other->state == state->state) {
other->start = state->start;
state->tree = NULL;
if (tree->last == state)
tree->last = other;
rb_erase(&state->rb_node, &tree->state);
free_extent_state(state);
}
Expand Down Expand Up @@ -378,7 +362,6 @@ static int insert_state(struct extent_io_tree *tree,
return -EEXIST;
}
state->tree = tree;
tree->last = state;
merge_state(tree, state);
return 0;
}
Expand Down Expand Up @@ -444,9 +427,6 @@ static int clear_state_bit(struct extent_io_tree *tree,
if (delete || state->state == 0) {
if (state->tree) {
clear_state_cb(tree, state, state->state);
if (tree->last == state) {
tree->last = extent_state_next(state);
}
rb_erase(&state->rb_node, &tree->state);
state->tree = NULL;
free_extent_state(state);
Expand Down
1 change: 0 additions & 1 deletion fs/btrfs/extent_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ struct extent_io_tree {
spinlock_t lock;
spinlock_t buffer_lock;
struct extent_io_ops *ops;
struct extent_state *last;
};

struct extent_state {
Expand Down
9 changes: 0 additions & 9 deletions fs/btrfs/extent_map.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ void extent_map_exit(void)
void extent_map_tree_init(struct extent_map_tree *tree, gfp_t mask)
{
tree->map.rb_node = NULL;
tree->last = NULL;
spin_lock_init(&tree->lock);
}
EXPORT_SYMBOL(extent_map_tree_init);
Expand Down Expand Up @@ -239,7 +238,6 @@ int add_extent_mapping(struct extent_map_tree *tree,
merge->in_tree = 0;
free_extent_map(merge);
}
tree->last = em;
out:
return ret;
}
Expand Down Expand Up @@ -273,10 +271,6 @@ struct extent_map *lookup_extent_mapping(struct extent_map_tree *tree,
u64 end = range_end(start, len);

BUG_ON(spin_trylock(&tree->lock));
em = tree->last;
if (em && end > em->start && start < extent_map_end(em))
goto found;

rb_node = __tree_search(&tree->map, start, &prev, &next);
if (!rb_node && prev) {
em = rb_entry(prev, struct extent_map, rb_node);
Expand Down Expand Up @@ -305,7 +299,6 @@ struct extent_map *lookup_extent_mapping(struct extent_map_tree *tree,

found:
atomic_inc(&em->refs);
tree->last = em;
out:
return em;
}
Expand All @@ -327,8 +320,6 @@ int remove_extent_mapping(struct extent_map_tree *tree, struct extent_map *em)
BUG_ON(spin_trylock(&tree->lock));
rb_erase(&em->rb_node, &tree->map);
em->in_tree = 0;
if (tree->last == em)
tree->last = NULL;
return ret;
}
EXPORT_SYMBOL(remove_extent_mapping);
1 change: 0 additions & 1 deletion fs/btrfs/extent_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ struct extent_map {

struct extent_map_tree {
struct rb_root map;
struct extent_map *last;
spinlock_t lock;
};

Expand Down
15 changes: 7 additions & 8 deletions fs/btrfs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -381,14 +381,13 @@ int btrfs_drop_extent_cache(struct inode *inode, u64 start, u64 end)
break;
}
if (test_bit(EXTENT_FLAG_PINNED, &em->flags)) {
start = em->start + em->len;
free_extent_map(em);
spin_unlock(&em_tree->lock);
if (start < end) {
len = end - start + 1;
continue;
}
break;
printk(KERN_CRIT "inode %lu trying to drop pinned "
"extent start %llu end %llu, em [%llu %llu]\n",
inode->i_ino,
(unsigned long long)start,
(unsigned long long)end,
(unsigned long long)em->start,
(unsigned long long)em->len);
}
remove_extent_mapping(em_tree, em);

Expand Down
54 changes: 30 additions & 24 deletions fs/btrfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ int btrfs_writepage_start_hook(struct page *page, u64 start, u64 end)
fixup = kzalloc(sizeof(*fixup), GFP_NOFS);
if (!fixup)
return -EAGAIN;
printk("queueing worker to fixup page %lu %Lu\n", inode->i_ino, page_offset(page));

SetPageChecked(page);
page_cache_get(page);
fixup->work.func = btrfs_writepage_fixup_worker;
Expand All @@ -502,11 +502,13 @@ static int btrfs_finish_ordered_io(struct inode *inode, u64 start, u64 end)
struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
struct extent_map_tree *em_tree = &BTRFS_I(inode)->extent_tree;
struct extent_map *em;
struct extent_map *em_orig;
u64 alloc_hint = 0;
u64 clear_start;
u64 clear_end;
struct list_head list;
struct btrfs_key ins;
struct rb_node *rb;
int ret;

ret = btrfs_dec_test_ordered_pending(inode, start, end - start + 1);
Expand Down Expand Up @@ -535,6 +537,22 @@ static int btrfs_finish_ordered_io(struct inode *inode, u64 start, u64 end)

mutex_lock(&BTRFS_I(inode)->extent_mutex);

spin_lock(&em_tree->lock);
clear_start = ordered_extent->file_offset;
clear_end = ordered_extent->file_offset + ordered_extent->len;
em = lookup_extent_mapping(em_tree, clear_start,
ordered_extent->len);
em_orig = em;
while(em && clear_start < extent_map_end(em) && clear_end > em->start) {
clear_bit(EXTENT_FLAG_PINNED, &em->flags);
rb = rb_next(&em->rb_node);
if (!rb)
break;
em = rb_entry(rb, struct extent_map, rb_node);
}
free_extent_map(em_orig);
spin_unlock(&em_tree->lock);

ret = btrfs_drop_extents(trans, root, inode,
ordered_extent->file_offset,
ordered_extent->file_offset +
Expand All @@ -548,22 +566,6 @@ static int btrfs_finish_ordered_io(struct inode *inode, u64 start, u64 end)
ordered_extent->len, 0);
BUG_ON(ret);

spin_lock(&em_tree->lock);
clear_start = ordered_extent->file_offset;
clear_end = ordered_extent->file_offset + ordered_extent->len;
while(clear_start < clear_end) {
em = lookup_extent_mapping(em_tree, clear_start,
clear_end - clear_start);
if (em) {
clear_bit(EXTENT_FLAG_PINNED, &em->flags);
clear_start = em->start + em->len;
free_extent_map(em);
} else {
break;
}
}
spin_unlock(&em_tree->lock);

btrfs_drop_extent_cache(inode, ordered_extent->file_offset,
ordered_extent->file_offset +
ordered_extent->len - 1);
Expand Down Expand Up @@ -2318,7 +2320,7 @@ struct extent_map *btrfs_get_extent(struct inode *inode, struct page *page,
u64 extent_end = 0;
u64 objectid = inode->i_ino;
u32 found_type;
struct btrfs_path *path;
struct btrfs_path *path = NULL;
struct btrfs_root *root = BTRFS_I(inode)->root;
struct btrfs_file_extent_item *item;
struct extent_buffer *leaf;
Expand All @@ -2328,9 +2330,6 @@ struct extent_map *btrfs_get_extent(struct inode *inode, struct page *page,
struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
struct btrfs_trans_handle *trans = NULL;

path = btrfs_alloc_path();
BUG_ON(!path);

again:
spin_lock(&em_tree->lock);
em = lookup_extent_mapping(em_tree, start, len);
Expand All @@ -2354,6 +2353,12 @@ struct extent_map *btrfs_get_extent(struct inode *inode, struct page *page,
em->bdev = root->fs_info->fs_devices->latest_bdev;
em->start = EXTENT_MAP_HOLE;
em->len = (u64)-1;

if (!path) {
path = btrfs_alloc_path();
BUG_ON(!path);
}

ret = btrfs_lookup_file_extent(trans, root, path,
objectid, start, trans != NULL);
if (ret < 0) {
Expand Down Expand Up @@ -2530,7 +2535,8 @@ struct extent_map *btrfs_get_extent(struct inode *inode, struct page *page,
}
spin_unlock(&em_tree->lock);
out:
btrfs_free_path(path);
if (path)
btrfs_free_path(path);
if (trans) {
ret = btrfs_end_transaction(trans, root);
if (!err) {
Expand Down Expand Up @@ -2643,8 +2649,8 @@ static int btrfs_writepage(struct page *page, struct writeback_control *wbc)
return extent_write_full_page(tree, page, btrfs_get_extent, wbc);
}

static int btrfs_writepages(struct address_space *mapping,
struct writeback_control *wbc)
int btrfs_writepages(struct address_space *mapping,
struct writeback_control *wbc)
{
struct extent_io_tree *tree;
tree = &BTRFS_I(mapping->host)->io_tree;
Expand Down
Loading

0 comments on commit f421950

Please sign in to comment.