Commit 6c282eb4 authored by Li Zefan's avatar Li Zefan Committed by Chris Mason

Btrfs: fix defrag regression

If a file has 3 small extents:

| ext1 | ext2 | ext3 |

Running "btrfs fi defrag" will only defrag the last two extents, if those
extent mappings hasn't been read into memory from disk.

This bug was introduced by commit 17ce6ef8
("Btrfs: add a check to decide if we should defrag the range")

The cause is, that commit looked into previous and next extents using
lookup_extent_mapping() only.

While at it, remove the code that checks the previous extent, since
it's sufficient to check the next extent.
Signed-off-by: default avatarLi Zefan <lizefan@huawei.com>
parent 7ddf5a42
...@@ -786,39 +786,57 @@ static int find_new_extents(struct btrfs_root *root, ...@@ -786,39 +786,57 @@ static int find_new_extents(struct btrfs_root *root,
return -ENOENT; return -ENOENT;
} }
/* static struct extent_map *defrag_lookup_extent(struct inode *inode, u64 start)
* Validaty check of prev em and next em:
* 1) no prev/next em
* 2) prev/next em is an hole/inline extent
*/
static int check_adjacent_extents(struct inode *inode, struct extent_map *em)
{ {
struct extent_map_tree *em_tree = &BTRFS_I(inode)->extent_tree; struct extent_map_tree *em_tree = &BTRFS_I(inode)->extent_tree;
struct extent_map *prev = NULL, *next = NULL; struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
int ret = 0; struct extent_map *em;
u64 len = PAGE_CACHE_SIZE;
/*
* hopefully we have this extent in the tree already, try without
* the full extent lock
*/
read_lock(&em_tree->lock); read_lock(&em_tree->lock);
prev = lookup_extent_mapping(em_tree, em->start - 1, (u64)-1); em = lookup_extent_mapping(em_tree, start, len);
next = lookup_extent_mapping(em_tree, em->start + em->len, (u64)-1);
read_unlock(&em_tree->lock); read_unlock(&em_tree->lock);
if ((!prev || prev->block_start >= EXTENT_MAP_LAST_BYTE) && if (!em) {
(!next || next->block_start >= EXTENT_MAP_LAST_BYTE)) /* get the big lock and read metadata off disk */
ret = 1; lock_extent(io_tree, start, start + len - 1);
free_extent_map(prev); em = btrfs_get_extent(inode, NULL, 0, start, len, 0);
free_extent_map(next); unlock_extent(io_tree, start, start + len - 1);
if (IS_ERR(em))
return NULL;
}
return em;
}
static bool defrag_check_next_extent(struct inode *inode, struct extent_map *em)
{
struct extent_map *next;
bool ret = true;
/* this is the last extent */
if (em->start + em->len >= i_size_read(inode))
return false;
next = defrag_lookup_extent(inode, em->start + em->len);
if (!next || next->block_start >= EXTENT_MAP_LAST_BYTE)
ret = false;
free_extent_map(next);
return ret; return ret;
} }
static int should_defrag_range(struct inode *inode, u64 start, u64 len, static int should_defrag_range(struct inode *inode, u64 start, int thresh,
int thresh, u64 *last_len, u64 *skip, u64 *last_len, u64 *skip, u64 *defrag_end)
u64 *defrag_end)
{ {
struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree; struct extent_map *em;
struct extent_map *em = NULL;
struct extent_map_tree *em_tree = &BTRFS_I(inode)->extent_tree;
int ret = 1; int ret = 1;
bool next_mergeable = true;
/* /*
* make sure that once we start defragging an extent, we keep on * make sure that once we start defragging an extent, we keep on
...@@ -829,23 +847,9 @@ static int should_defrag_range(struct inode *inode, u64 start, u64 len, ...@@ -829,23 +847,9 @@ static int should_defrag_range(struct inode *inode, u64 start, u64 len,
*skip = 0; *skip = 0;
/* em = defrag_lookup_extent(inode, start);
* hopefully we have this extent in the tree already, try without if (!em)
* the full extent lock
*/
read_lock(&em_tree->lock);
em = lookup_extent_mapping(em_tree, start, len);
read_unlock(&em_tree->lock);
if (!em) {
/* get the big lock and read metadata off disk */
lock_extent(io_tree, start, start + len - 1);
em = btrfs_get_extent(inode, NULL, 0, start, len, 0);
unlock_extent(io_tree, start, start + len - 1);
if (IS_ERR(em))
return 0; return 0;
}
/* this will cover holes, and inline extents */ /* this will cover holes, and inline extents */
if (em->block_start >= EXTENT_MAP_LAST_BYTE) { if (em->block_start >= EXTENT_MAP_LAST_BYTE) {
...@@ -853,18 +857,15 @@ static int should_defrag_range(struct inode *inode, u64 start, u64 len, ...@@ -853,18 +857,15 @@ static int should_defrag_range(struct inode *inode, u64 start, u64 len,
goto out; goto out;
} }
/* If we have nothing to merge with us, just skip. */ next_mergeable = defrag_check_next_extent(inode, em);
if (check_adjacent_extents(inode, em)) {
ret = 0;
goto out;
}
/* /*
* we hit a real extent, if it is big don't bother defragging it again * we hit a real extent, if it is big or the next extent is not a
* real extent, don't bother defragging it
*/ */
if ((*last_len == 0 || *last_len >= thresh) && em->len >= thresh) if ((*last_len == 0 || *last_len >= thresh) &&
(em->len >= thresh || !next_mergeable))
ret = 0; ret = 0;
out: out:
/* /*
* last_len ends up being a counter of how many bytes we've defragged. * last_len ends up being a counter of how many bytes we've defragged.
...@@ -1143,8 +1144,8 @@ int btrfs_defrag_file(struct inode *inode, struct file *file, ...@@ -1143,8 +1144,8 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
break; break;
if (!should_defrag_range(inode, (u64)i << PAGE_CACHE_SHIFT, if (!should_defrag_range(inode, (u64)i << PAGE_CACHE_SHIFT,
PAGE_CACHE_SIZE, extent_thresh, extent_thresh, &last_len, &skip,
&last_len, &skip, &defrag_end)) { &defrag_end)) {
unsigned long next; unsigned long next;
/* /*
* the should_defrag function tells us how much to skip * the should_defrag function tells us how much to skip
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment