Re: 6.2 regression: BTRFS_LOGICAL_INO_ARGS_IGNORE_OFFSET broken

[Zygo Blaxell <ce3g8jdj@umail.furryterror.org>]

On Wed, May 03, 2023 at 01:49:16PM +0100, Filipe Manana wrote:
> On Wed, May 3, 2023 at 1:37 PM Filipe Manana <fdmanana@kernel.org> wrote:
> >
> > On Wed, May 3, 2023 at 1:33 PM Vladimir Panteleev
> > <git@vladimir.panteleev.md> wrote:
> > >
> > > Hi,
> > >
> > > Commit 6ce6ba534418132f4c727d5707fe2794c797299c appears to have broken
> > > the BTRFS_LOGICAL_INO_ARGS_IGNORE_OFFSET flag to
> > > BTRFS_IOC_LOGICAL_INO_V2. The ioctl now always seems to return zero
> > > inodes with the flag, if the same happened without the flag, thus
> > > making it not very useful.
> > >
> > > Context: I maintain btdu, a disk usage profiler for btrfs. It uses
> > > BTRFS_LOGICAL_INO_ARGS_IGNORE_OFFSET to help users estimate the amount
> > > of space wasted by bookend extents, and identify files / applications
> > > / IO patterns which create excessive amounts of them.
> >
> > Are you able to apply and test a kernel patch?
> >
> > If so, try the following one (also at:
> > https://gist.github.com/fdmanana/9ae7f6c62779aacf4bfd3b155d175792)
> >
> > diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> > index e54f0884802a..c4c5784e897a 100644
> > --- a/fs/btrfs/backref.c
> > +++ b/fs/btrfs/backref.c
> > @@ -45,7 +45,8 @@ static int check_extent_in_eb(struct
> > btrfs_backref_walk_ctx *ctx,
> >         int root_count;
> >         bool cached;
> >
> > -       if (!btrfs_file_extent_compression(eb, fi) &&
> > +       if (!ctx->ignore_extent_item_pos &&
> 
> This misses a:
> 
> .. && ctx->extent_item_pos > 0 &&

Ummm...why?

With IGNORE_OFFSET set, we want to ignore the offset on the candidate
matching extent and on the original search bytenr, so we get matches in
cases where the search bytenr happens to be at offset 0 in the extent.

I think your first patch was the better one.  What am I missing?

> I've updated the gist with it:
> https://gist.githubusercontent.com/fdmanana/9ae7f6c62779aacf4bfd3b155d175792/raw/3f41c8486eb73a038f026c8bfe767bd763a016c9/logical_ino2_fix.patch
> 
> Thanks.
> 
> > +           !btrfs_file_extent_compression(eb, fi) &&
> >             !btrfs_file_extent_encryption(eb, fi) &&
> >             !btrfs_file_extent_other_encoding(eb, fi)) {
> >                 u64 data_offset;
> > @@ -552,13 +553,10 @@ static int add_all_parents(struct
> > btrfs_backref_walk_ctx *ctx,
> >                                 count++;
> >                         else
> >                                 goto next;
> > -                       if (!ctx->ignore_extent_item_pos) {
> > -                               ret = check_extent_in_eb(ctx, &key,
> > eb, fi, &eie);
> > -                               if (ret == BTRFS_ITERATE_EXTENT_INODES_STOP ||
> > -                                   ret < 0)
> > -                                       break;
> > -                       }
> > -                       if (ret > 0)
> > +                       ret = check_extent_in_eb(ctx, &key, eb, fi, &eie);
> > +                       if (ret == BTRFS_ITERATE_EXTENT_INODES_STOP || ret < 0)
> > +                               break;
> > +                       else if (ret > 0)
> >                                 goto next;
> >                         ret = ulist_add_merge_ptr(parents, eb->start,
> >                                                   eie, (void **)&old, GFP_NOFS);
> >
> > Thanks.
> > >
> > > Thanks!