Re: [PATCH] Btrfs: remove assertion when searching for a key in a node/leaf

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2226 bytes --]



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;
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]