* [PATCH 0/2] Remove v0 extent support
@ 2018-06-25 8:24 Nikolay Borisov
2018-06-25 8:24 ` [PATCH 1/2] btrfs: Remove V0 " Nikolay Borisov
2018-06-25 8:24 ` [PATCH 2/2] btrfs: Add graceful handling of V0 extents Nikolay Borisov
0 siblings, 2 replies; 14+ messages in thread
From: Nikolay Borisov @ 2018-06-25 8:24 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
It's unlikely there is anyone using that or even if they are they have bigger
problems than this patchset :). After all, v0 was introduced 9 years ago and
it was already conditionally compiled by the time BTRFS was merged in the
upstream kernel. The patches themselves are really simple - patch 1 removes
all the code within ifdef guards. Patch 2, in turn, ads graceful handling by
aborting transaction where it makes sense or calling btrfs_handle_fs_error and
printing an informative error message.
Nikolay Borisov (2):
btrfs: Remove V0 extent support
btrfs: Add graceful handling of V0 extents
fs/btrfs/ctree.c | 6 +-
fs/btrfs/ctree.h | 2 -
fs/btrfs/extent-tree.c | 239 +++++++------------------------------------------
fs/btrfs/print-tree.c | 35 ++------
fs/btrfs/relocation.c | 181 ++++++-------------------------------
5 files changed, 70 insertions(+), 393 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] btrfs: Remove V0 extent support
2018-06-25 8:24 [PATCH 0/2] Remove v0 extent support Nikolay Borisov
@ 2018-06-25 8:24 ` Nikolay Borisov
2018-06-25 8:24 ` [PATCH 2/2] btrfs: Add graceful handling of V0 extents Nikolay Borisov
1 sibling, 0 replies; 14+ messages in thread
From: Nikolay Borisov @ 2018-06-25 8:24 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
The v0 compat code was introduced in commit 5d4f98a28c7d
("Btrfs: Mixed back reference (FORWARD ROLLING FORMAT CHANGE)") 9
years ago, which was merged in 2.6.31. This means that the code is
there to support filesystems which are _VERY_ old and if you are using
btrfs on such an old kernel, you have much bigger problems. This coupled
with the fact that no one is likely testing/maintining this code likely
means it has bugs lurking. All things considered I think 43 kernel
releases later it's high time this remnant of the past got removed.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/ctree.c | 6 +-
fs/btrfs/ctree.h | 2 -
fs/btrfs/extent-tree.c | 209 +------------------------------------------------
fs/btrfs/print-tree.c | 30 +------
fs/btrfs/relocation.c | 151 +----------------------------------
5 files changed, 4 insertions(+), 394 deletions(-)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 6879697520d5..d1273ec2e0d5 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -888,11 +888,7 @@ int btrfs_block_can_be_shared(struct btrfs_root *root,
btrfs_root_last_snapshot(&root->root_item) ||
btrfs_header_flag(buf, BTRFS_HEADER_FLAG_RELOC)))
return 1;
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- if (test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
- btrfs_header_backref_rev(buf) < BTRFS_MIXED_BACKREF_REV)
- return 1;
-#endif
+
return 0;
}
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index e671a1fcbbec..bc52bf7ac572 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -55,8 +55,6 @@ struct btrfs_ordered_sum;
#define BTRFS_OLDEST_GENERATION 0ULL
-#define BTRFS_COMPAT_EXTENT_TREE_V0
-
/*
* the max metadata block size. This limit is somewhat artificial,
* but the memmove costs go through the roof for larger blocks.
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 0f7e797236dc..4129831523a2 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -870,17 +870,7 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
num_refs = btrfs_extent_refs(leaf, ei);
extent_flags = btrfs_extent_flags(leaf, ei);
} else {
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- struct btrfs_extent_item_v0 *ei0;
- BUG_ON(item_size != sizeof(*ei0));
- ei0 = btrfs_item_ptr(leaf, path->slots[0],
- struct btrfs_extent_item_v0);
- num_refs = btrfs_extent_refs_v0(leaf, ei0);
- /* FIXME: this isn't correct for data */
- extent_flags = BTRFS_BLOCK_FLAG_FULL_BACKREF;
-#else
BUG();
-#endif
}
BUG_ON(num_refs == 0);
} else {
@@ -1039,89 +1029,6 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
* tree block info structure.
*/
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
-static int convert_extent_item_v0(struct btrfs_trans_handle *trans,
- struct btrfs_fs_info *fs_info,
- struct btrfs_path *path,
- u64 owner, u32 extra_size)
-{
- struct btrfs_root *root = fs_info->extent_root;
- struct btrfs_extent_item *item;
- struct btrfs_extent_item_v0 *ei0;
- struct btrfs_extent_ref_v0 *ref0;
- struct btrfs_tree_block_info *bi;
- struct extent_buffer *leaf;
- struct btrfs_key key;
- struct btrfs_key found_key;
- u32 new_size = sizeof(*item);
- u64 refs;
- int ret;
-
- leaf = path->nodes[0];
- BUG_ON(btrfs_item_size_nr(leaf, path->slots[0]) != sizeof(*ei0));
-
- btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
- ei0 = btrfs_item_ptr(leaf, path->slots[0],
- struct btrfs_extent_item_v0);
- refs = btrfs_extent_refs_v0(leaf, ei0);
-
- if (owner == (u64)-1) {
- while (1) {
- if (path->slots[0] >= btrfs_header_nritems(leaf)) {
- ret = btrfs_next_leaf(root, path);
- if (ret < 0)
- return ret;
- BUG_ON(ret > 0); /* Corruption */
- leaf = path->nodes[0];
- }
- btrfs_item_key_to_cpu(leaf, &found_key,
- path->slots[0]);
- BUG_ON(key.objectid != found_key.objectid);
- if (found_key.type != BTRFS_EXTENT_REF_V0_KEY) {
- path->slots[0]++;
- continue;
- }
- ref0 = btrfs_item_ptr(leaf, path->slots[0],
- struct btrfs_extent_ref_v0);
- owner = btrfs_ref_objectid_v0(leaf, ref0);
- break;
- }
- }
- btrfs_release_path(path);
-
- if (owner < BTRFS_FIRST_FREE_OBJECTID)
- new_size += sizeof(*bi);
-
- new_size -= sizeof(*ei0);
- ret = btrfs_search_slot(trans, root, &key, path,
- new_size + extra_size, 1);
- if (ret < 0)
- return ret;
- BUG_ON(ret); /* Corruption */
-
- btrfs_extend_item(fs_info, path, new_size);
-
- leaf = path->nodes[0];
- item = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
- btrfs_set_extent_refs(leaf, item, refs);
- /* FIXME: get real generation */
- btrfs_set_extent_generation(leaf, item, 0);
- if (owner < BTRFS_FIRST_FREE_OBJECTID) {
- btrfs_set_extent_flags(leaf, item,
- BTRFS_EXTENT_FLAG_TREE_BLOCK |
- BTRFS_BLOCK_FLAG_FULL_BACKREF);
- bi = (struct btrfs_tree_block_info *)(item + 1);
- /* FIXME: get first key of the block */
- memzero_extent_buffer(leaf, (unsigned long)bi, sizeof(*bi));
- btrfs_set_tree_block_level(leaf, bi, (int)owner);
- } else {
- btrfs_set_extent_flags(leaf, item, BTRFS_EXTENT_FLAG_DATA);
- }
- btrfs_mark_buffer_dirty(leaf);
- return 0;
-}
-#endif
-
/*
* is_data == BTRFS_REF_TYPE_BLOCK, tree block type is required,
* is_data == BTRFS_REF_TYPE_DATA, data type is requried,
@@ -1251,17 +1158,6 @@ static noinline int lookup_extent_data_ref(struct btrfs_trans_handle *trans,
if (parent) {
if (!ret)
return 0;
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- key.type = BTRFS_EXTENT_REF_V0_KEY;
- btrfs_release_path(path);
- ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
- if (ret < 0) {
- err = ret;
- goto fail;
- }
- if (!ret)
- return 0;
-#endif
goto fail;
}
@@ -1406,13 +1302,6 @@ static noinline int remove_extent_data_ref(struct btrfs_trans_handle *trans,
ref2 = btrfs_item_ptr(leaf, path->slots[0],
struct btrfs_shared_data_ref);
num_refs = btrfs_shared_data_ref_count(leaf, ref2);
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
- struct btrfs_extent_ref_v0 *ref0;
- ref0 = btrfs_item_ptr(leaf, path->slots[0],
- struct btrfs_extent_ref_v0);
- num_refs = btrfs_ref_count_v0(leaf, ref0);
-#endif
} else {
BUG();
}
@@ -1428,14 +1317,6 @@ static noinline int remove_extent_data_ref(struct btrfs_trans_handle *trans,
btrfs_set_extent_data_ref_count(leaf, ref1, num_refs);
else if (key.type == BTRFS_SHARED_DATA_REF_KEY)
btrfs_set_shared_data_ref_count(leaf, ref2, num_refs);
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- else {
- struct btrfs_extent_ref_v0 *ref0;
- ref0 = btrfs_item_ptr(leaf, path->slots[0],
- struct btrfs_extent_ref_v0);
- btrfs_set_ref_count_v0(leaf, ref0, num_refs);
- }
-#endif
btrfs_mark_buffer_dirty(leaf);
}
return ret;
@@ -1475,13 +1356,6 @@ static noinline u32 extent_data_ref_count(struct btrfs_path *path,
ref2 = btrfs_item_ptr(leaf, path->slots[0],
struct btrfs_shared_data_ref);
num_refs = btrfs_shared_data_ref_count(leaf, ref2);
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
- struct btrfs_extent_ref_v0 *ref0;
- ref0 = btrfs_item_ptr(leaf, path->slots[0],
- struct btrfs_extent_ref_v0);
- num_refs = btrfs_ref_count_v0(leaf, ref0);
-#endif
} else {
WARN_ON(1);
}
@@ -1510,15 +1384,6 @@ static noinline int lookup_tree_block_ref(struct btrfs_trans_handle *trans,
ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
if (ret > 0)
ret = -ENOENT;
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- if (ret == -ENOENT && parent) {
- btrfs_release_path(path);
- key.type = BTRFS_EXTENT_REF_V0_KEY;
- ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
- if (ret > 0)
- ret = -ENOENT;
- }
-#endif
return ret;
}
@@ -1684,22 +1549,6 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- if (item_size < sizeof(*ei)) {
- if (!insert) {
- err = -ENOENT;
- goto out;
- }
- ret = convert_extent_item_v0(trans, fs_info, path, owner,
- extra_size);
- if (ret < 0) {
- err = ret;
- goto out;
- }
- leaf = path->nodes[0];
- item_size = btrfs_item_size_nr(leaf, path->slots[0]);
- }
-#endif
BUG_ON(item_size < sizeof(*ei));
ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
@@ -2433,17 +2282,6 @@ static int run_delayed_extent_op(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- if (item_size < sizeof(*ei)) {
- ret = convert_extent_item_v0(trans, fs_info, path, (u64)-1, 0);
- if (ret < 0) {
- err = ret;
- goto out;
- }
- leaf = path->nodes[0];
- item_size = btrfs_item_size_nr(leaf, path->slots[0]);
- }
-#endif
BUG_ON(item_size < sizeof(*ei));
ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
__run_delayed_extent_op(extent_op, leaf, ei);
@@ -3258,12 +3096,6 @@ static noinline int check_committed_ref(struct btrfs_root *root,
ret = 1;
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- if (item_size < sizeof(*ei)) {
- WARN_ON(item_size != sizeof(struct btrfs_extent_item_v0));
- goto out;
- }
-#endif
ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
if (item_size != sizeof(*ei) +
@@ -6907,11 +6739,7 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
break;
extent_slot--;
}
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- item_size = btrfs_item_size_nr(path->nodes[0], extent_slot);
- if (found_extent && item_size < sizeof(*ei))
- found_extent = 0;
-#endif
+
if (!found_extent) {
BUG_ON(iref);
ret = remove_extent_backref(trans, info, path, NULL,
@@ -6987,41 +6815,6 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, extent_slot);
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- if (item_size < sizeof(*ei)) {
- BUG_ON(found_extent || extent_slot != path->slots[0]);
- ret = convert_extent_item_v0(trans, info, path, owner_objectid,
- 0);
- if (ret < 0) {
- btrfs_abort_transaction(trans, ret);
- goto out;
- }
-
- btrfs_release_path(path);
- path->leave_spinning = 1;
-
- key.objectid = bytenr;
- key.type = BTRFS_EXTENT_ITEM_KEY;
- key.offset = num_bytes;
-
- ret = btrfs_search_slot(trans, extent_root, &key, path,
- -1, 1);
- if (ret) {
- btrfs_err(info,
- "umm, got %d back from search, was looking for %llu",
- ret, bytenr);
- btrfs_print_leaf(path->nodes[0]);
- }
- if (ret < 0) {
- btrfs_abort_transaction(trans, ret);
- goto out;
- }
-
- extent_slot = path->slots[0];
- leaf = path->nodes[0];
- item_size = btrfs_item_size_nr(leaf, extent_slot);
- }
-#endif
BUG_ON(item_size < sizeof(*ei));
ei = btrfs_item_ptr(leaf, extent_slot,
struct btrfs_extent_item);
diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
index a4e11cf04671..b03040b84fe9 100644
--- a/fs/btrfs/print-tree.c
+++ b/fs/btrfs/print-tree.c
@@ -52,18 +52,8 @@ static void print_extent_item(struct extent_buffer *eb, int slot, int type)
u64 offset;
int ref_index = 0;
- if (item_size < sizeof(*ei)) {
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- struct btrfs_extent_item_v0 *ei0;
- BUG_ON(item_size != sizeof(*ei0));
- ei0 = btrfs_item_ptr(eb, slot, struct btrfs_extent_item_v0);
- pr_info("\t\textent refs %u\n",
- btrfs_extent_refs_v0(eb, ei0));
- return;
-#else
+ if (item_size < sizeof(*ei))
BUG();
-#endif
- }
ei = btrfs_item_ptr(eb, slot, struct btrfs_extent_item);
flags = btrfs_extent_flags(eb, ei);
@@ -133,20 +123,6 @@ static void print_extent_item(struct extent_buffer *eb, int slot, int type)
WARN_ON(ptr > end);
}
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
-static void print_extent_ref_v0(struct extent_buffer *eb, int slot)
-{
- struct btrfs_extent_ref_v0 *ref0;
-
- ref0 = btrfs_item_ptr(eb, slot, struct btrfs_extent_ref_v0);
- printk("\t\textent back ref root %llu gen %llu owner %llu num_refs %lu\n",
- btrfs_ref_root_v0(eb, ref0),
- btrfs_ref_generation_v0(eb, ref0),
- btrfs_ref_objectid_v0(eb, ref0),
- (unsigned long)btrfs_ref_count_v0(eb, ref0));
-}
-#endif
-
static void print_uuid_item(struct extent_buffer *l, unsigned long offset,
u32 item_size)
{
@@ -280,11 +256,7 @@ void btrfs_print_leaf(struct extent_buffer *l)
btrfs_file_extent_ram_bytes(l, fi));
break;
case BTRFS_EXTENT_REF_V0_KEY:
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- print_extent_ref_v0(l, i);
-#else
BUG();
-#endif
break;
case BTRFS_BLOCK_GROUP_ITEM_KEY:
bi = btrfs_item_ptr(l, i,
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 879b76fa881a..52c1e3e6802d 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -586,29 +586,6 @@ static struct btrfs_root *read_fs_root(struct btrfs_fs_info *fs_info,
return btrfs_get_fs_root(fs_info, &key, false);
}
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
-static noinline_for_stack
-struct btrfs_root *find_tree_root(struct reloc_control *rc,
- struct extent_buffer *leaf,
- struct btrfs_extent_ref_v0 *ref0)
-{
- struct btrfs_root *root;
- u64 root_objectid = btrfs_ref_root_v0(leaf, ref0);
- u64 generation = btrfs_ref_generation_v0(leaf, ref0);
-
- BUG_ON(root_objectid == BTRFS_TREE_RELOC_OBJECTID);
-
- root = read_fs_root(rc->extent_root->fs_info, root_objectid);
- BUG_ON(IS_ERR(root));
-
- if (test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
- generation != btrfs_root_generation(&root->root_item))
- return NULL;
-
- return root;
-}
-#endif
-
static noinline_for_stack
int find_inline_backref(struct extent_buffer *leaf, int slot,
unsigned long *ptr, unsigned long *end)
@@ -621,12 +598,6 @@ int find_inline_backref(struct extent_buffer *leaf, int slot,
btrfs_item_key_to_cpu(leaf, &key, slot);
item_size = btrfs_item_size_nr(leaf, slot);
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- if (item_size < sizeof(*ei)) {
- WARN_ON(item_size != sizeof(struct btrfs_extent_item_v0));
- return 1;
- }
-#endif
ei = btrfs_item_ptr(leaf, slot, struct btrfs_extent_item);
WARN_ON(!(btrfs_extent_flags(leaf, ei) &
BTRFS_EXTENT_FLAG_TREE_BLOCK));
@@ -811,29 +782,8 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
goto next;
}
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- if (key.type == BTRFS_SHARED_BLOCK_REF_KEY ||
- key.type == BTRFS_EXTENT_REF_V0_KEY) {
- if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
- struct btrfs_extent_ref_v0 *ref0;
- ref0 = btrfs_item_ptr(eb, path1->slots[0],
- struct btrfs_extent_ref_v0);
- if (key.objectid == key.offset) {
- root = find_tree_root(rc, eb, ref0);
- if (root && !should_ignore_root(root))
- cur->root = root;
- else
- list_add(&cur->list, &useless);
- break;
- }
- if (is_cowonly_root(btrfs_ref_root_v0(eb,
- ref0)))
- cur->cowonly = 1;
- }
-#else
ASSERT(key.type != BTRFS_EXTENT_REF_V0_KEY);
if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
-#endif
if (key.objectid == key.offset) {
/*
* only root blocks of reloc trees use
@@ -3333,48 +3283,6 @@ int relocate_data_extent(struct inode *inode, struct btrfs_key *extent_key,
return 0;
}
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
-static int get_ref_objectid_v0(struct reloc_control *rc,
- struct btrfs_path *path,
- struct btrfs_key *extent_key,
- u64 *ref_objectid, int *path_change)
-{
- struct btrfs_key key;
- struct extent_buffer *leaf;
- struct btrfs_extent_ref_v0 *ref0;
- int ret;
- int slot;
-
- leaf = path->nodes[0];
- slot = path->slots[0];
- while (1) {
- if (slot >= btrfs_header_nritems(leaf)) {
- ret = btrfs_next_leaf(rc->extent_root, path);
- if (ret < 0)
- return ret;
- BUG_ON(ret > 0);
- leaf = path->nodes[0];
- slot = path->slots[0];
- if (path_change)
- *path_change = 1;
- }
- btrfs_item_key_to_cpu(leaf, &key, slot);
- if (key.objectid != extent_key->objectid)
- return -ENOENT;
-
- if (key.type != BTRFS_EXTENT_REF_V0_KEY) {
- slot++;
- continue;
- }
- ref0 = btrfs_item_ptr(leaf, slot,
- struct btrfs_extent_ref_v0);
- *ref_objectid = btrfs_ref_objectid_v0(leaf, ref0);
- break;
- }
- return 0;
-}
-#endif
-
/*
* helper to add a tree block to the list.
* the major work is getting the generation and level of the block
@@ -3408,22 +3316,7 @@ static int add_tree_block(struct reloc_control *rc,
}
generation = btrfs_extent_generation(eb, ei);
} else {
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- u64 ref_owner;
- int ret;
-
- BUG_ON(item_size != sizeof(struct btrfs_extent_item_v0));
- ret = get_ref_objectid_v0(rc, path, extent_key,
- &ref_owner, NULL);
- if (ret < 0)
- return ret;
- BUG_ON(ref_owner >= BTRFS_MAX_LEVEL);
- level = (int)ref_owner;
- /* FIXME: get real generation */
- generation = 0;
-#else
BUG();
-#endif
}
btrfs_release_path(path);
@@ -3781,12 +3674,7 @@ int add_data_references(struct reloc_control *rc,
eb = path->nodes[0];
ptr = btrfs_item_ptr_offset(eb, path->slots[0]);
end = ptr + btrfs_item_size_nr(eb, path->slots[0]);
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- if (ptr + sizeof(struct btrfs_extent_item_v0) == end)
- ptr = end;
- else
-#endif
- ptr += sizeof(struct btrfs_extent_item);
+ ptr += sizeof(struct btrfs_extent_item);
while (ptr < end) {
iref = (struct btrfs_extent_inline_ref *)ptr;
@@ -3832,13 +3720,8 @@ int add_data_references(struct reloc_control *rc,
if (key.objectid != extent_key->objectid)
break;
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- if (key.type == BTRFS_SHARED_DATA_REF_KEY ||
- key.type == BTRFS_EXTENT_REF_V0_KEY) {
-#else
BUG_ON(key.type == BTRFS_EXTENT_REF_V0_KEY);
if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
-#endif
ret = __add_tree_block(rc, key.offset, blocksize,
blocks);
} else if (key.type == BTRFS_EXTENT_DATA_REF_KEY) {
@@ -4086,39 +3969,7 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
BUG_ON(ret);
} else {
-#ifdef BTRFS_COMPAT_EXTENT_TREE_V0
- u64 ref_owner;
- int path_change = 0;
-
- BUG_ON(item_size !=
- sizeof(struct btrfs_extent_item_v0));
- ret = get_ref_objectid_v0(rc, path, &key, &ref_owner,
- &path_change);
- if (ret < 0) {
- err = ret;
- break;
- }
- if (ref_owner < BTRFS_FIRST_FREE_OBJECTID)
- flags = BTRFS_EXTENT_FLAG_TREE_BLOCK;
- else
- flags = BTRFS_EXTENT_FLAG_DATA;
-
- if (path_change) {
- btrfs_release_path(path);
-
- path->search_commit_root = 1;
- path->skip_locking = 1;
- ret = btrfs_search_slot(NULL, rc->extent_root,
- &key, path, 0, 0);
- if (ret < 0) {
- err = ret;
- break;
- }
- BUG_ON(ret > 0);
- }
-#else
BUG();
-#endif
}
if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK) {
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] btrfs: Add graceful handling of V0 extents
2018-06-25 8:24 [PATCH 0/2] Remove v0 extent support Nikolay Borisov
2018-06-25 8:24 ` [PATCH 1/2] btrfs: Remove V0 " Nikolay Borisov
@ 2018-06-25 8:24 ` Nikolay Borisov
2018-06-25 15:21 ` David Sterba
1 sibling, 1 reply; 14+ messages in thread
From: Nikolay Borisov @ 2018-06-25 8:24 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
Following the removal of the v0 handling code let's be courteous and
print an error message when such extents are handled. In the cases
where we have a transaction just abort it, otherwise just call
btrfs_handle_fs_error. Both cases result in the FS being re-mounted RO.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/extent-tree.c | 44 ++++++++++++++++++++++++++++++++++++++++----
fs/btrfs/print-tree.c | 11 ++++++++---
fs/btrfs/relocation.c | 36 +++++++++++++++++++++++++++++++-----
3 files changed, 79 insertions(+), 12 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 4129831523a2..131773528683 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -870,8 +870,17 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
num_refs = btrfs_extent_refs(leaf, ei);
extent_flags = btrfs_extent_flags(leaf, ei);
} else {
- BUG();
+ ret = -EINVAL;
+ btrfs_err(fs_info,
+ "Unsupported V0 extent filesystem detected. Aborting. Please re-create your filesystem with a newer kernel");
+ if (trans)
+ btrfs_abort_transaction(trans, ret);
+ else
+ btrfs_handle_fs_error(fs_info, ret, NULL);
+
+ goto out_free;
}
+
BUG_ON(num_refs == 0);
} else {
num_refs = 0;
@@ -1302,6 +1311,11 @@ static noinline int remove_extent_data_ref(struct btrfs_trans_handle *trans,
ref2 = btrfs_item_ptr(leaf, path->slots[0],
struct btrfs_shared_data_ref);
num_refs = btrfs_shared_data_ref_count(leaf, ref2);
+ } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+ btrfs_err(fs_info,
+ "Unsupported V0 extent filesystem detected. Aborting. Please re-create your filesystem with a newer kernel");
+ btrfs_abort_transaction(trans, -EINVAL);
+ return -EINVAL;
} else {
BUG();
}
@@ -1334,6 +1348,8 @@ static noinline u32 extent_data_ref_count(struct btrfs_path *path,
leaf = path->nodes[0];
btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
+
+ BUG_ON(key.type == BTRFS_EXTENT_REF_V0_KEY);
if (iref) {
/*
* If type is invalid, we should have bailed out earlier than
@@ -1549,7 +1565,13 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
- BUG_ON(item_size < sizeof(*ei));
+ if (item_size < sizeof(*ei)) {
+ err = -EINVAL;
+ btrfs_err(fs_info,
+ "Unsupported V0 extent filesystem detected. Aborting. Please re-create your filesystem with a newer kernel");
+ btrfs_abort_transaction(trans, err);
+ goto out;
+ }
ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
flags = btrfs_extent_flags(leaf, ei);
@@ -2282,7 +2304,15 @@ static int run_delayed_extent_op(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
- BUG_ON(item_size < sizeof(*ei));
+
+ if (item_size < sizeof(*ei)) {
+ err = -EINVAL;
+ btrfs_err(fs_info,
+ "Unsupported V0 extent filesystem detected. Aborting. Please re-create your filesystem with a newer kernel");
+ btrfs_abort_transaction(trans, err);
+ goto out;
+ }
+
ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
__run_delayed_extent_op(extent_op, leaf, ei);
@@ -6815,7 +6845,13 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, extent_slot);
- BUG_ON(item_size < sizeof(*ei));
+ if (item_size < sizeof(*ei)) {
+ ret = -EINVAL;
+ btrfs_err(info,
+ "Unsupported V0 extent filesystem detected. Aborting. Please re-create your filesystem with a newer kernel");
+ btrfs_abort_transaction(trans, ret);
+ goto out;
+ }
ei = btrfs_item_ptr(leaf, extent_slot,
struct btrfs_extent_item);
if (owner_objectid < BTRFS_FIRST_FREE_OBJECTID &&
diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
index b03040b84fe9..cd4387fc766c 100644
--- a/fs/btrfs/print-tree.c
+++ b/fs/btrfs/print-tree.c
@@ -52,8 +52,11 @@ static void print_extent_item(struct extent_buffer *eb, int slot, int type)
u64 offset;
int ref_index = 0;
- if (item_size < sizeof(*ei))
- BUG();
+ if (item_size < sizeof(*ei)) {
+ btrfs_err(eb->fs_info,
+ "Unsupported V0 extent detected! Please recreate the filesystem on newer kernel\n");
+ btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
+ }
ei = btrfs_item_ptr(eb, slot, struct btrfs_extent_item);
flags = btrfs_extent_flags(eb, ei);
@@ -256,7 +259,9 @@ void btrfs_print_leaf(struct extent_buffer *l)
btrfs_file_extent_ram_bytes(l, fi));
break;
case BTRFS_EXTENT_REF_V0_KEY:
- BUG();
+ btrfs_err(fs_info,
+ "Unsupported V0 extent detected! Please recreate the filesystem on newer kernel\n");
+ btrfs_handle_fs_error(fs_info, -EINVAL, NULL);
break;
case BTRFS_BLOCK_GROUP_ITEM_KEY:
bi = btrfs_item_ptr(l, i,
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 52c1e3e6802d..0491aec58bcc 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -598,6 +598,12 @@ int find_inline_backref(struct extent_buffer *leaf, int slot,
btrfs_item_key_to_cpu(leaf, &key, slot);
item_size = btrfs_item_size_nr(leaf, slot);
+ if (item_size < sizeof(*ei)) {
+ btrfs_err(leaf->fs_info,
+ "Unsupported V0 extent filesystem detected. Aborting. Please re-create your filesystem with a newer kernel");
+ btrfs_handle_fs_error(leaf->fs_info, -EINVAL, NULL);
+ return 1;
+ }
ei = btrfs_item_ptr(leaf, slot, struct btrfs_extent_item);
WARN_ON(!(btrfs_extent_flags(leaf, ei) &
BTRFS_EXTENT_FLAG_TREE_BLOCK));
@@ -782,8 +788,14 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
goto next;
}
- ASSERT(key.type != BTRFS_EXTENT_REF_V0_KEY);
- if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
+ if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+ err = -EINVAL;
+ btrfs_err(rc->extent_root->fs_info,
+ "Unsupported V0 extent detected! Please recreate the filesystem on newer kernel\n");
+ btrfs_handle_fs_error(rc->extent_root->fs_info, err,
+ NULL);
+ goto out;
+ } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
if (key.objectid == key.offset) {
/*
* only root blocks of reloc trees use
@@ -3315,6 +3327,11 @@ static int add_tree_block(struct reloc_control *rc,
level = (int)extent_key->offset;
}
generation = btrfs_extent_generation(eb, ei);
+ } else if (item_size == sizeof(struct btrfs_extent_item_v0)) {
+ btrfs_err(eb->fs_info,
+ "Unsupported V0 extent detected! Please recreate the filesystem on newer kernel\n");
+ btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
+ return -EINVAL;
} else {
BUG();
}
@@ -3720,8 +3737,12 @@ int add_data_references(struct reloc_control *rc,
if (key.objectid != extent_key->objectid)
break;
- BUG_ON(key.type == BTRFS_EXTENT_REF_V0_KEY);
- if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
+ if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+ btrfs_err(eb->fs_info,
+ "Unsupported V0 extent detected! Please recreate the filesystem on newer kernel\n");
+ btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
+ ret = -EINVAL;
+ } else if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
ret = __add_tree_block(rc, key.offset, blocksize,
blocks);
} else if (key.type == BTRFS_EXTENT_DATA_REF_KEY) {
@@ -3967,7 +3988,12 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
flags = btrfs_extent_flags(path->nodes[0], ei);
ret = check_extent_flags(flags);
BUG_ON(ret);
-
+ } else if (item_size == sizeof(struct btrfs_extent_item_v0)) {
+ err = -EINVAL;
+ btrfs_err(trans->fs_info,
+ "Unsupported V0 extent detected! Please recreate the filesystem on newer kernel\n");
+ btrfs_abort_transaction(trans, err);
+ break;
} else {
BUG();
}
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] btrfs: Add graceful handling of V0 extents
2018-06-25 8:24 ` [PATCH 2/2] btrfs: Add graceful handling of V0 extents Nikolay Borisov
@ 2018-06-25 15:21 ` David Sterba
2018-06-26 13:57 ` [PATCH v2] " Nikolay Borisov
0 siblings, 1 reply; 14+ messages in thread
From: David Sterba @ 2018-06-25 15:21 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
On Mon, Jun 25, 2018 at 11:24:50AM +0300, Nikolay Borisov wrote:
> Following the removal of the v0 handling code let's be courteous and
> print an error message when such extents are handled. In the cases
> where we have a transaction just abort it, otherwise just call
> btrfs_handle_fs_error. Both cases result in the FS being re-mounted RO.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/extent-tree.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> fs/btrfs/print-tree.c | 11 ++++++++---
> fs/btrfs/relocation.c | 36 +++++++++++++++++++++++++++++++-----
> 3 files changed, 79 insertions(+), 12 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 4129831523a2..131773528683 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -870,8 +870,17 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
> num_refs = btrfs_extent_refs(leaf, ei);
> extent_flags = btrfs_extent_flags(leaf, ei);
> } else {
> - BUG();
> + ret = -EINVAL;
> + btrfs_err(fs_info,
> + "Unsupported V0 extent filesystem detected. Aborting. Please re-create your filesystem with a newer kernel");
Can you please add a helper that prints the message so there aren't
several different copies of that? Also btrfs_err will add the last '\n',
so it's not needed.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] btrfs: Add graceful handling of V0 extents
2018-06-25 15:21 ` David Sterba
@ 2018-06-26 13:57 ` Nikolay Borisov
2018-06-26 14:17 ` David Sterba
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Nikolay Borisov @ 2018-06-26 13:57 UTC (permalink / raw)
To: dsterba; +Cc: linux-btrfs, Nikolay Borisov
Following the removal of the v0 handling code let's be courteous and
print an error message when such extents are handled. In the cases
where we have a transaction just abort it, otherwise just call
btrfs_handle_fs_error. Both cases result in the FS being re-mounted RO.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
V2:
* Replaced open-coded error printing with a helper function for consistency.
fs/btrfs/ctree.h | 7 +++++++
fs/btrfs/extent-tree.c | 39 +++++++++++++++++++++++++++++++++++----
fs/btrfs/print-tree.c | 9 ++++++---
fs/btrfs/relocation.c | 31 ++++++++++++++++++++++++++-----
4 files changed, 74 insertions(+), 12 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index bc52bf7ac572..629ae1977d2c 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3457,6 +3457,13 @@ static inline void assfail(char *expr, char *file, int line)
BUG();
}
+__cold
+static inline void btrfs_print_v0_err(struct btrfs_fs_info *fs_info)
+{
+ btrfs_err(fs_info,
+ "Unsupported V0 extent filesystem detected. Aborting... Please re-create your filesystem with a newer kernel");
+}
+
#define ASSERT(expr) \
(likely(expr) ? (void)0 : assfail(#expr, __FILE__, __LINE__))
#else
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 4129831523a2..dd3ef8699b67 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -870,8 +870,16 @@ int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
num_refs = btrfs_extent_refs(leaf, ei);
extent_flags = btrfs_extent_flags(leaf, ei);
} else {
- BUG();
+ ret = -EINVAL;
+ btrfs_print_v0_err(fs_info);
+ if (trans)
+ btrfs_abort_transaction(trans, ret);
+ else
+ btrfs_handle_fs_error(fs_info, ret, NULL);
+
+ goto out_free;
}
+
BUG_ON(num_refs == 0);
} else {
num_refs = 0;
@@ -1302,6 +1310,10 @@ static noinline int remove_extent_data_ref(struct btrfs_trans_handle *trans,
ref2 = btrfs_item_ptr(leaf, path->slots[0],
struct btrfs_shared_data_ref);
num_refs = btrfs_shared_data_ref_count(leaf, ref2);
+ } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+ btrfs_print_v0_err(fs_info);
+ btrfs_abort_transaction(trans, -EINVAL);
+ return -EINVAL;
} else {
BUG();
}
@@ -1334,6 +1346,8 @@ static noinline u32 extent_data_ref_count(struct btrfs_path *path,
leaf = path->nodes[0];
btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
+
+ BUG_ON(key.type == BTRFS_EXTENT_REF_V0_KEY);
if (iref) {
/*
* If type is invalid, we should have bailed out earlier than
@@ -1549,7 +1563,12 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
- BUG_ON(item_size < sizeof(*ei));
+ if (item_size < sizeof(*ei)) {
+ err = -EINVAL;
+ btrfs_print_v0_err(fs_info);
+ btrfs_abort_transaction(trans, err);
+ goto out;
+ }
ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
flags = btrfs_extent_flags(leaf, ei);
@@ -2282,7 +2301,14 @@ static int run_delayed_extent_op(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
- BUG_ON(item_size < sizeof(*ei));
+
+ if (item_size < sizeof(*ei)) {
+ err = -EINVAL;
+ btrfs_print_v0_err(fs_info);
+ btrfs_abort_transaction(trans, err);
+ goto out;
+ }
+
ei = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_extent_item);
__run_delayed_extent_op(extent_op, leaf, ei);
@@ -6815,7 +6841,12 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, extent_slot);
- BUG_ON(item_size < sizeof(*ei));
+ if (item_size < sizeof(*ei)) {
+ ret = -EINVAL;
+ btrfs_print_v0_err(info);
+ btrfs_abort_transaction(trans, ret);
+ goto out;
+ }
ei = btrfs_item_ptr(leaf, extent_slot,
struct btrfs_extent_item);
if (owner_objectid < BTRFS_FIRST_FREE_OBJECTID &&
diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
index b03040b84fe9..772560eecfa6 100644
--- a/fs/btrfs/print-tree.c
+++ b/fs/btrfs/print-tree.c
@@ -52,8 +52,10 @@ static void print_extent_item(struct extent_buffer *eb, int slot, int type)
u64 offset;
int ref_index = 0;
- if (item_size < sizeof(*ei))
- BUG();
+ if (item_size < sizeof(*ei)) {
+ btrfs_print_v0_err(eb->fs_info);
+ btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
+ }
ei = btrfs_item_ptr(eb, slot, struct btrfs_extent_item);
flags = btrfs_extent_flags(eb, ei);
@@ -256,7 +258,8 @@ void btrfs_print_leaf(struct extent_buffer *l)
btrfs_file_extent_ram_bytes(l, fi));
break;
case BTRFS_EXTENT_REF_V0_KEY:
- BUG();
+ btrfs_print_v0_err(fs_info);
+ btrfs_handle_fs_error(fs_info, -EINVAL, NULL);
break;
case BTRFS_BLOCK_GROUP_ITEM_KEY:
bi = btrfs_item_ptr(l, i,
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 52c1e3e6802d..6fcd88c856d3 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -598,6 +598,11 @@ int find_inline_backref(struct extent_buffer *leaf, int slot,
btrfs_item_key_to_cpu(leaf, &key, slot);
item_size = btrfs_item_size_nr(leaf, slot);
+ if (item_size < sizeof(*ei)) {
+ btrfs_print_v0_err(leaf->fs_info);
+ btrfs_handle_fs_error(leaf->fs_info, -EINVAL, NULL);
+ return 1;
+ }
ei = btrfs_item_ptr(leaf, slot, struct btrfs_extent_item);
WARN_ON(!(btrfs_extent_flags(leaf, ei) &
BTRFS_EXTENT_FLAG_TREE_BLOCK));
@@ -782,8 +787,13 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
goto next;
}
- ASSERT(key.type != BTRFS_EXTENT_REF_V0_KEY);
- if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
+ if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+ err = -EINVAL;
+ btrfs_print_v0_err(rc->extent_root->fs_info);
+ btrfs_handle_fs_error(rc->extent_root->fs_info, err,
+ NULL);
+ goto out;
+ } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
if (key.objectid == key.offset) {
/*
* only root blocks of reloc trees use
@@ -3315,6 +3325,10 @@ static int add_tree_block(struct reloc_control *rc,
level = (int)extent_key->offset;
}
generation = btrfs_extent_generation(eb, ei);
+ } else if (item_size == sizeof(struct btrfs_extent_item_v0)) {
+ btrfs_print_v0_err(eb->fs_info);
+ btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
+ return -EINVAL;
} else {
BUG();
}
@@ -3720,8 +3734,11 @@ int add_data_references(struct reloc_control *rc,
if (key.objectid != extent_key->objectid)
break;
- BUG_ON(key.type == BTRFS_EXTENT_REF_V0_KEY);
- if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
+ if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+ btrfs_print_v0_err(eb->fs_info);
+ btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
+ ret = -EINVAL;
+ } else if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
ret = __add_tree_block(rc, key.offset, blocksize,
blocks);
} else if (key.type == BTRFS_EXTENT_DATA_REF_KEY) {
@@ -3967,7 +3984,11 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
flags = btrfs_extent_flags(path->nodes[0], ei);
ret = check_extent_flags(flags);
BUG_ON(ret);
-
+ } else if (item_size == sizeof(struct btrfs_extent_item_v0)) {
+ err = -EINVAL;
+ btrfs_print_v0_err(trans->fs_info);
+ btrfs_abort_transaction(trans, err);
+ break;
} else {
BUG();
}
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] btrfs: Add graceful handling of V0 extents
2018-06-26 13:57 ` [PATCH v2] " Nikolay Borisov
@ 2018-06-26 14:17 ` David Sterba
2018-06-27 13:12 ` Noah Massey
2018-06-26 14:24 ` [PATCH] btrfs: annotate unlikely branches after V0 extent type removal David Sterba
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: David Sterba @ 2018-06-26 14:17 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: dsterba, linux-btrfs
On Tue, Jun 26, 2018 at 04:57:36PM +0300, Nikolay Borisov wrote:
> Following the removal of the v0 handling code let's be courteous and
> print an error message when such extents are handled. In the cases
> where we have a transaction just abort it, otherwise just call
> btrfs_handle_fs_error. Both cases result in the FS being re-mounted RO.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> - ASSERT(key.type != BTRFS_EXTENT_REF_V0_KEY);
> - if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> + if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> + err = -EINVAL;
> + btrfs_print_v0_err(rc->extent_root->fs_info);
> + btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> + NULL);
> + goto out;
> + } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
The v0 check should be made last as it's not expected to happen. I'm
commiting with this diff
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -787,13 +787,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
goto next;
}
- if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
- err = -EINVAL;
- btrfs_print_v0_err(rc->extent_root->fs_info);
- btrfs_handle_fs_error(rc->extent_root->fs_info, err,
- NULL);
- goto out;
- } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
+ if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
if (key.objectid == key.offset) {
/*
* only root blocks of reloc trees use
@@ -838,6 +832,12 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
goto next;
} else if (key.type != BTRFS_TREE_BLOCK_REF_KEY) {
goto next;
+ } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+ err = -EINVAL;
+ btrfs_print_v0_err(rc->extent_root->fs_info);
+ btrfs_handle_fs_error(rc->extent_root->fs_info, err,
+ NULL);
+ goto out;
}
/* key.type == BTRFS_TREE_BLOCK_REF_KEY */
@@ -3734,11 +3734,7 @@ int add_data_references(struct reloc_control *rc,
if (key.objectid != extent_key->objectid)
break;
- if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
- btrfs_print_v0_err(eb->fs_info);
- btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
- ret = -EINVAL;
- } else if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
+ if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
ret = __add_tree_block(rc, key.offset, blocksize,
blocks);
} else if (key.type == BTRFS_EXTENT_DATA_REF_KEY) {
@@ -3746,6 +3742,10 @@ int add_data_references(struct reloc_control *rc,
struct btrfs_extent_data_ref);
ret = find_data_references(rc, extent_key,
eb, dref, blocks);
+ } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+ btrfs_print_v0_err(eb->fs_info);
+ btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
+ ret = -EINVAL;
} else {
ret = 0;
}
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] btrfs: annotate unlikely branches after V0 extent type removal
2018-06-26 13:57 ` [PATCH v2] " Nikolay Borisov
2018-06-26 14:17 ` David Sterba
@ 2018-06-26 14:24 ` David Sterba
2018-06-26 14:31 ` Nikolay Borisov
2018-06-26 16:05 ` [PATCH v2] btrfs: Add graceful handling of V0 extents kbuild test robot
2018-06-26 17:44 ` kbuild test robot
3 siblings, 1 reply; 14+ messages in thread
From: David Sterba @ 2018-06-26 14:24 UTC (permalink / raw)
To: linux-btrfs; +Cc: nborisov, David Sterba
The v0 extent type checks are the right case for the unlikely
annotations as we don't expect to ever see them, so let's give the
compiler some hint.
Signed-off-by: David Sterba <dsterba@suse.com>
---
fs/btrfs/extent-tree.c | 8 ++++----
fs/btrfs/print-tree.c | 2 +-
fs/btrfs/relocation.c | 8 ++++----
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 8713a1693dfc..d3f7afe3a548 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1310,7 +1310,7 @@ static noinline int remove_extent_data_ref(struct btrfs_trans_handle *trans,
ref2 = btrfs_item_ptr(leaf, path->slots[0],
struct btrfs_shared_data_ref);
num_refs = btrfs_shared_data_ref_count(leaf, ref2);
- } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+ } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
btrfs_print_v0_err(fs_info);
btrfs_abort_transaction(trans, -EINVAL);
return -EINVAL;
@@ -1563,7 +1563,7 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
- if (item_size < sizeof(*ei)) {
+ if (unlikely(item_size < sizeof(*ei))) {
err = -EINVAL;
btrfs_print_v0_err(fs_info);
btrfs_abort_transaction(trans, err);
@@ -2269,7 +2269,7 @@ static int run_delayed_extent_op(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, path->slots[0]);
- if (item_size < sizeof(*ei)) {
+ if (unlikely(item_size < sizeof(*ei))) {
err = -EINVAL;
btrfs_print_v0_err(fs_info);
btrfs_abort_transaction(trans, err);
@@ -6811,7 +6811,7 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
leaf = path->nodes[0];
item_size = btrfs_item_size_nr(leaf, extent_slot);
- if (item_size < sizeof(*ei)) {
+ if (unlikely(item_size < sizeof(*ei))) {
ret = -EINVAL;
btrfs_print_v0_err(info);
btrfs_abort_transaction(trans, ret);
diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
index 772560eecfa6..0dd44d161935 100644
--- a/fs/btrfs/print-tree.c
+++ b/fs/btrfs/print-tree.c
@@ -52,7 +52,7 @@ static void print_extent_item(struct extent_buffer *eb, int slot, int type)
u64 offset;
int ref_index = 0;
- if (item_size < sizeof(*ei)) {
+ if (unlikely(item_size < sizeof(*ei))) {
btrfs_print_v0_err(eb->fs_info);
btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
}
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index a8dddefaae99..381bd3c24249 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -832,7 +832,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
goto next;
} else if (key.type != BTRFS_TREE_BLOCK_REF_KEY) {
goto next;
- } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+ } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
err = -EINVAL;
btrfs_print_v0_err(rc->extent_root->fs_info);
btrfs_handle_fs_error(rc->extent_root->fs_info, err,
@@ -3325,7 +3325,7 @@ static int add_tree_block(struct reloc_control *rc,
level = (int)extent_key->offset;
}
generation = btrfs_extent_generation(eb, ei);
- } else if (item_size == sizeof(struct btrfs_extent_item_v0)) {
+ } else if (unlikely(item_size == sizeof(struct btrfs_extent_item_v0))) {
btrfs_print_v0_err(eb->fs_info);
btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
return -EINVAL;
@@ -3742,7 +3742,7 @@ int add_data_references(struct reloc_control *rc,
struct btrfs_extent_data_ref);
ret = find_data_references(rc, extent_key,
eb, dref, blocks);
- } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
+ } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
btrfs_print_v0_err(eb->fs_info);
btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
ret = -EINVAL;
@@ -3984,7 +3984,7 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
flags = btrfs_extent_flags(path->nodes[0], ei);
ret = check_extent_flags(flags);
BUG_ON(ret);
- } else if (item_size == sizeof(struct btrfs_extent_item_v0)) {
+ } else if (unlikely(item_size == sizeof(struct btrfs_extent_item_v0))) {
err = -EINVAL;
btrfs_print_v0_err(trans->fs_info);
btrfs_abort_transaction(trans, err);
--
2.17.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] btrfs: annotate unlikely branches after V0 extent type removal
2018-06-26 14:24 ` [PATCH] btrfs: annotate unlikely branches after V0 extent type removal David Sterba
@ 2018-06-26 14:31 ` Nikolay Borisov
2018-06-26 14:43 ` David Sterba
0 siblings, 1 reply; 14+ messages in thread
From: Nikolay Borisov @ 2018-06-26 14:31 UTC (permalink / raw)
To: David Sterba, linux-btrfs
On 26.06.2018 17:24, David Sterba wrote:
> The v0 extent type checks are the right case for the unlikely
> annotations as we don't expect to ever see them, so let's give the
> compiler some hint.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
The question is how helpful those unlikelies are in the generated code.
Also according to the comment about __cold annotation this already gives
the compiler a hint that any code leading to such a cold function is
already unlikely.
Anyways:
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/extent-tree.c | 8 ++++----
> fs/btrfs/print-tree.c | 2 +-
> fs/btrfs/relocation.c | 8 ++++----
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 8713a1693dfc..d3f7afe3a548 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -1310,7 +1310,7 @@ static noinline int remove_extent_data_ref(struct btrfs_trans_handle *trans,
> ref2 = btrfs_item_ptr(leaf, path->slots[0],
> struct btrfs_shared_data_ref);
> num_refs = btrfs_shared_data_ref_count(leaf, ref2);
> - } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> + } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
> btrfs_print_v0_err(fs_info);
> btrfs_abort_transaction(trans, -EINVAL);
> return -EINVAL;
> @@ -1563,7 +1563,7 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans,
>
> leaf = path->nodes[0];
> item_size = btrfs_item_size_nr(leaf, path->slots[0]);
> - if (item_size < sizeof(*ei)) {
> + if (unlikely(item_size < sizeof(*ei))) {
> err = -EINVAL;
> btrfs_print_v0_err(fs_info);
> btrfs_abort_transaction(trans, err);
> @@ -2269,7 +2269,7 @@ static int run_delayed_extent_op(struct btrfs_trans_handle *trans,
> leaf = path->nodes[0];
> item_size = btrfs_item_size_nr(leaf, path->slots[0]);
>
> - if (item_size < sizeof(*ei)) {
> + if (unlikely(item_size < sizeof(*ei))) {
> err = -EINVAL;
> btrfs_print_v0_err(fs_info);
> btrfs_abort_transaction(trans, err);
> @@ -6811,7 +6811,7 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
>
> leaf = path->nodes[0];
> item_size = btrfs_item_size_nr(leaf, extent_slot);
> - if (item_size < sizeof(*ei)) {
> + if (unlikely(item_size < sizeof(*ei))) {
> ret = -EINVAL;
> btrfs_print_v0_err(info);
> btrfs_abort_transaction(trans, ret);
> diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
> index 772560eecfa6..0dd44d161935 100644
> --- a/fs/btrfs/print-tree.c
> +++ b/fs/btrfs/print-tree.c
> @@ -52,7 +52,7 @@ static void print_extent_item(struct extent_buffer *eb, int slot, int type)
> u64 offset;
> int ref_index = 0;
>
> - if (item_size < sizeof(*ei)) {
> + if (unlikely(item_size < sizeof(*ei))) {
> btrfs_print_v0_err(eb->fs_info);
> btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
> }
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index a8dddefaae99..381bd3c24249 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -832,7 +832,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
> goto next;
> } else if (key.type != BTRFS_TREE_BLOCK_REF_KEY) {
> goto next;
> - } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> + } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
> err = -EINVAL;
> btrfs_print_v0_err(rc->extent_root->fs_info);
> btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> @@ -3325,7 +3325,7 @@ static int add_tree_block(struct reloc_control *rc,
> level = (int)extent_key->offset;
> }
> generation = btrfs_extent_generation(eb, ei);
> - } else if (item_size == sizeof(struct btrfs_extent_item_v0)) {
> + } else if (unlikely(item_size == sizeof(struct btrfs_extent_item_v0))) {
> btrfs_print_v0_err(eb->fs_info);
> btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
> return -EINVAL;
> @@ -3742,7 +3742,7 @@ int add_data_references(struct reloc_control *rc,
> struct btrfs_extent_data_ref);
> ret = find_data_references(rc, extent_key,
> eb, dref, blocks);
> - } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> + } else if (unlikely(key.type == BTRFS_EXTENT_REF_V0_KEY)) {
> btrfs_print_v0_err(eb->fs_info);
> btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
> ret = -EINVAL;
> @@ -3984,7 +3984,7 @@ static noinline_for_stack int relocate_block_group(struct reloc_control *rc)
> flags = btrfs_extent_flags(path->nodes[0], ei);
> ret = check_extent_flags(flags);
> BUG_ON(ret);
> - } else if (item_size == sizeof(struct btrfs_extent_item_v0)) {
> + } else if (unlikely(item_size == sizeof(struct btrfs_extent_item_v0))) {
> err = -EINVAL;
> btrfs_print_v0_err(trans->fs_info);
> btrfs_abort_transaction(trans, err);
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] btrfs: annotate unlikely branches after V0 extent type removal
2018-06-26 14:31 ` Nikolay Borisov
@ 2018-06-26 14:43 ` David Sterba
0 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2018-06-26 14:43 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: David Sterba, linux-btrfs
On Tue, Jun 26, 2018 at 05:31:40PM +0300, Nikolay Borisov wrote:
>
>
> On 26.06.2018 17:24, David Sterba wrote:
> > The v0 extent type checks are the right case for the unlikely
> > annotations as we don't expect to ever see them, so let's give the
> > compiler some hint.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
>
> The question is how helpful those unlikelies are in the generated code.
They typically lead to inverse condition (so the jump is not taken) and
the body of the loop is moved farther away.
> Also according to the comment about __cold annotation this already gives
> the compiler a hint that any code leading to such a cold function is
> already unlikely.
Yes, that's another hint. The condition annotation affects the whole
statement block regardless. I also think that the unlikely in this case
serves as a code documentation (as long as this is consistent with other
uses of likely/unlikely).
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] btrfs: Add graceful handling of V0 extents
2018-06-26 13:57 ` [PATCH v2] " Nikolay Borisov
2018-06-26 14:17 ` David Sterba
2018-06-26 14:24 ` [PATCH] btrfs: annotate unlikely branches after V0 extent type removal David Sterba
@ 2018-06-26 16:05 ` kbuild test robot
2018-06-26 17:12 ` David Sterba
2018-06-26 17:44 ` kbuild test robot
3 siblings, 1 reply; 14+ messages in thread
From: kbuild test robot @ 2018-06-26 16:05 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: kbuild-all, dsterba, linux-btrfs, Nikolay Borisov
[-- Attachment #1: Type: text/plain, Size: 6594 bytes --]
Hi Nikolay,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on next-20180626]
[cannot apply to btrfs/next v4.18-rc2 v4.18-rc1 v4.17 v4.18-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nikolay-Borisov/btrfs-Add-graceful-handling-of-V0-extents/20180626-231445
config: x86_64-randconfig-x015-201825 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
fs//btrfs/extent-tree.c: In function 'btrfs_lookup_extent_info':
>> fs//btrfs/extent-tree.c:871:4: error: implicit declaration of function 'btrfs_print_v0_err'; did you mean 'btrfs_print_tree'? [-Werror=implicit-function-declaration]
btrfs_print_v0_err(fs_info);
^~~~~~~~~~~~~~~~~~
btrfs_print_tree
fs//btrfs/extent-tree.c: In function 'remove_extent_data_ref':
>> fs//btrfs/extent-tree.c:1308:22: error: 'fs_info' undeclared (first use in this function); did you mean 'qc_info'?
btrfs_print_v0_err(fs_info);
^~~~~~~
qc_info
fs//btrfs/extent-tree.c:1308:22: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors
--
fs//btrfs/print-tree.c: In function 'print_extent_item':
>> fs//btrfs/print-tree.c:56:3: error: implicit declaration of function 'btrfs_print_v0_err'; did you mean 'btrfs_print_tree'? [-Werror=implicit-function-declaration]
btrfs_print_v0_err(eb->fs_info);
^~~~~~~~~~~~~~~~~~
btrfs_print_tree
cc1: some warnings being treated as errors
--
fs//btrfs/relocation.c: In function 'find_inline_backref':
>> fs//btrfs/relocation.c:602:3: error: implicit declaration of function 'btrfs_print_v0_err'; did you mean 'btrfs_print_tree'? [-Werror=implicit-function-declaration]
btrfs_print_v0_err(leaf->fs_info);
^~~~~~~~~~~~~~~~~~
btrfs_print_tree
cc1: some warnings being treated as errors
vim +871 fs//btrfs/extent-tree.c
794
795 /*
796 * helper function to lookup reference count and flags of a tree block.
797 *
798 * the head node for delayed ref is used to store the sum of all the
799 * reference count modifications queued up in the rbtree. the head
800 * node may also store the extent flags to set. This way you can check
801 * to see what the reference count and extent flags would be if all of
802 * the delayed refs are not processed.
803 */
804 int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
805 struct btrfs_fs_info *fs_info, u64 bytenr,
806 u64 offset, int metadata, u64 *refs, u64 *flags)
807 {
808 struct btrfs_delayed_ref_head *head;
809 struct btrfs_delayed_ref_root *delayed_refs;
810 struct btrfs_path *path;
811 struct btrfs_extent_item *ei;
812 struct extent_buffer *leaf;
813 struct btrfs_key key;
814 u32 item_size;
815 u64 num_refs;
816 u64 extent_flags;
817 int ret;
818
819 /*
820 * If we don't have skinny metadata, don't bother doing anything
821 * different
822 */
823 if (metadata && !btrfs_fs_incompat(fs_info, SKINNY_METADATA)) {
824 offset = fs_info->nodesize;
825 metadata = 0;
826 }
827
828 path = btrfs_alloc_path();
829 if (!path)
830 return -ENOMEM;
831
832 if (!trans) {
833 path->skip_locking = 1;
834 path->search_commit_root = 1;
835 }
836
837 search_again:
838 key.objectid = bytenr;
839 key.offset = offset;
840 if (metadata)
841 key.type = BTRFS_METADATA_ITEM_KEY;
842 else
843 key.type = BTRFS_EXTENT_ITEM_KEY;
844
845 ret = btrfs_search_slot(trans, fs_info->extent_root, &key, path, 0, 0);
846 if (ret < 0)
847 goto out_free;
848
849 if (ret > 0 && metadata && key.type == BTRFS_METADATA_ITEM_KEY) {
850 if (path->slots[0]) {
851 path->slots[0]--;
852 btrfs_item_key_to_cpu(path->nodes[0], &key,
853 path->slots[0]);
854 if (key.objectid == bytenr &&
855 key.type == BTRFS_EXTENT_ITEM_KEY &&
856 key.offset == fs_info->nodesize)
857 ret = 0;
858 }
859 }
860
861 if (ret == 0) {
862 leaf = path->nodes[0];
863 item_size = btrfs_item_size_nr(leaf, path->slots[0]);
864 if (item_size >= sizeof(*ei)) {
865 ei = btrfs_item_ptr(leaf, path->slots[0],
866 struct btrfs_extent_item);
867 num_refs = btrfs_extent_refs(leaf, ei);
868 extent_flags = btrfs_extent_flags(leaf, ei);
869 } else {
870 ret = -EINVAL;
> 871 btrfs_print_v0_err(fs_info);
872 if (trans)
873 btrfs_abort_transaction(trans, ret);
874 else
875 btrfs_handle_fs_error(fs_info, ret, NULL);
876
877 goto out_free;
878 }
879
880 BUG_ON(num_refs == 0);
881 } else {
882 num_refs = 0;
883 extent_flags = 0;
884 ret = 0;
885 }
886
887 if (!trans)
888 goto out;
889
890 delayed_refs = &trans->transaction->delayed_refs;
891 spin_lock(&delayed_refs->lock);
892 head = btrfs_find_delayed_ref_head(delayed_refs, bytenr);
893 if (head) {
894 if (!mutex_trylock(&head->mutex)) {
895 refcount_inc(&head->refs);
896 spin_unlock(&delayed_refs->lock);
897
898 btrfs_release_path(path);
899
900 /*
901 * Mutex was contended, block until it's released and try
902 * again
903 */
904 mutex_lock(&head->mutex);
905 mutex_unlock(&head->mutex);
906 btrfs_put_delayed_ref_head(head);
907 goto search_again;
908 }
909 spin_lock(&head->lock);
910 if (head->extent_op && head->extent_op->update_flags)
911 extent_flags |= head->extent_op->flags_to_set;
912 else
913 BUG_ON(num_refs == 0);
914
915 num_refs += head->ref_mod;
916 spin_unlock(&head->lock);
917 mutex_unlock(&head->mutex);
918 }
919 spin_unlock(&delayed_refs->lock);
920 out:
921 WARN_ON(num_refs == 0);
922 if (refs)
923 *refs = num_refs;
924 if (flags)
925 *flags = extent_flags;
926 out_free:
927 btrfs_free_path(path);
928 return ret;
929 }
930
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27668 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] btrfs: Add graceful handling of V0 extents
2018-06-26 16:05 ` [PATCH v2] btrfs: Add graceful handling of V0 extents kbuild test robot
@ 2018-06-26 17:12 ` David Sterba
0 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2018-06-26 17:12 UTC (permalink / raw)
To: kbuild test robot; +Cc: Nikolay Borisov, kbuild-all, dsterba, linux-btrfs
On Wed, Jun 27, 2018 at 12:05:39AM +0800, kbuild test robot wrote:
> Hi Nikolay,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on next-20180626]
> [cannot apply to btrfs/next v4.18-rc2 v4.18-rc1 v4.17 v4.18-rc2]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Nikolay-Borisov/btrfs-Add-graceful-handling-of-V0-extents/20180626-231445
> config: x86_64-randconfig-x015-201825 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
> fs//btrfs/extent-tree.c: In function 'btrfs_lookup_extent_info':
> >> fs//btrfs/extent-tree.c:871:4: error: implicit declaration of function 'btrfs_print_v0_err'; did you mean 'btrfs_print_tree'? [-Werror=implicit-function-declaration]
> btrfs_print_v0_err(fs_info);
> ^~~~~~~~~~~~~~~~~~
Fixed by using trans->fs_info in the original patch so the followup that
removes 'fs_info' does not break compilation. Updated for-next branch
pushed.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] btrfs: Add graceful handling of V0 extents
2018-06-26 13:57 ` [PATCH v2] " Nikolay Borisov
` (2 preceding siblings ...)
2018-06-26 16:05 ` [PATCH v2] btrfs: Add graceful handling of V0 extents kbuild test robot
@ 2018-06-26 17:44 ` kbuild test robot
3 siblings, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2018-06-26 17:44 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: kbuild-all, dsterba, linux-btrfs, Nikolay Borisov
[-- Attachment #1: Type: text/plain, Size: 6377 bytes --]
Hi Nikolay,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on next-20180626]
[cannot apply to btrfs/next v4.18-rc2 v4.18-rc1 v4.17 v4.18-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nikolay-Borisov/btrfs-Add-graceful-handling-of-V0-extents/20180626-231445
config: i386-randconfig-s1-201825 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
fs/btrfs/extent-tree.c: In function 'btrfs_lookup_extent_info':
>> fs/btrfs/extent-tree.c:871:4: error: implicit declaration of function 'btrfs_print_v0_err' [-Werror=implicit-function-declaration]
btrfs_print_v0_err(fs_info);
^~~~~~~~~~~~~~~~~~
fs/btrfs/extent-tree.c: In function 'remove_extent_data_ref':
>> fs/btrfs/extent-tree.c:1308:22: error: 'fs_info' undeclared (first use in this function)
btrfs_print_v0_err(fs_info);
^~~~~~~
fs/btrfs/extent-tree.c:1308:22: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors
--
fs/btrfs/print-tree.c: In function 'print_extent_item':
>> fs/btrfs/print-tree.c:56:3: error: implicit declaration of function 'btrfs_print_v0_err' [-Werror=implicit-function-declaration]
btrfs_print_v0_err(eb->fs_info);
^~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
--
fs/btrfs/relocation.c: In function 'find_inline_backref':
>> fs/btrfs/relocation.c:602:3: error: implicit declaration of function 'btrfs_print_v0_err' [-Werror=implicit-function-declaration]
btrfs_print_v0_err(leaf->fs_info);
^~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/btrfs_print_v0_err +871 fs/btrfs/extent-tree.c
794
795 /*
796 * helper function to lookup reference count and flags of a tree block.
797 *
798 * the head node for delayed ref is used to store the sum of all the
799 * reference count modifications queued up in the rbtree. the head
800 * node may also store the extent flags to set. This way you can check
801 * to see what the reference count and extent flags would be if all of
802 * the delayed refs are not processed.
803 */
804 int btrfs_lookup_extent_info(struct btrfs_trans_handle *trans,
805 struct btrfs_fs_info *fs_info, u64 bytenr,
806 u64 offset, int metadata, u64 *refs, u64 *flags)
807 {
808 struct btrfs_delayed_ref_head *head;
809 struct btrfs_delayed_ref_root *delayed_refs;
810 struct btrfs_path *path;
811 struct btrfs_extent_item *ei;
812 struct extent_buffer *leaf;
813 struct btrfs_key key;
814 u32 item_size;
815 u64 num_refs;
816 u64 extent_flags;
817 int ret;
818
819 /*
820 * If we don't have skinny metadata, don't bother doing anything
821 * different
822 */
823 if (metadata && !btrfs_fs_incompat(fs_info, SKINNY_METADATA)) {
824 offset = fs_info->nodesize;
825 metadata = 0;
826 }
827
828 path = btrfs_alloc_path();
829 if (!path)
830 return -ENOMEM;
831
832 if (!trans) {
833 path->skip_locking = 1;
834 path->search_commit_root = 1;
835 }
836
837 search_again:
838 key.objectid = bytenr;
839 key.offset = offset;
840 if (metadata)
841 key.type = BTRFS_METADATA_ITEM_KEY;
842 else
843 key.type = BTRFS_EXTENT_ITEM_KEY;
844
845 ret = btrfs_search_slot(trans, fs_info->extent_root, &key, path, 0, 0);
846 if (ret < 0)
847 goto out_free;
848
849 if (ret > 0 && metadata && key.type == BTRFS_METADATA_ITEM_KEY) {
850 if (path->slots[0]) {
851 path->slots[0]--;
852 btrfs_item_key_to_cpu(path->nodes[0], &key,
853 path->slots[0]);
854 if (key.objectid == bytenr &&
855 key.type == BTRFS_EXTENT_ITEM_KEY &&
856 key.offset == fs_info->nodesize)
857 ret = 0;
858 }
859 }
860
861 if (ret == 0) {
862 leaf = path->nodes[0];
863 item_size = btrfs_item_size_nr(leaf, path->slots[0]);
864 if (item_size >= sizeof(*ei)) {
865 ei = btrfs_item_ptr(leaf, path->slots[0],
866 struct btrfs_extent_item);
867 num_refs = btrfs_extent_refs(leaf, ei);
868 extent_flags = btrfs_extent_flags(leaf, ei);
869 } else {
870 ret = -EINVAL;
> 871 btrfs_print_v0_err(fs_info);
872 if (trans)
873 btrfs_abort_transaction(trans, ret);
874 else
875 btrfs_handle_fs_error(fs_info, ret, NULL);
876
877 goto out_free;
878 }
879
880 BUG_ON(num_refs == 0);
881 } else {
882 num_refs = 0;
883 extent_flags = 0;
884 ret = 0;
885 }
886
887 if (!trans)
888 goto out;
889
890 delayed_refs = &trans->transaction->delayed_refs;
891 spin_lock(&delayed_refs->lock);
892 head = btrfs_find_delayed_ref_head(delayed_refs, bytenr);
893 if (head) {
894 if (!mutex_trylock(&head->mutex)) {
895 refcount_inc(&head->refs);
896 spin_unlock(&delayed_refs->lock);
897
898 btrfs_release_path(path);
899
900 /*
901 * Mutex was contended, block until it's released and try
902 * again
903 */
904 mutex_lock(&head->mutex);
905 mutex_unlock(&head->mutex);
906 btrfs_put_delayed_ref_head(head);
907 goto search_again;
908 }
909 spin_lock(&head->lock);
910 if (head->extent_op && head->extent_op->update_flags)
911 extent_flags |= head->extent_op->flags_to_set;
912 else
913 BUG_ON(num_refs == 0);
914
915 num_refs += head->ref_mod;
916 spin_unlock(&head->lock);
917 mutex_unlock(&head->mutex);
918 }
919 spin_unlock(&delayed_refs->lock);
920 out:
921 WARN_ON(num_refs == 0);
922 if (refs)
923 *refs = num_refs;
924 if (flags)
925 *flags = extent_flags;
926 out_free:
927 btrfs_free_path(path);
928 return ret;
929 }
930
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33304 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] btrfs: Add graceful handling of V0 extents
2018-06-26 14:17 ` David Sterba
@ 2018-06-27 13:12 ` Noah Massey
2018-06-27 13:21 ` David Sterba
0 siblings, 1 reply; 14+ messages in thread
From: Noah Massey @ 2018-06-27 13:12 UTC (permalink / raw)
To: David Sterba, Nikolay Borisov, David Sterba, linux-btrfs
On Tue, Jun 26, 2018 at 12:02 PM David Sterba <dsterba@suse.cz> wrote:
>
> On Tue, Jun 26, 2018 at 04:57:36PM +0300, Nikolay Borisov wrote:
> > Following the removal of the v0 handling code let's be courteous and
> > print an error message when such extents are handled. In the cases
> > where we have a transaction just abort it, otherwise just call
> > btrfs_handle_fs_error. Both cases result in the FS being re-mounted RO.
> >
> > Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>
> > - ASSERT(key.type != BTRFS_EXTENT_REF_V0_KEY);
> > - if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> > + if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> > + err = -EINVAL;
> > + btrfs_print_v0_err(rc->extent_root->fs_info);
> > + btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> > + NULL);
> > + goto out;
> > + } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
>
> The v0 check should be made last as it's not expected to happen. I'm
> commiting with this diff
>
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -787,13 +787,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
> goto next;
> }
>
> - if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> - err = -EINVAL;
> - btrfs_print_v0_err(rc->extent_root->fs_info);
> - btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> - NULL);
> - goto out;
> - } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> + if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> if (key.objectid == key.offset) {
> /*
> * only root blocks of reloc trees use
> @@ -838,6 +832,12 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
> goto next;
> } else if (key.type != BTRFS_TREE_BLOCK_REF_KEY) {
> goto next;
The V0 check needs to be before this one
> + } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> + err = -EINVAL;
> + btrfs_print_v0_err(rc->extent_root->fs_info);
> + btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> + NULL);
> + goto out;
> }
>
> /* key.type == BTRFS_TREE_BLOCK_REF_KEY */
> @@ -3734,11 +3734,7 @@ int add_data_references(struct reloc_control *rc,
> if (key.objectid != extent_key->objectid)
> break;
>
> - if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> - btrfs_print_v0_err(eb->fs_info);
> - btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
> - ret = -EINVAL;
> - } else if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
> + if (key.type == BTRFS_SHARED_DATA_REF_KEY) {
> ret = __add_tree_block(rc, key.offset, blocksize,
> blocks);
> } else if (key.type == BTRFS_EXTENT_DATA_REF_KEY) {
> @@ -3746,6 +3742,10 @@ int add_data_references(struct reloc_control *rc,
> struct btrfs_extent_data_ref);
> ret = find_data_references(rc, extent_key,
> eb, dref, blocks);
> + } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> + btrfs_print_v0_err(eb->fs_info);
> + btrfs_handle_fs_error(eb->fs_info, -EINVAL, NULL);
> + ret = -EINVAL;
> } else {
> ret = 0;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] btrfs: Add graceful handling of V0 extents
2018-06-27 13:12 ` Noah Massey
@ 2018-06-27 13:21 ` David Sterba
0 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2018-06-27 13:21 UTC (permalink / raw)
To: Noah Massey; +Cc: David Sterba, Nikolay Borisov, David Sterba, linux-btrfs
On Wed, Jun 27, 2018 at 09:12:06AM -0400, Noah Massey wrote:
> On Tue, Jun 26, 2018 at 12:02 PM David Sterba <dsterba@suse.cz> wrote:
> >
> > On Tue, Jun 26, 2018 at 04:57:36PM +0300, Nikolay Borisov wrote:
> > > Following the removal of the v0 handling code let's be courteous and
> > > print an error message when such extents are handled. In the cases
> > > where we have a transaction just abort it, otherwise just call
> > > btrfs_handle_fs_error. Both cases result in the FS being re-mounted RO.
> > >
> > > Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> >
> > > - ASSERT(key.type != BTRFS_EXTENT_REF_V0_KEY);
> > > - if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> > > + if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> > > + err = -EINVAL;
> > > + btrfs_print_v0_err(rc->extent_root->fs_info);
> > > + btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> > > + NULL);
> > > + goto out;
> > > + } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> >
> > The v0 check should be made last as it's not expected to happen. I'm
> > commiting with this diff
> >
> > --- a/fs/btrfs/relocation.c
> > +++ b/fs/btrfs/relocation.c
> > @@ -787,13 +787,7 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
> > goto next;
> > }
> >
> > - if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> > - err = -EINVAL;
> > - btrfs_print_v0_err(rc->extent_root->fs_info);
> > - btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> > - NULL);
> > - goto out;
> > - } else if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> > + if (key.type == BTRFS_SHARED_BLOCK_REF_KEY) {
> > if (key.objectid == key.offset) {
> > /*
> > * only root blocks of reloc trees use
> > @@ -838,6 +832,12 @@ struct backref_node *build_backref_tree(struct reloc_control *rc,
> > goto next;
> > } else if (key.type != BTRFS_TREE_BLOCK_REF_KEY) {
> > goto next;
>
> The V0 check needs to be before this one
Right, due to !=. Thanks for catching it.
> > + } else if (key.type == BTRFS_EXTENT_REF_V0_KEY) {
> > + err = -EINVAL;
> > + btrfs_print_v0_err(rc->extent_root->fs_info);
> > + btrfs_handle_fs_error(rc->extent_root->fs_info, err,
> > + NULL);
> > + goto out;
> > }
> >
> > /* key.type == BTRFS_TREE_BLOCK_REF_KEY */
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-06-27 13:21 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-25 8:24 [PATCH 0/2] Remove v0 extent support Nikolay Borisov
2018-06-25 8:24 ` [PATCH 1/2] btrfs: Remove V0 " Nikolay Borisov
2018-06-25 8:24 ` [PATCH 2/2] btrfs: Add graceful handling of V0 extents Nikolay Borisov
2018-06-25 15:21 ` David Sterba
2018-06-26 13:57 ` [PATCH v2] " Nikolay Borisov
2018-06-26 14:17 ` David Sterba
2018-06-27 13:12 ` Noah Massey
2018-06-27 13:21 ` David Sterba
2018-06-26 14:24 ` [PATCH] btrfs: annotate unlikely branches after V0 extent type removal David Sterba
2018-06-26 14:31 ` Nikolay Borisov
2018-06-26 14:43 ` David Sterba
2018-06-26 16:05 ` [PATCH v2] btrfs: Add graceful handling of V0 extents kbuild test robot
2018-06-26 17:12 ` David Sterba
2018-06-26 17:44 ` kbuild test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).