| Message ID | 5e4c3eaf235f8a73054f06d8fa68673566a5b323.1690783136.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: output extra debug info if we failed to find an inline backref | expand |
On Mon, Jul 31, 2023 at 7:16 AM Qu Wenruo <wqu@suse.com> wrote: > > [BUG] > Syzbot reported several warning triggered inside > lookup_inline_extent_backref(). > > [CAUSE] > As usual, the reproducer doesn't reliably trigger locally here, but at > least we know the WARN_ON() is triggered when an inline backref can not > be found, and it can only be triggered when @insert is true. (aka, > inserting a new inline backref, which means the backref should already > exist) > > [ENHANCEMENT] > Instead of a plain WARN_ON(), dump all the parameter and the extent tree > leaf to help debug. > > Link: https://syzkaller.appspot.com/bug?extid=d6f9ff86c1d804ba2bc6 > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/extent-tree.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 3cae798499e2..51a721b7156e 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -847,7 +847,13 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans, > if (ret && !insert) { > err = -ENOENT; > goto out; > - } else if (WARN_ON(ret)) { > + } else if (unlikely(ret)) { > + btrfs_err(fs_info, > +"inline backref not found, bytenr %llu num_bytes %llu parent %llu root_objectid %llu owner %llu offset %llu", > + bytenr, num_bytes, parent, root_objectid, owner, > + offset); > + btrfs_print_leaf(path->nodes[0]); > + WARN_ON(1); The error messages should be printed after dumping the leaf. This is what we generally do to make sure the message is seen on logs from screenshots sometimes sent by users. For the same reason, the WARN_ON() should also happen before the error message. Also why did you remove it from the else-if statement? WARN_ON() already uses 'unlikely'. Thanks. > err = -EIO; > goto out; > } > -- > 2.41.0 >
On 2023/7/31 20:06, Filipe Manana wrote: > On Mon, Jul 31, 2023 at 7:16 AM Qu Wenruo <wqu@suse.com> wrote: >> >> [BUG] >> Syzbot reported several warning triggered inside >> lookup_inline_extent_backref(). >> >> [CAUSE] >> As usual, the reproducer doesn't reliably trigger locally here, but at >> least we know the WARN_ON() is triggered when an inline backref can not >> be found, and it can only be triggered when @insert is true. (aka, >> inserting a new inline backref, which means the backref should already >> exist) >> >> [ENHANCEMENT] >> Instead of a plain WARN_ON(), dump all the parameter and the extent tree >> leaf to help debug. >> >> Link: https://syzkaller.appspot.com/bug?extid=d6f9ff86c1d804ba2bc6 >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> fs/btrfs/extent-tree.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >> index 3cae798499e2..51a721b7156e 100644 >> --- a/fs/btrfs/extent-tree.c >> +++ b/fs/btrfs/extent-tree.c >> @@ -847,7 +847,13 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans, >> if (ret && !insert) { >> err = -ENOENT; >> goto out; >> - } else if (WARN_ON(ret)) { >> + } else if (unlikely(ret)) { >> + btrfs_err(fs_info, >> +"inline backref not found, bytenr %llu num_bytes %llu parent %llu root_objectid %llu owner %llu offset %llu", >> + bytenr, num_bytes, parent, root_objectid, owner, >> + offset); >> + btrfs_print_leaf(path->nodes[0]); >> + WARN_ON(1); > > The error messages should be printed after dumping the leaf. > This is what we generally do to make sure the message is seen on logs > from screenshots sometimes sent by users. > > For the same reason, the WARN_ON() should also happen before the error message. > Also why did you remove it from the else-if statement? WARN_ON() > already uses 'unlikely'. I thought the "if (WARN_ON())" pattern is not recommended anymore. But I'm totally fine going back to the existing one. Thanks, Qu > > Thanks. > >> err = -EIO; >> goto out; >> } >> -- >> 2.41.0 >>
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 3cae798499e2..51a721b7156e 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -847,7 +847,13 @@ int lookup_inline_extent_backref(struct btrfs_trans_handle *trans, if (ret && !insert) { err = -ENOENT; goto out; - } else if (WARN_ON(ret)) { + } else if (unlikely(ret)) { + btrfs_err(fs_info, +"inline backref not found, bytenr %llu num_bytes %llu parent %llu root_objectid %llu owner %llu offset %llu", + bytenr, num_bytes, parent, root_objectid, owner, + offset); + btrfs_print_leaf(path->nodes[0]); + WARN_ON(1); err = -EIO; goto out; }
[BUG] Syzbot reported several warning triggered inside lookup_inline_extent_backref(). [CAUSE] As usual, the reproducer doesn't reliably trigger locally here, but at least we know the WARN_ON() is triggered when an inline backref can not be found, and it can only be triggered when @insert is true. (aka, inserting a new inline backref, which means the backref should already exist) [ENHANCEMENT] Instead of a plain WARN_ON(), dump all the parameter and the extent tree leaf to help debug. Link: https://syzkaller.appspot.com/bug?extid=d6f9ff86c1d804ba2bc6 Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/extent-tree.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)