| Message ID | 871fea9cea530b626f0f253c00d53a3e7a1ea531.1606938211.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup error handling in relocation | expand |
On 2020/12/3 上午3:50, 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(ret != -ENOENT), as this indicates we messed up the backref > walking code, but could indicate corruption so we do not want to have a > BUG_ON() here. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/relocation.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > index 4333ee329290..66515ccc04fe 100644 > --- a/fs/btrfs/relocation.c > +++ b/fs/btrfs/relocation.c > @@ -2027,7 +2027,7 @@ struct btrfs_root *select_reloc_root(struct btrfs_trans_handle *trans, > break; > } > if (!root) > - return NULL; > + return ERR_PTR(-ENOENT); > > next = node; > /* setup backref node path for btrfs_reloc_cow_block */ > @@ -2198,7 +2198,18 @@ 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); > + > + /* > + * This can happen if there's fs corruption, but if we > + * have ASSERT()'s on then we're developers and we > + * likely made a logic mistake in the backref code, so > + * check for this error condition. > + */ > + ASSERT(ret != -ENOENT); > + goto next; > + } > > if (upper->eb && !upper->locked) { > if (!lowest) { >
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index 4333ee329290..66515ccc04fe 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -2027,7 +2027,7 @@ struct btrfs_root *select_reloc_root(struct btrfs_trans_handle *trans, break; } if (!root) - return NULL; + return ERR_PTR(-ENOENT); next = node; /* setup backref node path for btrfs_reloc_cow_block */ @@ -2198,7 +2198,18 @@ 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); + + /* + * This can happen if there's fs corruption, but if we + * have ASSERT()'s on then we're developers and we + * likely made a logic mistake in the backref code, so + * check for this error condition. + */ + ASSERT(ret != -ENOENT); + goto next; + } if (upper->eb && !upper->locked) { if (!lowest) {
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(ret != -ENOENT), as this indicates we messed up the backref walking code, but could indicate corruption so we do not want to have a BUG_ON() here. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/relocation.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)