| Message ID | 20190220111143.15911-1-fdmanana@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Btrfs: remove assertion when searching for a key in a node/leaf | expand |
On 2019/2/20 下午7:11, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > At ctree.c:key_search(), the assertion that verifies the first key on a > child extent buffer corresponds to the key at a specific slot in the > parent has a disadvantage: we effectively hit a BUG_ON() which requires > rebooting the machine later. It also does not tell any information about > which extent buffer is affected, from which root, the expected and found > keys, etc. > > However as of commit 581c1760415c48 ("btrfs: Validate child tree block's > level and first key"), that assertion is not needed since at the time we > read an extent buffer from disk we validate that its first key matches the > key, at the respective slot, in the parent extent buffer. Therefore just > remove the assertion at key_search(). > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/ctree.c | 21 --------------------- > 1 file changed, 21 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 5b9f602fb9e2..e754bd019618 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -2529,26 +2529,6 @@ setup_nodes_for_search(struct btrfs_trans_handle *trans, > return ret; > } > > -static void key_search_validate(struct extent_buffer *b, > - const struct btrfs_key *key, > - int level) > -{ > -#ifdef CONFIG_BTRFS_ASSERT > - struct btrfs_disk_key disk_key; > - > - btrfs_cpu_key_to_disk(&disk_key, key); > - > - if (level == 0) > - ASSERT(!memcmp_extent_buffer(b, &disk_key, > - offsetof(struct btrfs_leaf, items[0].key), > - sizeof(disk_key))); > - else > - ASSERT(!memcmp_extent_buffer(b, &disk_key, > - offsetof(struct btrfs_node, ptrs[0].key), > - sizeof(disk_key))); > -#endif > -} > - > static int key_search(struct extent_buffer *b, const struct btrfs_key *key, > int level, int *prev_cmp, int *slot) > { > @@ -2557,7 +2537,6 @@ static int key_search(struct extent_buffer *b, const struct btrfs_key *key, > return *prev_cmp; > } > > - key_search_validate(b, key, level); > *slot = 0; > > return 0; >
On Wed, Feb 20, 2019 at 11:11:43AM +0000, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > At ctree.c:key_search(), the assertion that verifies the first key on a > child extent buffer corresponds to the key at a specific slot in the > parent has a disadvantage: we effectively hit a BUG_ON() which requires > rebooting the machine later. It also does not tell any information about > which extent buffer is affected, from which root, the expected and found > keys, etc. > > However as of commit 581c1760415c48 ("btrfs: Validate child tree block's > level and first key"), that assertion is not needed since at the time we > read an extent buffer from disk we validate that its first key matches the > key, at the respective slot, in the parent extent buffer. Therefore just > remove the assertion at key_search(). > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Added to misc-next, thanks.
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 5b9f602fb9e2..e754bd019618 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -2529,26 +2529,6 @@ setup_nodes_for_search(struct btrfs_trans_handle *trans, return ret; } -static void key_search_validate(struct extent_buffer *b, - const struct btrfs_key *key, - int level) -{ -#ifdef CONFIG_BTRFS_ASSERT - struct btrfs_disk_key disk_key; - - btrfs_cpu_key_to_disk(&disk_key, key); - - if (level == 0) - ASSERT(!memcmp_extent_buffer(b, &disk_key, - offsetof(struct btrfs_leaf, items[0].key), - sizeof(disk_key))); - else - ASSERT(!memcmp_extent_buffer(b, &disk_key, - offsetof(struct btrfs_node, ptrs[0].key), - sizeof(disk_key))); -#endif -} - static int key_search(struct extent_buffer *b, const struct btrfs_key *key, int level, int *prev_cmp, int *slot) { @@ -2557,7 +2537,6 @@ static int key_search(struct extent_buffer *b, const struct btrfs_key *key, return *prev_cmp; } - key_search_validate(b, key, level); *slot = 0; return 0;