Re: [PATCH 1/3] btrfs: Don't discard unwritten extents

[Filipe Manana <fdmanana@gmail.com>]

On Wed, Nov 27, 2019 at 4:08 PM David Sterba <dsterba@suse.cz> wrote:
>
> On Thu, Nov 21, 2019 at 02:03:29PM +0200, Nikolay Borisov wrote:
> > --- a/fs/btrfs/extent-tree.c
> > +++ b/fs/btrfs/extent-tree.c
> > @@ -4169,8 +4169,6 @@ static int __btrfs_free_reserved_extent(struct btrfs_fs_info *fs_info,
> >               if (ret)
> >                       goto out;
> >       } else {
> > -             if (btrfs_test_opt(fs_info, DISCARD))
> > -                     ret = btrfs_discard_extent(fs_info, start, len, NULL);
> >               btrfs_add_free_space(cache, start, len);
> >               btrfs_free_reserved_bytes(cache, len, delalloc);
> >               trace_btrfs_reserved_extent_free(fs_info, start, len);
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 0ac0f5b33003..5d80fe030e79 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -3250,10 +3250,15 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
> >               if ((ret || !logical_len) &&
> >                   clear_reserved_extent &&
> >                   !test_bit(BTRFS_ORDERED_NOCOW, &ordered_extent->flags) &&
> > -                 !test_bit(BTRFS_ORDERED_PREALLOC, &ordered_extent->flags))
> > +                 !test_bit(BTRFS_ORDERED_PREALLOC, &ordered_extent->flags)) {
> >                       btrfs_free_reserved_extent(fs_info,
> >                                                  ordered_extent->start,
> >                                                  ordered_extent->disk_len, 1);
> > +                     if (ret && btrfs_test_opt(fs_info, DISCARD))
> > +                             btrfs_discard_extent(fs_info,
> > +                             ordered_extent->start, ordered_extent->disk_len,
> > +                             NULL);
>
> This brings back vague memories of misplaced discard (in
> finish_ordered_io), that was quite hard to catch. I can't find the fix
> though. Filipe, is it the same issue?

It's different (as you identified already in another reply).
Even though this is rarely hit on a healthy system, it looks fine to me.

Reviewed-by: Filipe Manana <fdmanana@suse.com>

thanks



-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”