[04/11] xfs: merge xlog_commit_record with xlog_write_done()
diff mbox series

Message ID 20200304075401.21558-5-david@fromorbit.com
State Superseded
Headers show
Series
  • xfs: clean up log tickets and record writes
Related show

Commit Message

Dave Chinner March 4, 2020, 7:53 a.m. UTC
From: Dave Chinner <dchinner@redhat.com>

xlog_write_done() is just a thin wrapper around
xlog_commit_record(), so they can be merged together easily. Convert
all the xlog_commit_record() callers to use xlog_write_done() and
merge the implementations.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_log.c | 60 +++++++++++++++---------------------------------
 1 file changed, 19 insertions(+), 41 deletions(-)

Comments

Christoph Hellwig March 4, 2020, 3:50 p.m. UTC | #1
On Wed, Mar 04, 2020 at 06:53:54PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> xlog_write_done() is just a thin wrapper around
> xlog_commit_record(), so they can be merged together easily. Convert
> all the xlog_commit_record() callers to use xlog_write_done() and
> merge the implementations.

Ok, you are doing the merge here.  I think it would be nicer to just
simply do that from the beginning, though.
Brian Foster March 5, 2020, 6:06 p.m. UTC | #2
On Wed, Mar 04, 2020 at 06:53:54PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> xlog_write_done() is just a thin wrapper around
> xlog_commit_record(), so they can be merged together easily. Convert
> all the xlog_commit_record() callers to use xlog_write_done() and
> merge the implementations.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

