Re: [PATCH] fs: Fix a missing check bug

[Qu Wenruo]

On 2019/12/7 äå10:41, Dinghao Liu wrote:
> The return value of link_free_space(ctl, info) is checked out-sync. Only one branch of an if statement checks this return value after WARN_ON(ret).
> 
> Since this path pair is similar in semantic, there might be a missing check bug.
> 
> Fix this by simply adding a check on ret.

The main failure mode for link_free_space() is -EEXIST, which means
there is already free space in the cache.

Here EEXIST may not be a big problem, and we may really want to continue
the iteration other than error out.


Would you explain in details about why you believe error out is the
correct way other than current continue behavior?

Thanks,
Qu

> 
> Signed-off-by: Dinghao Liu <dinghao.liu@xxxxxxxxxx>
> ---
>  fs/btrfs/free-space-cache.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 3283da419200..acbb3a59d344 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -2437,6 +2437,8 @@ int btrfs_remove_free_space(struct btrfs_block_group *block_group,
>  			if (info->bytes) {
>  				ret = link_free_space(ctl, info);
>  				WARN_ON(ret);
> +				if (ret)
> +					goto out_lock;
>  			} else {
>  				kmem_cache_free(btrfs_free_space_cachep, info);
>  			}
>

Attachment: signature.asc
Description: OpenPGP digital signature