| Message ID | 8f6d53a745813c8267a20b1dc1caa4fb722423bb.1727970062.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: backref cache cleanups | expand |
On Thu, Oct 03, 2024 at 11:43:03AM -0400, Josef Bacik wrote: > This BUG_ON is meant to catch backref cache problems, but these can > arise from either bugs in the backref cache or corruption in the extent > tree. Fix it to be a proper error and change it to an ASSERT() so that > developers notice problems. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/relocation.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > index f3834f8d26b4..3c89e79d0259 100644 > --- a/fs/btrfs/relocation.c > +++ b/fs/btrfs/relocation.c > @@ -4399,8 +4399,20 @@ int btrfs_reloc_cow_block(struct btrfs_trans_handle *trans, > WARN_ON(!first_cow && level == 0); > > node = rc->backref_cache.path[level]; > - BUG_ON(node->bytenr != buf->start && > - node->new_bytenr != buf->start); > + > + /* > + * If node->bytenr != buf->start and node->new_bytenr != > + * buf->start then we've got the wrong backref node for what we > + * expected to see here and the cache is incorrect. > + */ > + if (node->bytenr != buf->start && > + node->new_bytenr != buf->start) { > + btrfs_err(fs_info, > +"bytenr %llu was found but our backref cache was expecting %llu or %llu", > + buf->start, node->bytenr, node->new_bytenr); > + ASSERT(0); Please don't add the assert(0), the error message and EUCLEAN should be sufficient. The caller btrfs_force_cow_block() aborts if it fails so this will be noticealbe. Syzbot is good at hitting strange assertions so we'll eventually get a report and have to fix it by removing the assert(0) again.
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index f3834f8d26b4..3c89e79d0259 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -4399,8 +4399,20 @@ int btrfs_reloc_cow_block(struct btrfs_trans_handle *trans, WARN_ON(!first_cow && level == 0); node = rc->backref_cache.path[level]; - BUG_ON(node->bytenr != buf->start && - node->new_bytenr != buf->start); + + /* + * If node->bytenr != buf->start and node->new_bytenr != + * buf->start then we've got the wrong backref node for what we + * expected to see here and the cache is incorrect. + */ + if (node->bytenr != buf->start && + node->new_bytenr != buf->start) { + btrfs_err(fs_info, +"bytenr %llu was found but our backref cache was expecting %llu or %llu", + buf->start, node->bytenr, node->new_bytenr); + ASSERT(0); + return -EUCLEAN; + } btrfs_backref_drop_node_buffer(node); atomic_inc(&cow->refs);
This BUG_ON is meant to catch backref cache problems, but these can arise from either bugs in the backref cache or corruption in the extent tree. Fix it to be a proper error and change it to an ASSERT() so that developers notice problems. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/relocation.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)