diff mbox series

[v2,08/15] xfs: support embedded dfops in transaction

Message ID 20180723130414.47980-9-bfoster@redhat.com (mailing list archive)
State Accepted
Headers show
Series xfs: embed dfops in the transaction | expand

Commit Message

Brian Foster July 23, 2018, 1:04 p.m. UTC
The dfops structure used by multi-transaction operations is
typically stored on the stack and carried around by the associated
transaction. The lifecycle of dfops does not quite match that of the
transaction, but they are tightly related in that the former depends
on the latter.

The relationship of these objects is tight enough that we can avoid
the cumbersome boilerplate code required in most cases to manage
them separately by just embedding an xfs_defer_ops in the
transaction itself. This means that a transaction allocation returns
with an initialized dfops, a transaction commit finishes pending
deferred items before the tx commit, a transaction cancel cancels
the dfops before the transaction and a transaction dup operation
transfers the current dfops state to the new transaction.

The dup operation is slightly complicated by the fact that we can no
longer just copy a dfops pointer from the old transaction to the new
transaction. This is solved through a dfops move helper that
transfers the pending items and other dfops state across the
transactions. This also requires that transaction rolling code
always refer to the transaction for the current dfops reference.

Finally, to facilitate incremental conversion to the internal dfops
and continue to support the current external dfops mode of
operation, create the new ->t_dfops_internal field with a layer of
indirection. On allocation, ->t_dfops points to the internal dfops.
This state is overridden by callers who re-init a local dfops on the
transaction. Once ->t_dfops is overridden, the external dfops
reference is maintained as the transaction rolls.

This patch adds the fundamental ability to support an internal
dfops. All codepaths that perform deferred processing continue to
override the internal dfops until they are converted over in
subsequent patches.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_defer.c | 22 ++++++++++++++++++++++
 fs/xfs/libxfs/xfs_defer.h | 16 ++--------------
 fs/xfs/xfs_trans.c        | 30 ++++++++++++++++++++++++++----
 fs/xfs/xfs_trans.h        | 17 ++++++++++++++++-
 4 files changed, 66 insertions(+), 19 deletions(-)

Comments

Bill O'Donnell July 24, 2018, 12:56 p.m. UTC | #1
On Mon, Jul 23, 2018 at 09:04:07AM -0400, Brian Foster wrote:
> The dfops structure used by multi-transaction operations is
> typically stored on the stack and carried around by the associated
> transaction. The lifecycle of dfops does not quite match that of the
> transaction, but they are tightly related in that the former depends
> on the latter.
> 
> The relationship of these objects is tight enough that we can avoid
> the cumbersome boilerplate code required in most cases to manage
> them separately by just embedding an xfs_defer_ops in the
> transaction itself. This means that a transaction allocation returns
> with an initialized dfops, a transaction commit finishes pending
> deferred items before the tx commit, a transaction cancel cancels
> the dfops before the transaction and a transaction dup operation
> transfers the current dfops state to the new transaction.
> 
> The dup operation is slightly complicated by the fact that we can no
> longer just copy a dfops pointer from the old transaction to the new
> transaction. This is solved through a dfops move helper that
> transfers the pending items and other dfops state across the
> transactions. This also requires that transaction rolling code
> always refer to the transaction for the current dfops reference.
> 
> Finally, to facilitate incremental conversion to the internal dfops
> and continue to support the current external dfops mode of
> operation, create the new ->t_dfops_internal field with a layer of
> indirection. On allocation, ->t_dfops points to the internal dfops.
> This state is overridden by callers who re-init a local dfops on the
> transaction. Once ->t_dfops is overridden, the external dfops
> reference is maintained as the transaction rolls.
> 
> This patch adds the fundamental ability to support an internal
> dfops. All codepaths that perform deferred processing continue to
> override the internal dfops until they are converted over in
> subsequent patches.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

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

