Re: [PATCH 01/12] btrfs: fix missing error handling when searching for inode refs during log replay

[Filipe Manana <fdmanana@kernel.org>]

On Wed, Jun 25, 2025 at 10:45 AM Johannes Thumshirn
<Johannes.Thumshirn@wdc.com> wrote:
>
> On 24.06.25 16:56, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > During log replay, at __add_inode_ref(), when we are searching for inode
> > ref keys we totally ignore if btrfs_search_slot() returns an error. This
> > may make a log replay succeed when there was an actual error and leave
> > some metadata inconsistency in a subvolume tree. Fix this by checking if
> > an error was returned from btrfs_search_slot() and if so, return it to
> > the caller.
> >
> > Fixes: e02119d5a7b4 ("Btrfs: Add a write ahead tree log to optimize synchronous operations")
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >   fs/btrfs/tree-log.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> > index 839252881138..08948803c857 100644
> > --- a/fs/btrfs/tree-log.c
> > +++ b/fs/btrfs/tree-log.c
> > @@ -1073,7 +1073,9 @@ static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
> >       search_key.type = BTRFS_INODE_REF_KEY;
> >       search_key.offset = parent_objectid;
> >       ret = btrfs_search_slot(NULL, root, &search_key, path, 0, 0);
> > -     if (ret == 0) {
> > +     if (ret < 0) {
> > +             return ret;
> > +     } else if (ret == 0) {
>
> Technically the else is not needed after the return.

Yes, but I don't think we have any rule that dictates to not use the else.
Just a matter of personal preference.

Thanks.
>
> Otherwise:
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>