diff mbox series

[02/14] xfs: factor out a xlog_wait_on_iclog helper

Message ID 20200316144233.900390-3-hch@lst.de (mailing list archive)
State Superseded
Headers show
Series [01/14] xfs: merge xlog_cil_push into xlog_cil_push_work | expand

Commit Message

Christoph Hellwig March 16, 2020, 2:42 p.m. UTC
Factor out the shared code to wait for a log force into a new helper.
This helper uses the XLOG_FORCED_SHUTDOWN check previous only used
by the unmount code over the equivalent iclog ioerror state used by
the other two functions.

There is a slight behavior change in that the force of the unmount
record is now accounted in the log force statistics.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c | 76 ++++++++++++++++++++----------------------------
 1 file changed, 31 insertions(+), 45 deletions(-)

Comments

Darrick J. Wong March 16, 2020, 8:20 p.m. UTC | #1
On Mon, Mar 16, 2020 at 03:42:21PM +0100, Christoph Hellwig wrote:
> Factor out the shared code to wait for a log force into a new helper.
> This helper uses the XLOG_FORCED_SHUTDOWN check previous only used
> by the unmount code over the equivalent iclog ioerror state used by
> the other two functions.
> 
> There is a slight behavior change in that the force of the unmount
> record is now accounted in the log force statistics.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok, though I reserve the right to become confused later on. :)
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_log.c | 76 ++++++++++++++++++++----------------------------
>  1 file changed, 31 insertions(+), 45 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 0986983ef6b5..955df2902c2c 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -859,6 +859,31 @@ xfs_log_mount_cancel(
>  	xfs_log_unmount(mp);
>  }
>  
> +/*
> + * Wait for the iclog to be written disk, or return an error if the log has been
> + * shut down.
> + */
> +static int
> +xlog_wait_on_iclog(
> +	struct xlog_in_core	*iclog)
> +		__releases(iclog->ic_log->l_icloglock)
> +{
> +	struct xlog		*log = iclog->ic_log;
> +
> +	if (!XLOG_FORCED_SHUTDOWN(log) &&
> +	    iclog->ic_state != XLOG_STATE_ACTIVE &&
> +	    iclog->ic_state != XLOG_STATE_DIRTY) {
> +		XFS_STATS_INC(log->l_mp, xs_log_force_sleep);
> +		xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
> +	} else {
> +		spin_unlock(&log->l_icloglock);
> +	}
> +
> +	if (XLOG_FORCED_SHUTDOWN(log))
> +		return -EIO;
> +	return 0;
> +}
> +
>  /*
>   * Final log writes as part of unmount.
>   *
> @@ -926,18 +951,7 @@ xfs_log_write_unmount_record(
>  	atomic_inc(&iclog->ic_refcnt);
>  	xlog_state_want_sync(log, iclog);
>  	error = xlog_state_release_iclog(log, iclog);
> -	switch (iclog->ic_state) {
> -	default:
> -		if (!XLOG_FORCED_SHUTDOWN(log)) {
> -			xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
> -			break;
> -		}
> -		/* fall through */
> -	case XLOG_STATE_ACTIVE:
> -	case XLOG_STATE_DIRTY:
> -		spin_unlock(&log->l_icloglock);
> -		break;
> -	}
> +	xlog_wait_on_iclog(iclog);
>  
>  	if (tic) {
>  		trace_xfs_log_umount_write(log, tic);
> @@ -3230,9 +3244,6 @@ xfs_log_force(
>  		 * previous iclog and go to sleep.
>  		 */
>  		iclog = iclog->ic_prev;
> -		if (iclog->ic_state == XLOG_STATE_ACTIVE ||
> -		    iclog->ic_state == XLOG_STATE_DIRTY)
> -			goto out_unlock;
>  	} else if (iclog->ic_state == XLOG_STATE_ACTIVE) {
>  		if (atomic_read(&iclog->ic_refcnt) == 0) {
>  			/*
> @@ -3248,8 +3259,7 @@ xfs_log_force(
>  			if (xlog_state_release_iclog(log, iclog))
>  				goto out_error;
>  
> -			if (be64_to_cpu(iclog->ic_header.h_lsn) != lsn ||
> -			    iclog->ic_state == XLOG_STATE_DIRTY)
> +			if (be64_to_cpu(iclog->ic_header.h_lsn) != lsn)
>  				goto out_unlock;
>  		} else {
>  			/*
> @@ -3269,17 +3279,8 @@ xfs_log_force(
>  		;
>  	}
>  
> -	if (!(flags & XFS_LOG_SYNC))
> -		goto out_unlock;
> -
> -	if (iclog->ic_state == XLOG_STATE_IOERROR)
> -		goto out_error;
> -	XFS_STATS_INC(mp, xs_log_force_sleep);
> -	xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
> -	if (iclog->ic_state == XLOG_STATE_IOERROR)
> -		return -EIO;
> -	return 0;
> -
> +	if (flags & XFS_LOG_SYNC)
> +		return xlog_wait_on_iclog(iclog);
>  out_unlock:
>  	spin_unlock(&log->l_icloglock);
>  	return 0;
> @@ -3310,9 +3311,6 @@ __xfs_log_force_lsn(
>  			goto out_unlock;
>  	}
>  
> -	if (iclog->ic_state == XLOG_STATE_DIRTY)
> -		goto out_unlock;
> -
>  	if (iclog->ic_state == XLOG_STATE_ACTIVE) {
>  		/*
>  		 * We sleep here if we haven't already slept (e.g. this is the
> @@ -3346,20 +3344,8 @@ __xfs_log_force_lsn(
>  			*log_flushed = 1;
>  	}
>  
> -	if (!(flags & XFS_LOG_SYNC) ||
> -	    (iclog->ic_state == XLOG_STATE_ACTIVE ||
> -	     iclog->ic_state == XLOG_STATE_DIRTY))
> -		goto out_unlock;
> -
> -	if (iclog->ic_state == XLOG_STATE_IOERROR)
> -		goto out_error;
> -
> -	XFS_STATS_INC(mp, xs_log_force_sleep);
> -	xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
> -	if (iclog->ic_state == XLOG_STATE_IOERROR)
> -		return -EIO;
> -	return 0;
> -
> +	if (flags & XFS_LOG_SYNC)
> +		return xlog_wait_on_iclog(iclog);
>  out_unlock:
>  	spin_unlock(&log->l_icloglock);
>  	return 0;
> -- 
> 2.24.1
>
Brian Foster March 17, 2020, 1:23 p.m. UTC | #2
On Mon, Mar 16, 2020 at 03:42:21PM +0100, Christoph Hellwig wrote:
> Factor out the shared code to wait for a log force into a new helper.
> This helper uses the XLOG_FORCED_SHUTDOWN check previous only used
> by the unmount code over the equivalent iclog ioerror state used by
> the other two functions.
> 
> There is a slight behavior change in that the force of the unmount
> record is now accounted in the log force statistics.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

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

>  fs/xfs/xfs_log.c | 76 ++++++++++++++++++++----------------------------
>  1 file changed, 31 insertions(+), 45 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 0986983ef6b5..955df2902c2c 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -859,6 +859,31 @@ xfs_log_mount_cancel(
>  	xfs_log_unmount(mp);
>  }
>  
> +/*
> + * Wait for the iclog to be written disk, or return an error if the log has been
> + * shut down.
> + */
> +static int
> +xlog_wait_on_iclog(
> +	struct xlog_in_core	*iclog)
> +		__releases(iclog->ic_log->l_icloglock)
> +{
> +	struct xlog		*log = iclog->ic_log;
> +
> +	if (!XLOG_FORCED_SHUTDOWN(log) &&
> +	    iclog->ic_state != XLOG_STATE_ACTIVE &&
> +	    iclog->ic_state != XLOG_STATE_DIRTY) {
> +		XFS_STATS_INC(log->l_mp, xs_log_force_sleep);
> +		xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
> +	} else {
> +		spin_unlock(&log->l_icloglock);
> +	}
> +
> +	if (XLOG_FORCED_SHUTDOWN(log))
> +		return -EIO;
> +	return 0;
> +}
> +
>  /*
>   * Final log writes as part of unmount.
>   *
> @@ -926,18 +951,7 @@ xfs_log_write_unmount_record(
>  	atomic_inc(&iclog->ic_refcnt);
>  	xlog_state_want_sync(log, iclog);
>  	error = xlog_state_release_iclog(log, iclog);
> -	switch (iclog->ic_state) {
> -	default:
> -		if (!XLOG_FORCED_SHUTDOWN(log)) {
> -			xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
> -			break;
> -		}
> -		/* fall through */
> -	case XLOG_STATE_ACTIVE:
> -	case XLOG_STATE_DIRTY:
> -		spin_unlock(&log->l_icloglock);
> -		break;
> -	}
> +	xlog_wait_on_iclog(iclog);
>  
>  	if (tic) {
>  		trace_xfs_log_umount_write(log, tic);
> @@ -3230,9 +3244,6 @@ xfs_log_force(
>  		 * previous iclog and go to sleep.
>  		 */
>  		iclog = iclog->ic_prev;
> -		if (iclog->ic_state == XLOG_STATE_ACTIVE ||
> -		    iclog->ic_state == XLOG_STATE_DIRTY)
> -			goto out_unlock;
>  	} else if (iclog->ic_state == XLOG_STATE_ACTIVE) {
>  		if (atomic_read(&iclog->ic_refcnt) == 0) {
>  			/*
> @@ -3248,8 +3259,7 @@ xfs_log_force(
>  			if (xlog_state_release_iclog(log, iclog))
>  				goto out_error;
>  
> -			if (be64_to_cpu(iclog->ic_header.h_lsn) != lsn ||
> -			    iclog->ic_state == XLOG_STATE_DIRTY)
> +			if (be64_to_cpu(iclog->ic_header.h_lsn) != lsn)
>  				goto out_unlock;
>  		} else {
>  			/*
> @@ -3269,17 +3279,8 @@ xfs_log_force(
>  		;
>  	}
>  
> -	if (!(flags & XFS_LOG_SYNC))
> -		goto out_unlock;
> -
> -	if (iclog->ic_state == XLOG_STATE_IOERROR)
> -		goto out_error;
> -	XFS_STATS_INC(mp, xs_log_force_sleep);
> -	xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
> -	if (iclog->ic_state == XLOG_STATE_IOERROR)
> -		return -EIO;
> -	return 0;
> -
> +	if (flags & XFS_LOG_SYNC)
> +		return xlog_wait_on_iclog(iclog);
>  out_unlock:
>  	spin_unlock(&log->l_icloglock);
>  	return 0;
> @@ -3310,9 +3311,6 @@ __xfs_log_force_lsn(
>  			goto out_unlock;
>  	}
>  
> -	if (iclog->ic_state == XLOG_STATE_DIRTY)
> -		goto out_unlock;
> -
>  	if (iclog->ic_state == XLOG_STATE_ACTIVE) {
>  		/*
>  		 * We sleep here if we haven't already slept (e.g. this is the
> @@ -3346,20 +3344,8 @@ __xfs_log_force_lsn(
>  			*log_flushed = 1;
>  	}
>  
> -	if (!(flags & XFS_LOG_SYNC) ||
> -	    (iclog->ic_state == XLOG_STATE_ACTIVE ||
> -	     iclog->ic_state == XLOG_STATE_DIRTY))
> -		goto out_unlock;
> -
> -	if (iclog->ic_state == XLOG_STATE_IOERROR)
> -		goto out_error;
> -
> -	XFS_STATS_INC(mp, xs_log_force_sleep);
> -	xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
> -	if (iclog->ic_state == XLOG_STATE_IOERROR)
> -		return -EIO;
> -	return 0;
> -
> +	if (flags & XFS_LOG_SYNC)
> +		return xlog_wait_on_iclog(iclog);
>  out_unlock:
>  	spin_unlock(&log->l_icloglock);
>  	return 0;
> -- 
> 2.24.1
>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 0986983ef6b5..955df2902c2c 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -859,6 +859,31 @@  xfs_log_mount_cancel(
 	xfs_log_unmount(mp);
 }
 
+/*
+ * Wait for the iclog to be written disk, or return an error if the log has been
+ * shut down.
+ */
+static int
+xlog_wait_on_iclog(
+	struct xlog_in_core	*iclog)
+		__releases(iclog->ic_log->l_icloglock)
+{
+	struct xlog		*log = iclog->ic_log;
+
+	if (!XLOG_FORCED_SHUTDOWN(log) &&
+	    iclog->ic_state != XLOG_STATE_ACTIVE &&
+	    iclog->ic_state != XLOG_STATE_DIRTY) {
+		XFS_STATS_INC(log->l_mp, xs_log_force_sleep);
+		xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
+	} else {
+		spin_unlock(&log->l_icloglock);
+	}
+
+	if (XLOG_FORCED_SHUTDOWN(log))
+		return -EIO;
+	return 0;
+}
+
 /*
  * Final log writes as part of unmount.
  *
@@ -926,18 +951,7 @@  xfs_log_write_unmount_record(
 	atomic_inc(&iclog->ic_refcnt);
 	xlog_state_want_sync(log, iclog);
 	error = xlog_state_release_iclog(log, iclog);
-	switch (iclog->ic_state) {
-	default:
-		if (!XLOG_FORCED_SHUTDOWN(log)) {
-			xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
-			break;
-		}
-		/* fall through */
-	case XLOG_STATE_ACTIVE:
-	case XLOG_STATE_DIRTY:
-		spin_unlock(&log->l_icloglock);
-		break;
-	}
+	xlog_wait_on_iclog(iclog);
 
 	if (tic) {
 		trace_xfs_log_umount_write(log, tic);
@@ -3230,9 +3244,6 @@  xfs_log_force(
 		 * previous iclog and go to sleep.
 		 */
 		iclog = iclog->ic_prev;
-		if (iclog->ic_state == XLOG_STATE_ACTIVE ||
-		    iclog->ic_state == XLOG_STATE_DIRTY)
-			goto out_unlock;
 	} else if (iclog->ic_state == XLOG_STATE_ACTIVE) {
 		if (atomic_read(&iclog->ic_refcnt) == 0) {
 			/*
@@ -3248,8 +3259,7 @@  xfs_log_force(
 			if (xlog_state_release_iclog(log, iclog))
 				goto out_error;
 
-			if (be64_to_cpu(iclog->ic_header.h_lsn) != lsn ||
-			    iclog->ic_state == XLOG_STATE_DIRTY)
+			if (be64_to_cpu(iclog->ic_header.h_lsn) != lsn)
 				goto out_unlock;
 		} else {
 			/*
@@ -3269,17 +3279,8 @@  xfs_log_force(
 		;
 	}
 
-	if (!(flags & XFS_LOG_SYNC))
-		goto out_unlock;
-
-	if (iclog->ic_state == XLOG_STATE_IOERROR)
-		goto out_error;
-	XFS_STATS_INC(mp, xs_log_force_sleep);
-	xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
-	if (iclog->ic_state == XLOG_STATE_IOERROR)
-		return -EIO;
-	return 0;
-
+	if (flags & XFS_LOG_SYNC)
+		return xlog_wait_on_iclog(iclog);
 out_unlock:
 	spin_unlock(&log->l_icloglock);
 	return 0;
@@ -3310,9 +3311,6 @@  __xfs_log_force_lsn(
 			goto out_unlock;
 	}
 
-	if (iclog->ic_state == XLOG_STATE_DIRTY)
-		goto out_unlock;
-
 	if (iclog->ic_state == XLOG_STATE_ACTIVE) {
 		/*
 		 * We sleep here if we haven't already slept (e.g. this is the
@@ -3346,20 +3344,8 @@  __xfs_log_force_lsn(
 			*log_flushed = 1;
 	}
 
-	if (!(flags & XFS_LOG_SYNC) ||
-	    (iclog->ic_state == XLOG_STATE_ACTIVE ||
-	     iclog->ic_state == XLOG_STATE_DIRTY))
-		goto out_unlock;
-
-	if (iclog->ic_state == XLOG_STATE_IOERROR)
-		goto out_error;
-
-	XFS_STATS_INC(mp, xs_log_force_sleep);
-	xlog_wait(&iclog->ic_force_wait, &log->l_icloglock);
-	if (iclog->ic_state == XLOG_STATE_IOERROR)
-		return -EIO;
-	return 0;
-
+	if (flags & XFS_LOG_SYNC)
+		return xlog_wait_on_iclog(iclog);
 out_unlock:
 	spin_unlock(&log->l_icloglock);
 	return 0;