> ---
>  fs/xfs/libxfs/xfs_defer.c | 22 ++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_defer.h | 16 ++--------------
>  fs/xfs/xfs_trans.c        | 30 ++++++++++++++++++++++++++----
>  fs/xfs/xfs_trans.h        | 17 ++++++++++++++++-
>  4 files changed, 66 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 23f2a52b088e..b63cc9e730da 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -555,3 +555,25 @@ xfs_defer_init(
>  	}
>  	trace_xfs_defer_init(mp, dop, _RET_IP_);
>  }
> +
> +/*
> + * Move state from one xfs_defer_ops to another and reset the source to initial
> + * state. This is primarily used to carry state forward across transaction rolls
> + * with internal dfops.
> + */
> +void
> +xfs_defer_move(
> +	struct xfs_defer_ops	*dst,
> +	struct xfs_defer_ops	*src)
> +{
> +	ASSERT(dst != src);
> +
> +	list_splice_init(&src->dop_intake, &dst->dop_intake);
> +	list_splice_init(&src->dop_pending, &dst->dop_pending);
> +
> +	memcpy(dst->dop_inodes, src->dop_inodes, sizeof(dst->dop_inodes));
> +	memcpy(dst->dop_bufs, src->dop_bufs, sizeof(dst->dop_bufs));
> +	dst->dop_low = src->dop_low;
> +
> +	xfs_defer_reset(src);
> +}
> diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
> index 8f58f217fdff..35507ca9a148 100644
> --- a/fs/xfs/libxfs/xfs_defer.h
> +++ b/fs/xfs/libxfs/xfs_defer.h
> @@ -7,6 +7,7 @@
>  #define	__XFS_DEFER_H__
>  
>  struct xfs_defer_op_type;
> +struct xfs_defer_ops;
>  
>  /*
>   * Save a log intent item and a list of extents, so that we can replay
> @@ -45,20 +46,6 @@ enum xfs_defer_ops_type {
>  	XFS_DEFER_OPS_TYPE_MAX,
>  };
>  
> -#define XFS_DEFER_OPS_NR_INODES	2	/* join up to two inodes */
> -#define XFS_DEFER_OPS_NR_BUFS	2	/* join up to two buffers */
> -
> -struct xfs_defer_ops {
> -	struct list_head	dop_intake;	/* unlogged pending work */
> -	struct list_head	dop_pending;	/* logged pending work */
> -
> -	/* relog these with each roll */
> -	struct xfs_inode	*dop_inodes[XFS_DEFER_OPS_NR_INODES];
> -	struct xfs_buf		*dop_bufs[XFS_DEFER_OPS_NR_BUFS];
> -
> -	bool			dop_low;	/* alloc in low mode */
> -};
> -
>  void xfs_defer_add(struct xfs_defer_ops *dop, enum xfs_defer_ops_type type,
>  		struct list_head *h);
>  int xfs_defer_finish(struct xfs_trans **tp, struct xfs_defer_ops *dop);
> @@ -67,6 +54,7 @@ void xfs_defer_init(struct xfs_trans *tp, struct xfs_defer_ops *dop);
>  bool xfs_defer_has_unfinished_work(struct xfs_defer_ops *dop);
>  int xfs_defer_ijoin(struct xfs_defer_ops *dop, struct xfs_inode *ip);
>  int xfs_defer_bjoin(struct xfs_defer_ops *dop, struct xfs_buf *bp);
> +void xfs_defer_move(struct xfs_defer_ops *dst, struct xfs_defer_ops *src);
>  
>  /* Description of a deferred type. */
>  struct xfs_defer_op_type {
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index de00f79ff698..412c8d236c71 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -119,7 +119,13 @@ xfs_trans_dup(
>  	ntp->t_rtx_res = tp->t_rtx_res - tp->t_rtx_res_used;
>  	tp->t_rtx_res = tp->t_rtx_res_used;
>  	ntp->t_pflags = tp->t_pflags;
> -	ntp->t_dfops = tp->t_dfops;
> +
> +	/* copy the dfops pointer if it's external, otherwise move it */
> +	xfs_defer_init(ntp, &ntp->t_dfops_internal);
> +	if (tp->t_dfops != &tp->t_dfops_internal)
> +		ntp->t_dfops = tp->t_dfops;
> +	else
> +		xfs_defer_move(ntp->t_dfops, tp->t_dfops);
>  
>  	xfs_trans_dup_dqinfo(tp, ntp);
>  
> @@ -275,6 +281,13 @@ xfs_trans_alloc(
>  	INIT_LIST_HEAD(&tp->t_items);
>  	INIT_LIST_HEAD(&tp->t_busy);
>  	tp->t_firstblock = NULLFSBLOCK;
> +	/*
> +	 * We only roll transactions with permanent log reservation. Don't init
> +	 * ->t_dfops to skip attempts to finish or cancel an empty dfops with a
> +	 * non-permanent res.
> +	 */
> +	if (resp->tr_logflags & XFS_TRANS_PERM_LOG_RES)
> +		xfs_defer_init(tp, &tp->t_dfops_internal);
>  
>  	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
>  	if (error) {
> @@ -916,11 +929,17 @@ __xfs_trans_commit(
>  	int			error = 0;
>  	int			sync = tp->t_flags & XFS_TRANS_SYNC;
>  
> -	ASSERT(!tp->t_dfops ||
> -	       !xfs_defer_has_unfinished_work(tp->t_dfops) || regrant);
> -
>  	trace_xfs_trans_commit(tp, _RET_IP_);
>  
> +	/* finish deferred items on final commit */
> +	if (!regrant && tp->t_dfops) {
> +		error = xfs_defer_finish(&tp, tp->t_dfops);
> +		if (error) {
> +			xfs_defer_cancel(tp->t_dfops);
> +			goto out_unreserve;
> +		}
> +	}
> +
>  	/*
>  	 * If there is nothing to be logged by the transaction,
>  	 * then unlock all of the items associated with the
> @@ -1010,6 +1029,9 @@ xfs_trans_cancel(
>  
>  	trace_xfs_trans_cancel(tp, _RET_IP_);
>  
> +	if (tp->t_dfops)
> +		xfs_defer_cancel(tp->t_dfops);
> +
>  	/*
>  	 * See if the caller is relying on us to shut down the
>  	 * filesystem.  This happens in paths where we detect
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 6f857af61455..dc79e3c1d3e8 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -24,7 +24,6 @@ struct xfs_rui_log_item;
>  struct xfs_btree_cur;
>  struct xfs_cui_log_item;
>  struct xfs_cud_log_item;
> -struct xfs_defer_ops;
>  struct xfs_bui_log_item;
>  struct xfs_bud_log_item;
>  
> @@ -90,6 +89,21 @@ void	xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,
>  #define XFS_ITEM_LOCKED		2
>  #define XFS_ITEM_FLUSHING	3
>  
> +/*
> + * Deferred operations tracking structure.
> + */
> +#define XFS_DEFER_OPS_NR_INODES	2	/* join up to two inodes */
> +#define XFS_DEFER_OPS_NR_BUFS	2	/* join up to two buffers */
> +struct xfs_defer_ops {
> +	struct list_head	dop_intake;	/* unlogged pending work */
> +	struct list_head	dop_pending;	/* logged pending work */
> +
> +	/* relog these with each roll */
> +	struct xfs_inode	*dop_inodes[XFS_DEFER_OPS_NR_INODES];
> +	struct xfs_buf		*dop_bufs[XFS_DEFER_OPS_NR_BUFS];
> +
> +	bool			dop_low;	/* alloc in low mode */
> +};
>  
>  /*
>   * This is the structure maintained for every active transaction.
> @@ -130,6 +144,7 @@ typedef struct xfs_trans {
>  	struct list_head	t_items;	/* log item descriptors */
>  	struct list_head	t_busy;		/* list of busy extents */
>  	unsigned long		t_pflags;	/* saved process flags state */
> +	struct xfs_defer_ops	t_dfops_internal;
>  } xfs_trans_t;
>  
>  /*
> -- 
> 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:51 p.m. UTC | #2
On Mon, Jul 23, 2018 at 09:04:07AM -0400, Brian Foster wrote:
> The dfops structure used by multi-transaction operations is
> typically stored on the stack and carried around by the associated
> transaction. The lifecycle of dfops does not quite match that of the
> transaction, but they are tightly related in that the former depends
> on the latter.
> 
> The relationship of these objects is tight enough that we can avoid
> the cumbersome boilerplate code required in most cases to manage
> them separately by just embedding an xfs_defer_ops in the
> transaction itself. This means that a transaction allocation returns
> with an initialized dfops, a transaction commit finishes pending
> deferred items before the tx commit, a transaction cancel cancels
> the dfops before the transaction and a transaction dup operation
> transfers the current dfops state to the new transaction.
> 
> The dup operation is slightly complicated by the fact that we can no
> longer just copy a dfops pointer from the old transaction to the new
> transaction. This is solved through a dfops move helper that
> transfers the pending items and other dfops state across the
> transactions. This also requires that transaction rolling code
> always refer to the transaction for the current dfops reference.
> 
> Finally, to facilitate incremental conversion to the internal dfops
> and continue to support the current external dfops mode of
> operation, create the new ->t_dfops_internal field with a layer of
> indirection. On allocation, ->t_dfops points to the internal dfops.
> This state is overridden by callers who re-init a local dfops on the
> transaction. Once ->t_dfops is overridden, the external dfops
> reference is maintained as the transaction rolls.
> 
> This patch adds the fundamental ability to support an internal
> dfops. All codepaths that perform deferred processing continue to
> override the internal dfops until they are converted over in
> subsequent patches.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

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

--D

> ---
>  fs/xfs/libxfs/xfs_defer.c | 22 ++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_defer.h | 16 ++--------------
>  fs/xfs/xfs_trans.c        | 30 ++++++++++++++++++++++++++----
>  fs/xfs/xfs_trans.h        | 17 ++++++++++++++++-
>  4 files changed, 66 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 23f2a52b088e..b63cc9e730da 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -555,3 +555,25 @@ xfs_defer_init(
>  	}
>  	trace_xfs_defer_init(mp, dop, _RET_IP_);
>  }
> +
> +/*
> + * Move state from one xfs_defer_ops to another and reset the source to initial
> + * state. This is primarily used to carry state forward across transaction rolls
> + * with internal dfops.
> + */
> +void
> +xfs_defer_move(
> +	struct xfs_defer_ops	*dst,
> +	struct xfs_defer_ops	*src)
> +{
> +	ASSERT(dst != src);
> +
> +	list_splice_init(&src->dop_intake, &dst->dop_intake);
> +	list_splice_init(&src->dop_pending, &dst->dop_pending);
> +
> +	memcpy(dst->dop_inodes, src->dop_inodes, sizeof(dst->dop_inodes));
> +	memcpy(dst->dop_bufs, src->dop_bufs, sizeof(dst->dop_bufs));
> +	dst->dop_low = src->dop_low;
> +
> +	xfs_defer_reset(src);
> +}
> diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
> index 8f58f217fdff..35507ca9a148 100644
> --- a/fs/xfs/libxfs/xfs_defer.h
> +++ b/fs/xfs/libxfs/xfs_defer.h
> @@ -7,6 +7,7 @@
>  #define	__XFS_DEFER_H__
>  
>  struct xfs_defer_op_type;
> +struct xfs_defer_ops;
>  
>  /*
>   * Save a log intent item and a list of extents, so that we can replay
> @@ -45,20 +46,6 @@ enum xfs_defer_ops_type {
>  	XFS_DEFER_OPS_TYPE_MAX,
>  };
>  
> -#define XFS_DEFER_OPS_NR_INODES	2	/* join up to two inodes */
> -#define XFS_DEFER_OPS_NR_BUFS	2	/* join up to two buffers */
> -
> -struct xfs_defer_ops {
> -	struct list_head	dop_intake;	/* unlogged pending work */
> -	struct list_head	dop_pending;	/* logged pending work */
> -
> -	/* relog these with each roll */
> -	struct xfs_inode	*dop_inodes[XFS_DEFER_OPS_NR_INODES];
> -	struct xfs_buf		*dop_bufs[XFS_DEFER_OPS_NR_BUFS];
> -
> -	bool			dop_low;	/* alloc in low mode */
> -};
> -
>  void xfs_defer_add(struct xfs_defer_ops *dop, enum xfs_defer_ops_type type,
>  		struct list_head *h);
>  int xfs_defer_finish(struct xfs_trans **tp, struct xfs_defer_ops *dop);
> @@ -67,6 +54,7 @@ void xfs_defer_init(struct xfs_trans *tp, struct xfs_defer_ops *dop);
>  bool xfs_defer_has_unfinished_work(struct xfs_defer_ops *dop);
>  int xfs_defer_ijoin(struct xfs_defer_ops *dop, struct xfs_inode *ip);
>  int xfs_defer_bjoin(struct xfs_defer_ops *dop, struct xfs_buf *bp);
> +void xfs_defer_move(struct xfs_defer_ops *dst, struct xfs_defer_ops *src);
>  
>  /* Description of a deferred type. */
>  struct xfs_defer_op_type {
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index de00f79ff698..412c8d236c71 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -119,7 +119,13 @@ xfs_trans_dup(
>  	ntp->t_rtx_res = tp->t_rtx_res - tp->t_rtx_res_used;
>  	tp->t_rtx_res = tp->t_rtx_res_used;
>  	ntp->t_pflags = tp->t_pflags;
> -	ntp->t_dfops = tp->t_dfops;
> +
> +	/* copy the dfops pointer if it's external, otherwise move it */
> +	xfs_defer_init(ntp, &ntp->t_dfops_internal);
> +	if (tp->t_dfops != &tp->t_dfops_internal)
> +		ntp->t_dfops = tp->t_dfops;
> +	else
> +		xfs_defer_move(ntp->t_dfops, tp->t_dfops);
>  
>  	xfs_trans_dup_dqinfo(tp, ntp);
>  
> @@ -275,6 +281,13 @@ xfs_trans_alloc(
>  	INIT_LIST_HEAD(&tp->t_items);
>  	INIT_LIST_HEAD(&tp->t_busy);
>  	tp->t_firstblock = NULLFSBLOCK;
> +	/*
> +	 * We only roll transactions with permanent log reservation. Don't init
> +	 * ->t_dfops to skip attempts to finish or cancel an empty dfops with a
> +	 * non-permanent res.
> +	 */
> +	if (resp->tr_logflags & XFS_TRANS_PERM_LOG_RES)
> +		xfs_defer_init(tp, &tp->t_dfops_internal);
>  
>  	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
>  	if (error) {
> @@ -916,11 +929,17 @@ __xfs_trans_commit(
>  	int			error = 0;
>  	int			sync = tp->t_flags & XFS_TRANS_SYNC;
>  
> -	ASSERT(!tp->t_dfops ||
> -	       !xfs_defer_has_unfinished_work(tp->t_dfops) || regrant);
> -
>  	trace_xfs_trans_commit(tp, _RET_IP_);
>  
> +	/* finish deferred items on final commit */
> +	if (!regrant && tp->t_dfops) {
> +		error = xfs_defer_finish(&tp, tp->t_dfops);
> +		if (error) {
> +			xfs_defer_cancel(tp->t_dfops);
> +			goto out_unreserve;
> +		}
> +	}
> +
>  	/*
>  	 * If there is nothing to be logged by the transaction,
>  	 * then unlock all of the items associated with the
> @@ -1010,6 +1029,9 @@ xfs_trans_cancel(
>  
>  	trace_xfs_trans_cancel(tp, _RET_IP_);
>  
> +	if (tp->t_dfops)
> +		xfs_defer_cancel(tp->t_dfops);
> +
>  	/*
>  	 * See if the caller is relying on us to shut down the
>  	 * filesystem.  This happens in paths where we detect
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 6f857af61455..dc79e3c1d3e8 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -24,7 +24,6 @@ struct xfs_rui_log_item;
>  struct xfs_btree_cur;
>  struct xfs_cui_log_item;
>  struct xfs_cud_log_item;
> -struct xfs_defer_ops;
>  struct xfs_bui_log_item;
>  struct xfs_bud_log_item;
>  
> @@ -90,6 +89,21 @@ void	xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,
>  #define XFS_ITEM_LOCKED		2
>  #define XFS_ITEM_FLUSHING	3
>  
> +/*
> + * Deferred operations tracking structure.
> + */
> +#define XFS_DEFER_OPS_NR_INODES	2	/* join up to two inodes */
> +#define XFS_DEFER_OPS_NR_BUFS	2	/* join up to two buffers */
> +struct xfs_defer_ops {
> +	struct list_head	dop_intake;	/* unlogged pending work */
> +	struct list_head	dop_pending;	/* logged pending work */
> +
> +	/* relog these with each roll */
> +	struct xfs_inode	*dop_inodes[XFS_DEFER_OPS_NR_INODES];
> +	struct xfs_buf		*dop_bufs[XFS_DEFER_OPS_NR_BUFS];
> +
> +	bool			dop_low;	/* alloc in low mode */
> +};
>  
>  /*
>   * This is the structure maintained for every active transaction.
> @@ -130,6 +144,7 @@ typedef struct xfs_trans {
>  	struct list_head	t_items;	/* log item descriptors */
>  	struct list_head	t_busy;		/* list of busy extents */
>  	unsigned long		t_pflags;	/* saved process flags state */
> +	struct xfs_defer_ops	t_dfops_internal;
>  } xfs_trans_t;
>  
>  /*
> -- 
> 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
Christoph Hellwig July 25, 2018, 5:04 a.m. UTC | #3
On Mon, Jul 23, 2018 at 09:04:07AM -0400, Brian Foster wrote:
> The dfops structure used by multi-transaction operations is
> typically stored on the stack and carried around by the associated
> transaction. The lifecycle of dfops does not quite match that of the
> transaction, but they are tightly related in that the former depends
> on the latter.
> 
> The relationship of these objects is tight enough that we can avoid
> the cumbersome boilerplate code required in most cases to manage
> them separately by just embedding an xfs_defer_ops in the
> transaction itself. This means that a transaction allocation returns
> with an initialized dfops, a transaction commit finishes pending
> deferred items before the tx commit, a transaction cancel cancels
> the dfops before the transaction and a transaction dup operation
> transfers the current dfops state to the new transaction.
> 
> The dup operation is slightly complicated by the fact that we can no
> longer just copy a dfops pointer from the old transaction to the new
> transaction. This is solved through a dfops move helper that
> transfers the pending items and other dfops state across the
> transactions. This also requires that transaction rolling code
> always refer to the transaction for the current dfops reference.
> 
> Finally, to facilitate incremental conversion to the internal dfops
> and continue to support the current external dfops mode of
> operation, create the new ->t_dfops_internal field with a layer of
> indirection. On allocation, ->t_dfops points to the internal dfops.
> This state is overridden by callers who re-init a local dfops on the
> transaction. Once ->t_dfops is overridden, the external dfops
> reference is maintained as the transaction rolls.
> 
> This patch adds the fundamental ability to support an internal
> dfops. All codepaths that perform deferred processing continue to
> override the internal dfops until they are converted over in
> subsequent patches.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
--
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
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 23f2a52b088e..b63cc9e730da 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -555,3 +555,25 @@  xfs_defer_init(
 	}
 	trace_xfs_defer_init(mp, dop, _RET_IP_);
 }
+
+/*
+ * Move state from one xfs_defer_ops to another and reset the source to initial
+ * state. This is primarily used to carry state forward across transaction rolls
+ * with internal dfops.
+ */
+void
+xfs_defer_move(
+	struct xfs_defer_ops	*dst,
+	struct xfs_defer_ops	*src)
+{
+	ASSERT(dst != src);
+
+	list_splice_init(&src->dop_intake, &dst->dop_intake);
+	list_splice_init(&src->dop_pending, &dst->dop_pending);
+
+	memcpy(dst->dop_inodes, src->dop_inodes, sizeof(dst->dop_inodes));
+	memcpy(dst->dop_bufs, src->dop_bufs, sizeof(dst->dop_bufs));
+	dst->dop_low = src->dop_low;
+
+	xfs_defer_reset(src);
+}
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index 8f58f217fdff..35507ca9a148 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -7,6 +7,7 @@ 
 #define	__XFS_DEFER_H__
 
 struct xfs_defer_op_type;
+struct xfs_defer_ops;
 
 /*
  * Save a log intent item and a list of extents, so that we can replay
@@ -45,20 +46,6 @@  enum xfs_defer_ops_type {
 	XFS_DEFER_OPS_TYPE_MAX,
 };
 
-#define XFS_DEFER_OPS_NR_INODES	2	/* join up to two inodes */
-#define XFS_DEFER_OPS_NR_BUFS	2	/* join up to two buffers */
-
-struct xfs_defer_ops {
-	struct list_head	dop_intake;	/* unlogged pending work */
-	struct list_head	dop_pending;	/* logged pending work */
-
-	/* relog these with each roll */
-	struct xfs_inode	*dop_inodes[XFS_DEFER_OPS_NR_INODES];
-	struct xfs_buf		*dop_bufs[XFS_DEFER_OPS_NR_BUFS];
-
-	bool			dop_low;	/* alloc in low mode */
-};
-
 void xfs_defer_add(struct xfs_defer_ops *dop, enum xfs_defer_ops_type type,
 		struct list_head *h);
 int xfs_defer_finish(struct xfs_trans **tp, struct xfs_defer_ops *dop);
@@ -67,6 +54,7 @@  void xfs_defer_init(struct xfs_trans *tp, struct xfs_defer_ops *dop);
 bool xfs_defer_has_unfinished_work(struct xfs_defer_ops *dop);
 int xfs_defer_ijoin(struct xfs_defer_ops *dop, struct xfs_inode *ip);
 int xfs_defer_bjoin(struct xfs_defer_ops *dop, struct xfs_buf *bp);
+void xfs_defer_move(struct xfs_defer_ops *dst, struct xfs_defer_ops *src);
 
 /* Description of a deferred type. */
 struct xfs_defer_op_type {
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index de00f79ff698..412c8d236c71 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -119,7 +119,13 @@  xfs_trans_dup(
 	ntp->t_rtx_res = tp->t_rtx_res - tp->t_rtx_res_used;
 	tp->t_rtx_res = tp->t_rtx_res_used;
 	ntp->t_pflags = tp->t_pflags;
-	ntp->t_dfops = tp->t_dfops;
+
+	/* copy the dfops pointer if it's external, otherwise move it */
+	xfs_defer_init(ntp, &ntp->t_dfops_internal);
+	if (tp->t_dfops != &tp->t_dfops_internal)
+		ntp->t_dfops = tp->t_dfops;
+	else
+		xfs_defer_move(ntp->t_dfops, tp->t_dfops);
 
 	xfs_trans_dup_dqinfo(tp, ntp);
 
@@ -275,6 +281,13 @@  xfs_trans_alloc(
 	INIT_LIST_HEAD(&tp->t_items);
 	INIT_LIST_HEAD(&tp->t_busy);
 	tp->t_firstblock = NULLFSBLOCK;
+	/*
+	 * We only roll transactions with permanent log reservation. Don't init
+	 * ->t_dfops to skip attempts to finish or cancel an empty dfops with a
+	 * non-permanent res.
+	 */
+	if (resp->tr_logflags & XFS_TRANS_PERM_LOG_RES)
+		xfs_defer_init(tp, &tp->t_dfops_internal);
 
 	error = xfs_trans_reserve(tp, resp, blocks, rtextents);
 	if (error) {
@@ -916,11 +929,17 @@  __xfs_trans_commit(
 	int			error = 0;
 	int			sync = tp->t_flags & XFS_TRANS_SYNC;
 
-	ASSERT(!tp->t_dfops ||
-	       !xfs_defer_has_unfinished_work(tp->t_dfops) || regrant);
-
 	trace_xfs_trans_commit(tp, _RET_IP_);
 
+	/* finish deferred items on final commit */
+	if (!regrant && tp->t_dfops) {
+		error = xfs_defer_finish(&tp, tp->t_dfops);
+		if (error) {
+			xfs_defer_cancel(tp->t_dfops);
+			goto out_unreserve;
+		}
+	}
+
 	/*
 	 * If there is nothing to be logged by the transaction,
 	 * then unlock all of the items associated with the
@@ -1010,6 +1029,9 @@  xfs_trans_cancel(
 
 	trace_xfs_trans_cancel(tp, _RET_IP_);
 
+	if (tp->t_dfops)
+		xfs_defer_cancel(tp->t_dfops);
+
 	/*
 	 * See if the caller is relying on us to shut down the
 	 * filesystem.  This happens in paths where we detect
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 6f857af61455..dc79e3c1d3e8 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -24,7 +24,6 @@  struct xfs_rui_log_item;
 struct xfs_btree_cur;
 struct xfs_cui_log_item;
 struct xfs_cud_log_item;
-struct xfs_defer_ops;
 struct xfs_bui_log_item;
 struct xfs_bud_log_item;
 
@@ -90,6 +89,21 @@  void	xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,
 #define XFS_ITEM_LOCKED		2
 #define XFS_ITEM_FLUSHING	3
 
+/*
+ * Deferred operations tracking structure.
+ */
+#define XFS_DEFER_OPS_NR_INODES	2	/* join up to two inodes */
+#define XFS_DEFER_OPS_NR_BUFS	2	/* join up to two buffers */
+struct xfs_defer_ops {
+	struct list_head	dop_intake;	/* unlogged pending work */
+	struct list_head	dop_pending;	/* logged pending work */
+
+	/* relog these with each roll */
+	struct xfs_inode	*dop_inodes[XFS_DEFER_OPS_NR_INODES];
+	struct xfs_buf		*dop_bufs[XFS_DEFER_OPS_NR_BUFS];
+
+	bool			dop_low;	/* alloc in low mode */
+};
 
 /*
  * This is the structure maintained for every active transaction.
@@ -130,6 +144,7 @@  typedef struct xfs_trans {
 	struct list_head	t_items;	/* log item descriptors */
 	struct list_head	t_busy;		/* list of busy extents */
 	unsigned long		t_pflags;	/* saved process flags state */
+	struct xfs_defer_ops	t_dfops_internal;
 } xfs_trans_t;
 
 /*