Re: [PATCH] btrfs: delete unneeded assignments in btrfs_defrag_file

[David Sterba <dsterba@suse.cz>]

On Tue, Apr 27, 2021 at 09:50:21AM +0800, Tian Tao wrote:
> ret is assigned -EAGAIN at line 1455 and then reassigned defrag_count
> at line 1547 after exiting the while loop, but the btrfs_defrag_file
> function returns a negative number indicating that the execution failed
> because it does not make sense to reassign defrag_count to ret, so
> delete it.

The line references are fragile, so the 1455 is after defrag is
cancelled.

> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> ---
>  fs/btrfs/ioctl.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index ee1dbab..2b3b228 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1544,8 +1544,6 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
>  		btrfs_set_fs_incompat(fs_info, COMPRESS_ZSTD);
>  	}
>  
> -	ret = defrag_count;

But this would change semantics of the whole function, after deleting
this line any stale value of 'ret' would be returned, it's used for some
intermediate return values in the whole while loop.

1597                 if (btrfs_defrag_cancelled(fs_info)) {
1598                         btrfs_debug(fs_info, "defrag_file cancelled");
1599                         ret = -EAGAIN;
1600                         break;
1601                 }

Jumping to the 'out_ra' label looks like a candidate fix but that also
jumps around all the incompat bit setting, so that could in some cases
miss to set them properly. And actually this is a problem with all the
other error cases.

I'm not yet sure what's the proper fix, but the errors from within the
while loop should be returned and incompat bits set.