Re: [PATCH v7 03/38] btrfs: handle errors from select_reloc_root()
From: David Sterba <hidden>
Date: 2021-02-26 18:34:14
On Wed, Dec 16, 2020 at 11:26:19AM -0500, Josef Bacik wrote:
quoted hunk ↗ jump to hunk
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 <josef@toxicpanda.com> --- 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.
quoted hunk ↗ jump to hunk
+ } 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