Re: [PATCH 3/4] btrfs: open code btrfs_set_prop in inherit_prop

[Nikolay Borisov <nborisov@suse.com>]

On 14.03.19 г. 7:05 ч., Anand Jain wrote:
> When an inode inherits property from its parent, we call btrfs_set_prop().
> btrfs_set_prop() does an elaborate checks, which is not required in the
> context of inheriting a property. Instead just open-code only the required
> items from btrfs_set_prop() and then call btrfs_setxattr() directly. So
> now the only user of btrfs_set_prop() is gone, (except for the wraper
> function btrfs_set_prop_trans()).

Um, no:

git grep -P '(?<!__)btrfs_set_prop' fs/btrfs/
fs/btrfs/ioctl.c:               ret = btrfs_set_prop(inode, "btrfs.compression", NULL, 0, 0);
fs/btrfs/ioctl.c:               ret = btrfs_set_prop(inode, "btrfs.compression",
fs/btrfs/ioctl.c:               ret = btrfs_set_prop(inode, "btrfs.compression", NULL, 0, 0);
fs/btrfs/props.c:int btrfs_set_prop(struct inode *inode,
fs/btrfs/props.h:int btrfs_set_prop(struct inode *inode,
fs/btrfs/xattr.c:       return btrfs_set_prop(inode, name, value, size, flags);


Rebase the patch on misc-next and remove the last sentence 
since it's factually wrong.

> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/props.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
> index 72a06c4d3c70..fb84e76f3b1d 100644
> --- a/fs/btrfs/props.c
> +++ b/fs/btrfs/props.c
> @@ -367,20 +367,35 @@ static int inherit_props(struct btrfs_trans_handle *trans,
>  		if (!value)
>  			continue;
>  
> +		/* may be removed */
> +		ret = h->validate(value, strlen(value));
> +		if (ret)
> +			continue;
> +
>  		num_bytes = btrfs_calc_trans_metadata_size(fs_info, 1);
>  		ret = btrfs_block_rsv_add(root, trans->block_rsv,
>  					  num_bytes, BTRFS_RESERVE_NO_FLUSH);
>  		if (ret)
> -			goto out;
> -		ret = btrfs_set_prop(trans, inode, h->xattr_name, value,

What branch is this code based on, currently misc-next is using (and 
has been since 2014) __btrfs_set_prop yet you remove btrfs_set_prop?

> +			return ret;
> +
> +		ret = btrfs_setxattr(trans, inode, h->xattr_name, value,
>  				     strlen(value), 0);
> +		if (!ret) {
> +			ret = h->apply(inode, value, strlen(value));
> +			if (ret)
> +				btrfs_setxattr(trans, inode, h->xattr_name,
> +					       NULL, 0, 0);
> +			else
> +				set_bit(BTRFS_INODE_HAS_PROPS,
> +					&BTRFS_I(inode)->runtime_flags);
> +		}
> +
>  		btrfs_block_rsv_release(fs_info, trans->block_rsv, num_bytes);
>  		if (ret)
> -			goto out;
> +			return ret;
>  	}
> -	ret = 0;
> -out:
> -	return ret;
> +
> +	return 0;
>  }
>  
>  int btrfs_inode_inherit_props(struct btrfs_trans_handle *trans,
>