diff mbox series

xfs: avoid shared rmap operations for attr fork extents

Message ID 20200923182340.GV7955@magnolia (mailing list archive)
State Superseded
Headers show
Series xfs: avoid shared rmap operations for attr fork extents | expand

Commit Message

Darrick J. Wong Sept. 23, 2020, 6:23 p.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

During code review, I noticed that the rmap code uses the (slower)
shared mappings rmap functions for any extent of a reflinked file, even
if those extents are for the attr fork, which doesn't support sharing.
We can speed up rmap a tiny bit by optimizing out this case.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_rmap.c |   27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

Comments

Chandan Babu R Sept. 24, 2020, 4:44 a.m. UTC | #1 
On Wednesday 23 September 2020 11:53:40 PM IST Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> During code review, I noticed that the rmap code uses the (slower)
> shared mappings rmap functions for any extent of a reflinked file, even
> if those extents are for the attr fork, which doesn't support sharing.
> We can speed up rmap a tiny bit by optimizing out this case.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good to me.

Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>

> ---
>  fs/xfs/libxfs/xfs_rmap.c |   27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
> index 27c39268c31f..340c83f76c80 100644
> --- a/fs/xfs/libxfs/xfs_rmap.c
> +++ b/fs/xfs/libxfs/xfs_rmap.c
> @@ -2505,12 +2505,15 @@ xfs_rmap_map_extent(
>  	int			whichfork,
>  	struct xfs_bmbt_irec	*PREV)
>  {
> +	enum xfs_rmap_intent_type type = XFS_RMAP_MAP;
> +
>  	if (!xfs_rmap_update_is_needed(tp->t_mountp, whichfork))
>  		return;
>  
> -	__xfs_rmap_add(tp, xfs_is_reflink_inode(ip) ?
> -			XFS_RMAP_MAP_SHARED : XFS_RMAP_MAP, ip->i_ino,
> -			whichfork, PREV);
> +	if (whichfork != XFS_ATTR_FORK && xfs_is_reflink_inode(ip))
> +		type = XFS_RMAP_MAP_SHARED;
> +
> +	__xfs_rmap_add(tp, type, ip->i_ino, whichfork, PREV);
>  }
>  
>  /* Unmap an extent out of a file. */
> @@ -2521,12 +2524,15 @@ xfs_rmap_unmap_extent(
>  	int			whichfork,
>  	struct xfs_bmbt_irec	*PREV)
>  {
> +	enum xfs_rmap_intent_type type = XFS_RMAP_UNMAP;
> +
>  	if (!xfs_rmap_update_is_needed(tp->t_mountp, whichfork))
>  		return;
>  
> -	__xfs_rmap_add(tp, xfs_is_reflink_inode(ip) ?
> -			XFS_RMAP_UNMAP_SHARED : XFS_RMAP_UNMAP, ip->i_ino,
> -			whichfork, PREV);
> +	if (whichfork != XFS_ATTR_FORK && xfs_is_reflink_inode(ip))
> +		type = XFS_RMAP_UNMAP_SHARED;
> +
> +	__xfs_rmap_add(tp, type, ip->i_ino, whichfork, PREV);
>  }
>  
>  /*
> @@ -2543,12 +2549,15 @@ xfs_rmap_convert_extent(
>  	int			whichfork,
>  	struct xfs_bmbt_irec	*PREV)
>  {
> +	enum xfs_rmap_intent_type type = XFS_RMAP_CONVERT;
> +
>  	if (!xfs_rmap_update_is_needed(mp, whichfork))
>  		return;
>  
> -	__xfs_rmap_add(tp, xfs_is_reflink_inode(ip) ?
> -			XFS_RMAP_CONVERT_SHARED : XFS_RMAP_CONVERT, ip->i_ino,
> -			whichfork, PREV);
> +	if (whichfork != XFS_ATTR_FORK && xfs_is_reflink_inode(ip))
> +		type = XFS_RMAP_CONVERT_SHARED;
> +
> +	__xfs_rmap_add(tp, type, ip->i_ino, whichfork, PREV);
>  }
>  
>  /* Schedule the creation of an rmap for non-file data. */
>
Christoph Hellwig Sept. 24, 2020, 5:44 a.m. UTC | #2 
On Wed, Sep 23, 2020 at 11:23:40AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> During code review, I noticed that the rmap code uses the (slower)
> shared mappings rmap functions for any extent of a reflinked file, even
> if those extents are for the attr fork, which doesn't support sharing.
> We can speed up rmap a tiny bit by optimizing out this case.

This looks correct:

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
index 27c39268c31f..340c83f76c80 100644
--- a/fs/xfs/libxfs/xfs_rmap.c
+++ b/fs/xfs/libxfs/xfs_rmap.c
@@ -2505,12 +2505,15 @@  xfs_rmap_map_extent(
 	int			whichfork,
 	struct xfs_bmbt_irec	*PREV)
 {
+	enum xfs_rmap_intent_type type = XFS_RMAP_MAP;
+
 	if (!xfs_rmap_update_is_needed(tp->t_mountp, whichfork))
 		return;
 
-	__xfs_rmap_add(tp, xfs_is_reflink_inode(ip) ?
-			XFS_RMAP_MAP_SHARED : XFS_RMAP_MAP, ip->i_ino,
-			whichfork, PREV);
+	if (whichfork != XFS_ATTR_FORK && xfs_is_reflink_inode(ip))
+		type = XFS_RMAP_MAP_SHARED;
+
+	__xfs_rmap_add(tp, type, ip->i_ino, whichfork, PREV);
 }
 
 /* Unmap an extent out of a file. */
@@ -2521,12 +2524,15 @@  xfs_rmap_unmap_extent(
 	int			whichfork,
 	struct xfs_bmbt_irec	*PREV)
 {
+	enum xfs_rmap_intent_type type = XFS_RMAP_UNMAP;
+
 	if (!xfs_rmap_update_is_needed(tp->t_mountp, whichfork))
 		return;
 
-	__xfs_rmap_add(tp, xfs_is_reflink_inode(ip) ?
-			XFS_RMAP_UNMAP_SHARED : XFS_RMAP_UNMAP, ip->i_ino,
-			whichfork, PREV);
+	if (whichfork != XFS_ATTR_FORK && xfs_is_reflink_inode(ip))
+		type = XFS_RMAP_UNMAP_SHARED;
+
+	__xfs_rmap_add(tp, type, ip->i_ino, whichfork, PREV);
 }
 
 /*
@@ -2543,12 +2549,15 @@  xfs_rmap_convert_extent(
 	int			whichfork,
 	struct xfs_bmbt_irec	*PREV)
 {
+	enum xfs_rmap_intent_type type = XFS_RMAP_CONVERT;
+
 	if (!xfs_rmap_update_is_needed(mp, whichfork))
 		return;
 
-	__xfs_rmap_add(tp, xfs_is_reflink_inode(ip) ?
-			XFS_RMAP_CONVERT_SHARED : XFS_RMAP_CONVERT, ip->i_ino,
-			whichfork, PREV);
+	if (whichfork != XFS_ATTR_FORK && xfs_is_reflink_inode(ip))
+		type = XFS_RMAP_CONVERT_SHARED;
+
+	__xfs_rmap_add(tp, type, ip->i_ino, whichfork, PREV);
 }
 
 /* Schedule the creation of an rmap for non-file data. */