I think Christoph disagrees but I actually prefer to see the incremental
steps:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_log.c | 60 +++++++++++++++---------------------------------
>  1 file changed, 19 insertions(+), 41 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 702b38e4db6e..100eeaed4a7d 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -24,13 +24,6 @@
>  kmem_zone_t	*xfs_log_ticket_zone;
>  
>  /* Local miscellaneous function prototypes */
> -STATIC int
> -xlog_commit_record(
> -	struct xlog		*log,
> -	struct xlog_ticket	*ticket,
> -	struct xlog_in_core	**iclog,
> -	xfs_lsn_t		*commitlsnp);
> -
>  STATIC struct xlog *
>  xlog_alloc_log(
>  	struct xfs_mount	*mp,
> @@ -493,7 +486,8 @@ xfs_log_reserve(
>   */
>  
>  /*
> - * Write a commit record to the log to close off a running log write.
> + * Write out the commit record of a transaction associated with the given
> + * ticket to close off a running log write. Return the lsn of the commit record.
>   */
>  int
>  xlog_write_done(
> @@ -502,10 +496,26 @@ xlog_write_done(
>  	struct xlog_in_core	**iclog,
>  	xfs_lsn_t		*lsn)
>  {
> +	struct xfs_log_iovec reg = {
> +		.i_addr = NULL,
> +		.i_len = 0,
> +		.i_type = XLOG_REG_TYPE_COMMIT,
> +	};
> +	struct xfs_log_vec vec = {
> +		.lv_niovecs = 1,
> +		.lv_iovecp = &reg,
> +	};
> +	int	error;
> +
> +	ASSERT_ALWAYS(iclog);
> +
>  	if (XLOG_FORCED_SHUTDOWN(log))
>  		return -EIO;
>  
> -	return xlog_commit_record(log, ticket, iclog, lsn);
> +	error = xlog_write(log, &vec, ticket, lsn, iclog, XLOG_COMMIT_TRANS);
> +	if (error)
> +		xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
> +	return error;
>  }
>  
>  /*
> @@ -1529,38 +1539,6 @@ xlog_alloc_log(
>  	return ERR_PTR(error);
>  }	/* xlog_alloc_log */
>  
> -
> -/*
> - * Write out the commit record of a transaction associated with the given
> - * ticket.  Return the lsn of the commit record.
> - */
> -STATIC int
> -xlog_commit_record(
> -	struct xlog		*log,
> -	struct xlog_ticket	*ticket,
> -	struct xlog_in_core	**iclog,
> -	xfs_lsn_t		*commitlsnp)
> -{
> -	struct xfs_mount *mp = log->l_mp;
> -	int	error;
> -	struct xfs_log_iovec reg = {
> -		.i_addr = NULL,
> -		.i_len = 0,
> -		.i_type = XLOG_REG_TYPE_COMMIT,
> -	};
> -	struct xfs_log_vec vec = {
> -		.lv_niovecs = 1,
> -		.lv_iovecp = &reg,
> -	};
> -
> -	ASSERT_ALWAYS(iclog);
> -	error = xlog_write(log, &vec, ticket, commitlsnp, iclog,
> -					XLOG_COMMIT_TRANS);
> -	if (error)
> -		xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR);
> -	return error;
> -}
> -
>  /*
>   * Push on the buffer cache code if we ever use more than 75% of the on-disk
>   * log space.  This code pushes on the lsn which would supposedly free up
> -- 
> 2.24.0.rc0
>

Patch
diff mbox series

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 702b38e4db6e..100eeaed4a7d 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -24,13 +24,6 @@ 
 kmem_zone_t	*xfs_log_ticket_zone;
 
 /* Local miscellaneous function prototypes */
-STATIC int
-xlog_commit_record(
-	struct xlog		*log,
-	struct xlog_ticket	*ticket,
-	struct xlog_in_core	**iclog,
-	xfs_lsn_t		*commitlsnp);
-
 STATIC struct xlog *
 xlog_alloc_log(
 	struct xfs_mount	*mp,
@@ -493,7 +486,8 @@  xfs_log_reserve(
  */
 
 /*
- * Write a commit record to the log to close off a running log write.
+ * Write out the commit record of a transaction associated with the given
+ * ticket to close off a running log write. Return the lsn of the commit record.
  */
 int
 xlog_write_done(
@@ -502,10 +496,26 @@  xlog_write_done(
 	struct xlog_in_core	**iclog,
 	xfs_lsn_t		*lsn)
 {
+	struct xfs_log_iovec reg = {
+		.i_addr = NULL,
+		.i_len = 0,
+		.i_type = XLOG_REG_TYPE_COMMIT,
+	};
+	struct xfs_log_vec vec = {
+		.lv_niovecs = 1,
+		.lv_iovecp = &reg,
+	};
+	int	error;
+
+	ASSERT_ALWAYS(iclog);
+
 	if (XLOG_FORCED_SHUTDOWN(log))
 		return -EIO;
 
-	return xlog_commit_record(log, ticket, iclog, lsn);
+	error = xlog_write(log, &vec, ticket, lsn, iclog, XLOG_COMMIT_TRANS);
+	if (error)
+		xfs_force_shutdown(log->l_mp, SHUTDOWN_LOG_IO_ERROR);
+	return error;
 }
 
 /*
@@ -1529,38 +1539,6 @@  xlog_alloc_log(
 	return ERR_PTR(error);
 }	/* xlog_alloc_log */
 
-
-/*
- * Write out the commit record of a transaction associated with the given
- * ticket.  Return the lsn of the commit record.
- */
-STATIC int
-xlog_commit_record(
-	struct xlog		*log,
-	struct xlog_ticket	*ticket,
-	struct xlog_in_core	**iclog,
-	xfs_lsn_t		*commitlsnp)
-{
-	struct xfs_mount *mp = log->l_mp;
-	int	error;
-	struct xfs_log_iovec reg = {
-		.i_addr = NULL,
-		.i_len = 0,
-		.i_type = XLOG_REG_TYPE_COMMIT,
-	};
-	struct xfs_log_vec vec = {
-		.lv_niovecs = 1,
-		.lv_iovecp = &reg,
-	};
-
-	ASSERT_ALWAYS(iclog);
-	error = xlog_write(log, &vec, ticket, commitlsnp, iclog,
-					XLOG_COMMIT_TRANS);
-	if (error)
-		xfs_force_shutdown(mp, SHUTDOWN_LOG_IO_ERROR);
-	return error;
-}
-
 /*
  * Push on the buffer cache code if we ever use more than 75% of the on-disk
  * log space.  This code pushes on the lsn which would supposedly free up