On Wed, Nov 27, 2019 at 4:08 PM David Sterba <dsterba@xxxxxxx> 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@xxxxxxxx>
thanks
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”