[11/13] xfs: combine iunlink inode update functions

Message ID	20200812092556.2567285-12-david@fromorbit.com (mailing list archive)
State	Accepted
Headers	show Return-Path: <SRS0=vsMz=BW=vger.kernel.org=linux-xfs-owner@kernel.org> From: Dave Chinner <david@fromorbit.com> To: linux-xfs@vger.kernel.org Subject: [PATCH 11/13] xfs: combine iunlink inode update functions Date: Wed, 12 Aug 2020 19:25:54 +1000 Message-Id: <20200812092556.2567285-12-david@fromorbit.com> In-Reply-To: <20200812092556.2567285-1-david@fromorbit.com> References: <20200812092556.2567285-1-david@fromorbit.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk
Series	xfs: in memory inode unlink log items \| expand [00/13] xfs: in memory inode unlink log items [01/13] xfs: xfs_iflock is no longer a completion [02/13] xfs: add log item precommit operation [03/13] xfs: factor the xfs_iunlink functions [04/13] xfs: arrange all unlinked inodes into one list [05/13] xfs: add unlink list pointers to xfs_inode [06/13] xfs: replace iunlink backref lookups with list lookups [07/13] xfs: mapping unlinked inodes is now redundant [09/13] xfs: validate the unlinked list pointer on update [10/13] xfs: re-order AGI updates in unlink list updates [11/13] xfs: combine iunlink inode update functions [12/13] xfs: add in-memory iunlink log item [13/13] xfs: reorder iunlink remove operation in xfs_ifree

Message ID

20200812092556.2567285-12-david@fromorbit.com (mailing list archive)

State

Accepted

Headers

From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 11/13] xfs: combine iunlink inode update functions
Date: Wed, 12 Aug 2020 19:25:54 +1000
Message-Id: <20200812092556.2567285-12-david@fromorbit.com>
In-Reply-To: <20200812092556.2567285-1-david@fromorbit.com>
References: <20200812092556.2567285-1-david@fromorbit.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-xfs-owner@vger.kernel.org
Precedence: bulk

Series

xfs: in memory inode unlink log items | expand

Commit Message

Dave Chinner Aug. 12, 2020, 9:25 a.m. UTC

From: Dave Chinner <dchinner@redhat.com>

Combine the logging of the inode unlink list update into the
calling function that looks up the buffer we end up logging. These
do not need to be separate functions as they are both short, simple
operations and there's only a single call path through them. This
new function will end up being the core of the iunlink log item
processing...

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_inode.c | 58 ++++++++++++++++------------------------------
 1 file changed, 20 insertions(+), 38 deletions(-)

Comments

Darrick J. Wong Aug. 19, 2020, 12:30 a.m. UTC | #1

On Wed, Aug 12, 2020 at 07:25:54PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Combine the logging of the inode unlink list update into the
> calling function that looks up the buffer we end up logging. These
> do not need to be separate functions as they are both short, simple
> operations and there's only a single call path through them. This
> new function will end up being the core of the iunlink log item
> processing...
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

