[v2,06/15] xfs: reset dfops to initial state after finish
diff mbox series

Message ID 20180723130414.47980-7-bfoster@redhat.com
State Accepted
Headers show
Series
  • xfs: embed dfops in the transaction
Related show

Commit Message

Brian Foster July 23, 2018, 1:04 p.m. UTC
xfs_defer_init() is currently used in two particular situations. The
first and most obvious case is raw initialization of an
xfs_defer_ops struct. The other case is partial reinit of
xfs_defer_ops on reuse due to iteration.

Most instances of the first case will be replaced by a single init
of a dfops embedded in the transaction. Init calls are still
technically required for the second case because the dfops may have
low space mode enabled or have joined items that need to be reset
before the dfops should be reused.

Since the current dfops usage expects either a final transaction
commit after xfs_defer_finish() or xfs_defer_init() if dfops is to
be reused, we can shift some of the init logic into
xfs_defer_finish() such that the latter returns with a reinitialized
dfops. This eliminates the second dependency noted above such that a
dfops is immediately ready for reuse after an xfs_defer_finish()
without the need to change any calling code.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_defer.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Comments

Bill O'Donnell July 24, 2018, 12:53 p.m. UTC | #1
On Mon, Jul 23, 2018 at 09:04:05AM -0400, Brian Foster wrote:
> xfs_defer_init() is currently used in two particular situations. The
> first and most obvious case is raw initialization of an
> xfs_defer_ops struct. The other case is partial reinit of
> xfs_defer_ops on reuse due to iteration.
> 
> Most instances of the first case will be replaced by a single init
> of a dfops embedded in the transaction. Init calls are still
> technically required for the second case because the dfops may have
> low space mode enabled or have joined items that need to be reset
> before the dfops should be reused.
> 
> Since the current dfops usage expects either a final transaction
> commit after xfs_defer_finish() or xfs_defer_init() if dfops is to
> be reused, we can shift some of the init logic into
> xfs_defer_finish() such that the latter returns with a reinitialized
> dfops. This eliminates the second dependency noted above such that a
> dfops is immediately ready for reuse after an xfs_defer_finish()
> without the need to change any calling code.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks good.
Reviewed-by: Bill O'Donnell <billodo@redhat.com>

> ---
>  fs/xfs/libxfs/xfs_defer.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 0df09c094e42..23f2a52b088e 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -319,6 +319,19 @@ xfs_defer_bjoin(
>  	return -EFSCORRUPTED;
>  }
>  
> +/*
> + * Reset an already used dfops after finish.
> + */
> +static void
> +xfs_defer_reset(
> +	struct xfs_defer_ops	*dop)
> +{
> +	ASSERT(!xfs_defer_has_unfinished_work(dop));
> +	dop->dop_low = false;
> +	memset(dop->dop_inodes, 0, sizeof(dop->dop_inodes));
> +	memset(dop->dop_bufs, 0, sizeof(dop->dop_bufs));
> +}
> +
>  /*
>   * Finish all the pending work.  This involves logging intent items for
>   * any work items that wandered in since the last transaction roll (if
> @@ -427,10 +440,13 @@ xfs_defer_finish(
>  		dop = (*tp)->t_dfops;
>  	}
>  out:
> -	if (error)
> +	if (error) {
>  		trace_xfs_defer_finish_error((*tp)->t_mountp, dop, error);
> -	else
> +	} else {
>  		trace_xfs_defer_finish_done((*tp)->t_mountp, dop, _RET_IP_);
> +		xfs_defer_reset(dop);
> +	}
> +
>  	return error;
>  }
>  
> -- 
> 2.17.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick J. Wong July 24, 2018, 8:46 p.m. UTC | #2
On Mon, Jul 23, 2018 at 09:04:05AM -0400, Brian Foster wrote:
> xfs_defer_init() is currently used in two particular situations. The
> first and most obvious case is raw initialization of an
> xfs_defer_ops struct. The other case is partial reinit of
> xfs_defer_ops on reuse due to iteration.
> 
> Most instances of the first case will be replaced by a single init
> of a dfops embedded in the transaction. Init calls are still
> technically required for the second case because the dfops may have
> low space mode enabled or have joined items that need to be reset
> before the dfops should be reused.
> 
> Since the current dfops usage expects either a final transaction
> commit after xfs_defer_finish() or xfs_defer_init() if dfops is to
> be reused, we can shift some of the init logic into
> xfs_defer_finish() such that the latter returns with a reinitialized
> dfops. This eliminates the second dependency noted above such that a
> dfops is immediately ready for reuse after an xfs_defer_finish()
> without the need to change any calling code.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_defer.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 0df09c094e42..23f2a52b088e 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -319,6 +319,19 @@ xfs_defer_bjoin(
>  	return -EFSCORRUPTED;
>  }
>  
> +/*
> + * Reset an already used dfops after finish.
> + */
> +static void
> +xfs_defer_reset(
> +	struct xfs_defer_ops	*dop)
> +{
> +	ASSERT(!xfs_defer_has_unfinished_work(dop));
> +	dop->dop_low = false;
> +	memset(dop->dop_inodes, 0, sizeof(dop->dop_inodes));
> +	memset(dop->dop_bufs, 0, sizeof(dop->dop_bufs));
> +}
> +
>  /*
>   * Finish all the pending work.  This involves logging intent items for
>   * any work items that wandered in since the last transaction roll (if
> @@ -427,10 +440,13 @@ xfs_defer_finish(
>  		dop = (*tp)->t_dfops;
>  	}
>  out:
> -	if (error)
> +	if (error) {
>  		trace_xfs_defer_finish_error((*tp)->t_mountp, dop, error);
> -	else
> +	} else {
>  		trace_xfs_defer_finish_done((*tp)->t_mountp, dop, _RET_IP_);
> +		xfs_defer_reset(dop);
> +	}
> +
>  	return error;
>  }
>  
> -- 
> 2.17.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox series

diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 0df09c094e42..23f2a52b088e 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -319,6 +319,19 @@  xfs_defer_bjoin(
 	return -EFSCORRUPTED;
 }
 
+/*
+ * Reset an already used dfops after finish.
+ */
+static void
+xfs_defer_reset(
+	struct xfs_defer_ops	*dop)
+{
+	ASSERT(!xfs_defer_has_unfinished_work(dop));
+	dop->dop_low = false;
+	memset(dop->dop_inodes, 0, sizeof(dop->dop_inodes));
+	memset(dop->dop_bufs, 0, sizeof(dop->dop_bufs));
+}
+
 /*
  * Finish all the pending work.  This involves logging intent items for
  * any work items that wandered in since the last transaction roll (if
@@ -427,10 +440,13 @@  xfs_defer_finish(
 		dop = (*tp)->t_dfops;
 	}
 out:
-	if (error)
+	if (error) {
 		trace_xfs_defer_finish_error((*tp)->t_mountp, dop, error);
-	else
+	} else {
 		trace_xfs_defer_finish_done((*tp)->t_mountp, dop, _RET_IP_);
+		xfs_defer_reset(dop);
+	}
+
 	return error;
 }