Re: [PATCH] btrfs: Don't panic when we can't find a root key

From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <wqu@suse.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: Don't panic when we can't find a root key
Date: Mon, 25 Feb 2019 13:26:11 +0100	[thread overview]
Message-ID: <20190225122611.GO9874@twin.jikos.cz> (raw)
In-Reply-To: <20190225051106.22091-1-wqu@suse.com>

On Mon, Feb 25, 2019 at 01:11:06PM +0800, Qu Wenruo wrote:
> When we failed to find a root key in btrfs_update_root(), we just panic.
> 
> That's definitely not cool, fix it by aborting current transaction and
> return an error value.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/root-tree.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> index 65bda0682928..c48a3e18866d 100644
> --- a/fs/btrfs/root-tree.c
> +++ b/fs/btrfs/root-tree.c
> @@ -137,11 +137,15 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root
>  		goto out;
>  	}
>  
> -	if (ret != 0) {
> +	if (ret > 0) {
>  		btrfs_print_leaf(path->nodes[0]);

You leave the leaf dump here, which I believe was related to the BUG_ON
so there's information for later analysis. This is not usually done
where transaction is aborted and error returned. I'm not saying it's not
worth keeping it here, as the logic that would lead to this error is
in the 'never happens' category. But we know such things happen due to
the bitflips and random memory corruptions so a dump might make sense.

If you think this makes sense, please put it to the changelog and write
a comment before the leaf dump. This will be a good pattern to follow as
there are many instances of leaf_dump/BUG to clean up or review.