This was pretty easy to follow,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_inode.c | 58 ++++++++++++++++------------------------------
>  1 file changed, 20 insertions(+), 38 deletions(-)
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 4f616e1b64dc..82242d15b1d7 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1972,38 +1972,12 @@ xfs_iunlink_update_bucket(
>  	return 0;
>  }
>  
> -/* Set an on-disk inode's next_unlinked pointer. */
> -STATIC void
> -xfs_iunlink_update_dinode(
> -	struct xfs_trans	*tp,
> -	xfs_agnumber_t		agno,
> -	xfs_agino_t		agino,
> -	struct xfs_buf		*ibp,
> -	struct xfs_dinode	*dip,
> -	struct xfs_imap		*imap,
> -	xfs_agino_t		next_agino)
> -{
> -	struct xfs_mount	*mp = tp->t_mountp;
> -	int			offset;
> -
> -	ASSERT(xfs_verify_agino_or_null(mp, agno, next_agino));
> -
> -	trace_xfs_iunlink_update_dinode(mp, agno, agino,
> -			be32_to_cpu(dip->di_next_unlinked), next_agino);
> -
> -	dip->di_next_unlinked = cpu_to_be32(next_agino);
> -	offset = imap->im_boffset +
> -			offsetof(struct xfs_dinode, di_next_unlinked);
> -
> -	/* need to recalc the inode CRC if appropriate */
> -	xfs_dinode_calc_crc(mp, dip);
> -	xfs_trans_inode_buf(tp, ibp);
> -	xfs_trans_log_buf(tp, ibp, offset, offset + sizeof(xfs_agino_t) - 1);
> -}
> -
> -/* Set an in-core inode's unlinked pointer and return the old value. */
> +/*
> + * Look up the inode cluster buffer and log the on-disk unlinked inode change
> + * we need to make.
> + */
>  STATIC int
> -xfs_iunlink_update_inode(
> +xfs_iunlink_log_inode(
>  	struct xfs_trans	*tp,
>  	struct xfs_inode	*ip,
>  	xfs_agnumber_t		agno,
> @@ -2013,6 +1987,7 @@ xfs_iunlink_update_inode(
>  	struct xfs_mount	*mp = tp->t_mountp;
>  	struct xfs_dinode	*dip;
>  	struct xfs_buf		*ibp;
> +	int			offset;
>  	int			error;
>  
>  	ASSERT(xfs_verify_agino_or_null(mp, agno, next_agino));
> @@ -2028,9 +2003,17 @@ xfs_iunlink_update_inode(
>  		return -EFSCORRUPTED;
>  	}
>  
> -	/* Ok, update the new pointer. */
> -	xfs_iunlink_update_dinode(tp, agno, XFS_INO_TO_AGINO(mp, ip->i_ino),
> -			ibp, dip, &ip->i_imap, next_agino);
> +	trace_xfs_iunlink_update_dinode(mp, agno,
> +			XFS_INO_TO_AGINO(mp, ip->i_ino),
> +			be32_to_cpu(dip->di_next_unlinked), next_agino);
> +
> +	dip->di_next_unlinked = cpu_to_be32(next_agino);
> +	offset = ip->i_imap.im_boffset +
> +			offsetof(struct xfs_dinode, di_next_unlinked);
> +
> +	xfs_dinode_calc_crc(mp, dip);
> +	xfs_trans_inode_buf(tp, ibp);
> +	xfs_trans_log_buf(tp, ibp, offset, offset + sizeof(xfs_agino_t) - 1);
>  	return 0;
>  }
>  
> @@ -2056,7 +2039,7 @@ xfs_iunlink_insert_inode(
>  		 * inode to the current head of the list.
>  		 */
>  		next_agino = XFS_INO_TO_AGINO(mp, nip->i_ino);
> -		error = xfs_iunlink_update_inode(tp, ip, agno, NULLAGINO,
> +		error = xfs_iunlink_log_inode(tp, ip, agno, NULLAGINO,
>  						 next_agino);
>  		if (error)
>  			return error;
> @@ -2129,7 +2112,7 @@ xfs_iunlink_remove_inode(
>  	}
>  
>  	/* Clear the on disk next unlinked pointer for this inode. */
> -	error = xfs_iunlink_update_inode(tp, ip, agno, next_agino, NULLAGINO);
> +	error = xfs_iunlink_log_inode(tp, ip, agno, next_agino, NULLAGINO);
>  	if (error)
>  		return error;
>  
> @@ -2138,8 +2121,7 @@ xfs_iunlink_remove_inode(
>  					struct xfs_inode, i_unlink)) {
>  		struct xfs_inode *pip = list_prev_entry(ip, i_unlink);
>  
> -		return xfs_iunlink_update_inode(tp, pip, agno, agino,
> -						next_agino);
> +		return xfs_iunlink_log_inode(tp, pip, agno, agino, next_agino);
>  	}
>  
>  	/* Point the head of the list to the next unlinked inode. */
> -- 
> 2.26.2.761.g0e0b3e54be
>

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 4f616e1b64dc..82242d15b1d7 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1972,38 +1972,12 @@  xfs_iunlink_update_bucket(
 	return 0;
 }
 
-/* Set an on-disk inode's next_unlinked pointer. */
-STATIC void
-xfs_iunlink_update_dinode(
-	struct xfs_trans	*tp,
-	xfs_agnumber_t		agno,
-	xfs_agino_t		agino,
-	struct xfs_buf		*ibp,
-	struct xfs_dinode	*dip,
-	struct xfs_imap		*imap,
-	xfs_agino_t		next_agino)
-{
-	struct xfs_mount	*mp = tp->t_mountp;
-	int			offset;
-
-	ASSERT(xfs_verify_agino_or_null(mp, agno, next_agino));
-
-	trace_xfs_iunlink_update_dinode(mp, agno, agino,
-			be32_to_cpu(dip->di_next_unlinked), next_agino);
-
-	dip->di_next_unlinked = cpu_to_be32(next_agino);
-	offset = imap->im_boffset +
-			offsetof(struct xfs_dinode, di_next_unlinked);
-
-	/* need to recalc the inode CRC if appropriate */
-	xfs_dinode_calc_crc(mp, dip);
-	xfs_trans_inode_buf(tp, ibp);
-	xfs_trans_log_buf(tp, ibp, offset, offset + sizeof(xfs_agino_t) - 1);
-}
-
-/* Set an in-core inode's unlinked pointer and return the old value. */
+/*
+ * Look up the inode cluster buffer and log the on-disk unlinked inode change
+ * we need to make.
+ */
 STATIC int
-xfs_iunlink_update_inode(
+xfs_iunlink_log_inode(
 	struct xfs_trans	*tp,
 	struct xfs_inode	*ip,
 	xfs_agnumber_t		agno,
@@ -2013,6 +1987,7 @@  xfs_iunlink_update_inode(
 	struct xfs_mount	*mp = tp->t_mountp;
 	struct xfs_dinode	*dip;
 	struct xfs_buf		*ibp;
+	int			offset;
 	int			error;
 
 	ASSERT(xfs_verify_agino_or_null(mp, agno, next_agino));
@@ -2028,9 +2003,17 @@  xfs_iunlink_update_inode(
 		return -EFSCORRUPTED;
 	}
 
-	/* Ok, update the new pointer. */
-	xfs_iunlink_update_dinode(tp, agno, XFS_INO_TO_AGINO(mp, ip->i_ino),
-			ibp, dip, &ip->i_imap, next_agino);
+	trace_xfs_iunlink_update_dinode(mp, agno,
+			XFS_INO_TO_AGINO(mp, ip->i_ino),
+			be32_to_cpu(dip->di_next_unlinked), next_agino);
+
+	dip->di_next_unlinked = cpu_to_be32(next_agino);
+	offset = ip->i_imap.im_boffset +
+			offsetof(struct xfs_dinode, di_next_unlinked);
+
+	xfs_dinode_calc_crc(mp, dip);
+	xfs_trans_inode_buf(tp, ibp);
+	xfs_trans_log_buf(tp, ibp, offset, offset + sizeof(xfs_agino_t) - 1);
 	return 0;
 }
 
@@ -2056,7 +2039,7 @@  xfs_iunlink_insert_inode(
 		 * inode to the current head of the list.
 		 */
 		next_agino = XFS_INO_TO_AGINO(mp, nip->i_ino);
-		error = xfs_iunlink_update_inode(tp, ip, agno, NULLAGINO,
+		error = xfs_iunlink_log_inode(tp, ip, agno, NULLAGINO,
 						 next_agino);
 		if (error)
 			return error;
@@ -2129,7 +2112,7 @@  xfs_iunlink_remove_inode(
 	}
 
 	/* Clear the on disk next unlinked pointer for this inode. */
-	error = xfs_iunlink_update_inode(tp, ip, agno, next_agino, NULLAGINO);
+	error = xfs_iunlink_log_inode(tp, ip, agno, next_agino, NULLAGINO);
 	if (error)
 		return error;
 
@@ -2138,8 +2121,7 @@  xfs_iunlink_remove_inode(
 					struct xfs_inode, i_unlink)) {
 		struct xfs_inode *pip = list_prev_entry(ip, i_unlink);
 
-		return xfs_iunlink_update_inode(tp, pip, agno, agino,
-						next_agino);
+		return xfs_iunlink_log_inode(tp, pip, agno, agino, next_agino);
 	}
 
 	/* Point the head of the list to the next unlinked inode. */

[11/13] xfs: combine iunlink inode update functions

Commit Message

Comments

Patch