On Tue, Apr 23, 2013 at 02:20:22PM -0400, Josef Bacik wrote:
> We kept leaking extent buffers when mounting a broken file system and it turns
> out it's because not everybody uses read_tree_block properly. You need to check
> and make sure the extent_buffer is uptodate before you use it. This patch fixes
> everybody who calls read_tree_block directly to make sure they check that it is
> uptodate and free it and return an error if it is not. With this we no longer
> leak EB's when things go horribly wrong. Thanks,
What about hook the check into read_tree_block()?
That way we can save much efforts.
thanks,
liubo
>
> Signed-off-by: Josef Bacik <jbacik@xxxxxxxxxxxx>
> ---
> fs/btrfs/backref.c | 10 ++++++++--
> fs/btrfs/ctree.c | 21 ++++++++++++++++-----
> fs/btrfs/disk-io.c | 19 +++++++++++++++++--
> fs/btrfs/extent-tree.c | 4 +++-
> fs/btrfs/relocation.c | 18 +++++++++++++++---
> 5 files changed, 59 insertions(+), 13 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 23e927b..04b5b30 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -423,7 +423,10 @@ static int __add_missing_keys(struct btrfs_fs_info *fs_info,
> BUG_ON(!ref->wanted_disk_byte);
> eb = read_tree_block(fs_info->tree_root, ref->wanted_disk_byte,
> fs_info->tree_root->leafsize, 0);
> - BUG_ON(!eb);
> + if (!eb || !extent_buffer_uptodate(eb)) {
> + free_extent_buffer(eb);
> + return -EIO;
> + }
> btrfs_tree_read_lock(eb);
> if (btrfs_header_level(eb) == 0)
> btrfs_item_key_to_cpu(eb, &ref->key_for_search, 0);
> @@ -913,7 +916,10 @@ again:
> info_level);
> eb = read_tree_block(fs_info->extent_root,
> ref->parent, bsz, 0);
> - BUG_ON(!eb);
> + if (!eb || !extent_buffer_uptodate(eb)) {
> + free_extent_buffer(eb);
> + return -EIO;
> + }
> ret = find_extent_in_eb(eb, bytenr,
> *extent_item_pos, &eie);
> ref->inode_list = eie;
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 566d99b..2bc3440 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -1281,7 +1281,8 @@ get_old_root(struct btrfs_root *root, u64 time_seq)
> free_extent_buffer(eb_root);
> blocksize = btrfs_level_size(root, old_root->level);
> old = read_tree_block(root, logical, blocksize, 0);
> - if (!old) {
> + if (!old || !extent_buffer_uptodate(old)) {
> + free_extent_buffer(old);
> pr_warn("btrfs: failed to read tree block %llu from get_old_root\n",
> logical);
> WARN_ON(1);
> @@ -1526,8 +1527,10 @@ int btrfs_realloc_node(struct btrfs_trans_handle *trans,
> if (!cur) {
> cur = read_tree_block(root, blocknr,
> blocksize, gen);
> - if (!cur)
> + if (!cur || !extent_buffer_uptodate(cur)) {
> + free_extent_buffer(cur);
> return -EIO;
> + }
> } else if (!uptodate) {
> err = btrfs_read_buffer(cur, gen);
> if (err) {
> @@ -1692,6 +1695,8 @@ static noinline struct extent_buffer *read_node_slot(struct btrfs_root *root,
> struct extent_buffer *parent, int slot)
> {
> int level = btrfs_header_level(parent);
> + struct extent_buffer *eb;
> +
> if (slot < 0)
> return NULL;
> if (slot >= btrfs_header_nritems(parent))
> @@ -1699,9 +1704,15 @@ static noinline struct extent_buffer *read_node_slot(struct btrfs_root *root,
>
> BUG_ON(level == 0);
>
> - return read_tree_block(root, btrfs_node_blockptr(parent, slot),
> - btrfs_level_size(root, level - 1),
> - btrfs_node_ptr_generation(parent, slot));
> + eb = read_tree_block(root, btrfs_node_blockptr(parent, slot),
> + btrfs_level_size(root, level - 1),
> + btrfs_node_ptr_generation(parent, slot));
> + if (eb && !extent_buffer_uptodate(eb)) {
> + free_extent_buffer(eb);
> + eb = NULL;
> + }
> +
> + return eb;
> }
>
> /*
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index fb0e5c2..4605cc7 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1534,6 +1534,14 @@ struct btrfs_root *btrfs_read_fs_root_no_radix(struct btrfs_root *tree_root,
> blocksize = btrfs_level_size(root, btrfs_root_level(&root->root_item));
> root->node = read_tree_block(root, btrfs_root_bytenr(&root->root_item),
> blocksize, generation);
> + if (!root->node || !extent_buffer_uptodate(root->node)) {
> + ret = (!root->node) ? -ENOMEM : -EIO;
> +
> + free_extent_buffer(root->node);
> + kfree(root);
> + return ERR_PTR(ret);
> + }
> +
> root->commit_root = btrfs_root_node(root);
> BUG_ON(!root->node); /* -ENOMEM */
> out:
> @@ -2572,8 +2580,8 @@ int open_ctree(struct super_block *sb,
> chunk_root->node = read_tree_block(chunk_root,
> btrfs_super_chunk_root(disk_super),
> blocksize, generation);
> - BUG_ON(!chunk_root->node); /* -ENOMEM */
> - if (!test_bit(EXTENT_BUFFER_UPTODATE, &chunk_root->node->bflags)) {
> + if (!chunk_root->node ||
> + !test_bit(EXTENT_BUFFER_UPTODATE, &chunk_root->node->bflags)) {
> printk(KERN_WARNING "btrfs: failed to read chunk root on %s\n",
> sb->s_id);
> goto fail_tree_roots;
> @@ -2758,6 +2766,13 @@ retry_root_backup:
> log_tree_root->node = read_tree_block(tree_root, bytenr,
> blocksize,
> generation + 1);
> + if (!log_tree_root->node ||
> + !extent_buffer_uptodate(log_tree_root->node)) {
> + printk(KERN_ERR "btrfs: failed to read log tree\n");
> + free_extent_buffer(log_tree_root->node);
> + kfree(log_tree_root);
> + goto fail_trans_kthread;
> + }
> /* returns with log_tree_root freed on success */
> ret = btrfs_recover_log_trees(log_tree_root);
> if (ret) {
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 10a980c..ea92671 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -7103,8 +7103,10 @@ static noinline int do_walk_down(struct btrfs_trans_handle *trans,
> if (reada && level == 1)
> reada_walk_down(trans, root, wc, path);
> next = read_tree_block(root, bytenr, blocksize, generation);
> - if (!next)
> + if (!next || !extent_buffer_uptodate(next)) {
> + free_extent_buffer(next);
> return -EIO;
> + }
> btrfs_tree_lock(next);
> btrfs_set_lock_blocking(next);
> }
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index a5955e8..ed568e3 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -1771,7 +1771,11 @@ again:
>
> eb = read_tree_block(dest, old_bytenr, blocksize,
> old_ptr_gen);
> - BUG_ON(!eb);
> + if (!eb || !extent_buffer_uptodate(eb)) {
> + ret = (!eb) ? -ENOMEM : -EIO;
> + free_extent_buffer(eb);
> + return ret;
> + }
> btrfs_tree_lock(eb);
> if (cow) {
> ret = btrfs_cow_block(trans, dest, eb, parent,
> @@ -1924,6 +1928,10 @@ int walk_down_reloc_tree(struct btrfs_root *root, struct btrfs_path *path,
> bytenr = btrfs_node_blockptr(eb, path->slots[i]);
> blocksize = btrfs_level_size(root, i - 1);
> eb = read_tree_block(root, bytenr, blocksize, ptr_gen);
> + if (!eb || !extent_buffer_uptodate(eb)) {
> + free_extent_buffer(eb);
> + return -EIO;
> + }
> BUG_ON(btrfs_header_level(eb) != i - 1);
> path->nodes[i - 1] = eb;
> path->slots[i - 1] = 0;
> @@ -2601,7 +2609,8 @@ static int do_relocation(struct btrfs_trans_handle *trans,
> blocksize = btrfs_level_size(root, node->level);
> generation = btrfs_node_ptr_generation(upper->eb, slot);
> eb = read_tree_block(root, bytenr, blocksize, generation);
> - if (!eb) {
> + if (!eb || !extent_buffer_uptodate(eb)) {
> + free_extent_buffer(eb);
> err = -EIO;
> goto next;
> }
> @@ -2762,7 +2771,10 @@ static int get_tree_block_key(struct reloc_control *rc,
> BUG_ON(block->key_ready);
> eb = read_tree_block(rc->extent_root, block->bytenr,
> block->key.objectid, block->key.offset);
> - BUG_ON(!eb);
> + if (!eb || !extent_buffer_uptodate(eb)) {
> + free_extent_buffer(eb);
> + return -EIO;
> + }
> WARN_ON(btrfs_header_level(eb) != block->level);
> if (block->level == 0)
> btrfs_item_key_to_cpu(eb, &block->key, 0);
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
