On Wed, Dec 16, 2020 at 11:26:19AM -0500, Josef Bacik wrote:
> Currently select_reloc_root() doesn't return an error, but followup
> patches will make it possible for it to return an error. We do have
> proper error recovery in do_relocation however, so handle the
> possibility of select_reloc_root() having an error properly instead of
> BUG_ON(!root). I've also adjusted select_reloc_root() to return
> ERR_PTR(-ENOENT) if we don't find a root, instead of NULL, to make the
> error case easier to deal with. I've replaced the BUG_ON(!root) with an
> ASSERT(0) for this case as it indicates we messed up the backref walking
> code, but it could also indicate corruption.
>
> Signed-off-by: Josef Bacik <[email protected]>
> ---
> fs/btrfs/relocation.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 08ffaa93b78f..741068580455 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -2024,8 +2024,14 @@ struct btrfs_root *select_reloc_root(struct
> btrfs_trans_handle *trans,
> if (!next || next->level <= node->level)
> break;
> }
> - if (!root)
> - return NULL;
> + if (!root) {
> + /*
> + * This can happen if there's fs corruption or if there's a bug
> + * in the backref lookup code.
> + */
> + ASSERT(0);
You've added these assert(0) to several patches and I think it's wrong.
The asserts are supposed to verify invariants, you can hardly say that
we're expecting 0 to be true, so the construct serves as "please crash
here", which is no better than BUG(). It's been spreading, there are
like 25 now.
> + return ERR_PTR(-ENOENT);
The caller that does expect ENOENT because that would be a logical error
should do the assert.
> + }
>
> next = node;
> /* setup backref node path for btrfs_reloc_cow_block */
> @@ -2196,7 +2202,10 @@ static int do_relocation(struct btrfs_trans_handle
> *trans,
>
> upper = edge->node[UPPER];
> root = select_reloc_root(trans, rc, upper, edges);
> - BUG_ON(!root);
> + if (IS_ERR(root)) {
> + ret = PTR_ERR(root);
> + goto next;
> + }
>
> if (upper->eb && !upper->locked) {
> if (!lowest) {
> --
> 2.26.2
