Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 7 Sep 2022 07:55:45 -0700
From:   "Darrick J. Wong" <djwong@kernel.org>
To:     Zeng Heng <zengheng4@huawei.com>
Cc:     dchinner@redhat.com, chandan.babu@oracle.com,
        linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org,
        weiyongjun1@huawei.com, guoxuenan@huawei.com
Subject: Re: [PATCH -next] xfs: rmap: only XFS_DATA_FORK field would be set
 shared
Message-ID: <Yxiw8f55e0ZzjtNU@magnolia>
References: <20220906093554.899222-1-zengheng4@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220906093554.899222-1-zengheng4@huawei.com>
Precedence: bulk

On Tue, Sep 06, 2022 at 05:35:54PM +0800, Zeng Heng wrote:
> whichfork could be one of the following 3 types:
> XFS_DATA_FORK
> XFS_ATTR_FORK
> XFS_COW_FORK
> 
> XFS_COW_FORK is in-memory field instead of on-disk
> field, which doesn't need to update in rmap_btree
> (already preclude by `xfs_rmap_update_is_needed` ahead).
> 
> XFS_ATTR_FORK field doesn't support reflink share.
> 
> So here propose to set "whichfork == XFS_DATA_FORK"
> as condition.

As you already state, xfs_rmap_update_is_needed() filters out
whichfork==XFS_COW_FORK.  Why is this change necessary?  It looks like
pointless churn to me.

--D

> 
> Signed-off-by: Zeng Heng <zengheng4@huawei.com>
> ---
>  fs/xfs/libxfs/xfs_rmap.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> index 094dfc897ebc..74b34a331cf0 100644
> --- a/fs/xfs/libxfs/xfs_rmap.c
> +++ b/fs/xfs/libxfs/xfs_rmap.c
> @@ -2549,7 +2549,7 @@ xfs_rmap_map_extent(
>  	if (!xfs_rmap_update_is_needed(tp->t_mountp, whichfork))
>  		return;
>  
> -	if (whichfork != XFS_ATTR_FORK && xfs_is_reflink_inode(ip))
> +	if (whichfork == XFS_DATA_FORK && xfs_is_reflink_inode(ip))
>  		type = XFS_RMAP_MAP_SHARED;
>  
>  	__xfs_rmap_add(tp, type, ip->i_ino, whichfork, PREV);
> @@ -2568,7 +2568,7 @@ xfs_rmap_unmap_extent(
>  	if (!xfs_rmap_update_is_needed(tp->t_mountp, whichfork))
>  		return;
>  
> -	if (whichfork != XFS_ATTR_FORK && xfs_is_reflink_inode(ip))
> +	if (whichfork == XFS_DATA_FORK && xfs_is_reflink_inode(ip))
>  		type = XFS_RMAP_UNMAP_SHARED;
>  
>  	__xfs_rmap_add(tp, type, ip->i_ino, whichfork, PREV);
> @@ -2593,7 +2593,7 @@ xfs_rmap_convert_extent(
>  	if (!xfs_rmap_update_is_needed(mp, whichfork))
>  		return;
>  
> -	if (whichfork != XFS_ATTR_FORK && xfs_is_reflink_inode(ip))
> +	if (whichfork == XFS_DATA_FORK && xfs_is_reflink_inode(ip))
>  		type = XFS_RMAP_CONVERT_SHARED;
>  
>  	__xfs_rmap_add(tp, type, ip->i_ino, whichfork, PREV);
> -- 
> 2.25.1
>