* explicitly join inodes to deferred operations V2 (this time for real)
@ 2017-08-28 7:44 Christoph Hellwig
2017-08-28 7:44 ` [PATCH 1/3] xfs: refactor xfs_trans_roll Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Christoph Hellwig @ 2017-08-28 7:44 UTC (permalink / raw)
To: linux-xfs
This series cleanup up the rolled transaction and defer code to
not treat inodes special - the low-level roll code now doesn't know
about inodes any more, and xfs_defer_finish loses the inode argument.
Changes since V1:
- rename __xfs_trans_roll to xfs_trans_roll
- rename xfs_defer_join_inode to xfs_defer_ijoin
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] xfs: refactor xfs_trans_roll
2017-08-28 7:44 explicitly join inodes to deferred operations V2 (this time for real) Christoph Hellwig
@ 2017-08-28 7:44 ` Christoph Hellwig
2017-08-28 17:33 ` Darrick J. Wong
2017-08-28 7:44 ` [PATCH 2/3] xfs: rename xfs_defer_join to xfs_defer_ijoin Christoph Hellwig
2017-08-28 7:44 ` [PATCH 3/3] xfs: remove the ip argument to xfs_defer_finish Christoph Hellwig
2 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2017-08-28 7:44 UTC (permalink / raw)
To: linux-xfs
Split xfs_trans_roll into a low-level helper that just rolls the
actual transaction and a new higher level xfs_trans_roll_inode
that takes care of logging and rejoining the inode. This gets
rid of the NULL inode case, and allows to simplify the special
cases in the deferred operation code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_attr.c | 16 ++++++++--------
fs/xfs/libxfs/xfs_attr_leaf.c | 6 +++---
fs/xfs/libxfs/xfs_attr_remote.c | 4 ++--
fs/xfs/libxfs/xfs_defer.c | 23 +++++++++--------------
fs/xfs/xfs_attr_inactive.c | 6 +++---
fs/xfs/xfs_inode.c | 4 ++--
fs/xfs/xfs_trans.c | 28 +++++-----------------------
fs/xfs/xfs_trans.h | 3 ++-
fs/xfs/xfs_trans_inode.c | 14 ++++++++++++++
9 files changed, 48 insertions(+), 56 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index de7b9bd30bec..bafa0f6bfafa 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -341,7 +341,7 @@ xfs_attr_set(
* transaction to add the new attribute to the leaf.
*/
- error = xfs_trans_roll(&args.trans, dp);
+ error = xfs_trans_roll_inode(&args.trans, dp);
if (error)
goto out;
@@ -605,7 +605,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
* Commit the current trans (including the inode) and start
* a new one.
*/
- error = xfs_trans_roll(&args->trans, dp);
+ error = xfs_trans_roll_inode(&args->trans, dp);
if (error)
return error;
@@ -620,7 +620,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
* Commit the transaction that added the attr name so that
* later routines can manage their own transactions.
*/
- error = xfs_trans_roll(&args->trans, dp);
+ error = xfs_trans_roll_inode(&args->trans, dp);
if (error)
return error;
@@ -697,7 +697,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
/*
* Commit the remove and start the next trans in series.
*/
- error = xfs_trans_roll(&args->trans, dp);
+ error = xfs_trans_roll_inode(&args->trans, dp);
} else if (args->rmtblkno > 0) {
/*
@@ -885,7 +885,7 @@ xfs_attr_node_addname(xfs_da_args_t *args)
* Commit the node conversion and start the next
* trans in the chain.
*/
- error = xfs_trans_roll(&args->trans, dp);
+ error = xfs_trans_roll_inode(&args->trans, dp);
if (error)
goto out;
@@ -925,7 +925,7 @@ xfs_attr_node_addname(xfs_da_args_t *args)
* Commit the leaf addition or btree split and start the next
* trans in the chain.
*/
- error = xfs_trans_roll(&args->trans, dp);
+ error = xfs_trans_roll_inode(&args->trans, dp);
if (error)
goto out;
@@ -1012,7 +1012,7 @@ xfs_attr_node_addname(xfs_da_args_t *args)
/*
* Commit and start the next trans in the chain.
*/
- error = xfs_trans_roll(&args->trans, dp);
+ error = xfs_trans_roll_inode(&args->trans, dp);
if (error)
goto out;
@@ -1132,7 +1132,7 @@ xfs_attr_node_removename(xfs_da_args_t *args)
/*
* Commit the Btree join operation and start a new trans.
*/
- error = xfs_trans_roll(&args->trans, dp);
+ error = xfs_trans_roll_inode(&args->trans, dp);
if (error)
goto out;
}
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index c6c15e5717e4..5c16db86b38f 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -2608,7 +2608,7 @@ xfs_attr3_leaf_clearflag(
/*
* Commit the flag value change and start the next trans in series.
*/
- return xfs_trans_roll(&args->trans, args->dp);
+ return xfs_trans_roll_inode(&args->trans, args->dp);
}
/*
@@ -2659,7 +2659,7 @@ xfs_attr3_leaf_setflag(
/*
* Commit the flag value change and start the next trans in series.
*/
- return xfs_trans_roll(&args->trans, args->dp);
+ return xfs_trans_roll_inode(&args->trans, args->dp);
}
/*
@@ -2777,7 +2777,7 @@ xfs_attr3_leaf_flipflags(
/*
* Commit the flag value change and start the next trans in series.
*/
- error = xfs_trans_roll(&args->trans, args->dp);
+ error = xfs_trans_roll_inode(&args->trans, args->dp);
return error;
}
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 5236d8e45146..433c36714e40 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -484,7 +484,7 @@ xfs_attr_rmtval_set(
/*
* Start the next trans in the chain.
*/
- error = xfs_trans_roll(&args->trans, dp);
+ error = xfs_trans_roll_inode(&args->trans, dp);
if (error)
return error;
}
@@ -621,7 +621,7 @@ xfs_attr_rmtval_remove(
/*
* Close out trans and start the next one in the chain.
*/
- error = xfs_trans_roll(&args->trans, args->dp);
+ error = xfs_trans_roll_inode(&args->trans, args->dp);
if (error)
return error;
}
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 5c2929f94bd3..4ea2f068d95c 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -240,23 +240,19 @@ xfs_defer_trans_abort(
STATIC int
xfs_defer_trans_roll(
struct xfs_trans **tp,
- struct xfs_defer_ops *dop,
- struct xfs_inode *ip)
+ struct xfs_defer_ops *dop)
{
int i;
int error;
- /* Log all the joined inodes except the one we passed in. */
- for (i = 0; i < XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++) {
- if (dop->dop_inodes[i] == ip)
- continue;
+ /* Log all the joined inodes. */
+ for (i = 0; i < XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++)
xfs_trans_log_inode(*tp, dop->dop_inodes[i], XFS_ILOG_CORE);
- }
trace_xfs_defer_trans_roll((*tp)->t_mountp, dop);
/* Roll the transaction. */
- error = xfs_trans_roll(tp, ip);
+ error = xfs_trans_roll(tp);
if (error) {
trace_xfs_defer_trans_roll_error((*tp)->t_mountp, dop, error);
xfs_defer_trans_abort(*tp, dop, error);
@@ -264,12 +260,9 @@ xfs_defer_trans_roll(
}
dop->dop_committed = true;
- /* Rejoin the joined inodes except the one we passed in. */
- for (i = 0; i < XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++) {
- if (dop->dop_inodes[i] == ip)
- continue;
+ /* Rejoin the joined inodes. */
+ for (i = 0; i < XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++)
xfs_trans_ijoin(*tp, dop->dop_inodes[i], 0);
- }
return error;
}
@@ -331,13 +324,15 @@ xfs_defer_finish(
trace_xfs_defer_finish((*tp)->t_mountp, dop);
+ xfs_defer_join(dop, ip);
+
/* Until we run out of pending work to finish... */
while (xfs_defer_has_unfinished_work(dop)) {
/* Log intents for work items sitting in the intake. */
xfs_defer_intake_work(*tp, dop);
/* Roll the transaction. */
- error = xfs_defer_trans_roll(tp, dop, ip);
+ error = xfs_defer_trans_roll(tp, dop);
if (error)
goto out;
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
index be0b79d8900f..ebd66b19fbfc 100644
--- a/fs/xfs/xfs_attr_inactive.c
+++ b/fs/xfs/xfs_attr_inactive.c
@@ -97,7 +97,7 @@ xfs_attr3_leaf_freextent(
/*
* Roll to next transaction.
*/
- error = xfs_trans_roll(trans, dp);
+ error = xfs_trans_roll_inode(trans, dp);
if (error)
return error;
}
@@ -308,7 +308,7 @@ xfs_attr3_node_inactive(
/*
* Atomically commit the whole invalidate stuff.
*/
- error = xfs_trans_roll(trans, dp);
+ error = xfs_trans_roll_inode(trans, dp);
if (error)
return error;
}
@@ -375,7 +375,7 @@ xfs_attr3_root_inactive(
/*
* Commit the invalidate and start the next transaction.
*/
- error = xfs_trans_roll(trans, dp);
+ error = xfs_trans_roll_inode(trans, dp);
return error;
}
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index ff48f0096810..5b4eda44f39f 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1055,7 +1055,7 @@ xfs_dir_ialloc(
tp->t_flags &= ~(XFS_TRANS_DQ_DIRTY);
}
- code = xfs_trans_roll(&tp, NULL);
+ code = xfs_trans_roll(&tp);
if (committed != NULL)
*committed = 1;
@@ -1611,7 +1611,7 @@ xfs_itruncate_extents(
if (error)
goto out_bmap_cancel;
- error = xfs_trans_roll(&tp, ip);
+ error = xfs_trans_roll_inode(&tp, ip);
if (error)
goto out;
}
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 2011620008de..a87f657f59c9 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -1035,25 +1035,18 @@ xfs_trans_cancel(
*/
int
xfs_trans_roll(
- struct xfs_trans **tpp,
- struct xfs_inode *dp)
+ struct xfs_trans **tpp)
{
- struct xfs_trans *trans;
+ struct xfs_trans *trans = *tpp;
struct xfs_trans_res tres;
int error;
/*
- * Ensure that the inode is always logged.
- */
- trans = *tpp;
- if (dp)
- xfs_trans_log_inode(trans, dp, XFS_ILOG_CORE);
-
- /*
* Copy the critical parameters from one trans to the next.
*/
tres.tr_logres = trans->t_log_res;
tres.tr_logcount = trans->t_log_count;
+
*tpp = xfs_trans_dup(trans);
/*
@@ -1067,10 +1060,8 @@ xfs_trans_roll(
if (error)
return error;
- trans = *tpp;
-
/*
- * Reserve space in the log for th next transaction.
+ * Reserve space in the log for the next transaction.
* This also pushes items in the "AIL", the list of logged items,
* out to disk if they are taking up space at the tail of the log
* that we want to use. This requires that either nothing be locked
@@ -1078,14 +1069,5 @@ xfs_trans_roll(
* the prior and the next transactions.
*/
tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
- error = xfs_trans_reserve(trans, &tres, 0, 0);
- /*
- * Ensure that the inode is in the new transaction and locked.
- */
- if (error)
- return error;
-
- if (dp)
- xfs_trans_ijoin(trans, dp, 0);
- return 0;
+ return xfs_trans_reserve(*tpp, &tres, 0, 0);
}
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 7d627721e4b3..b25d3d22e289 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -228,7 +228,8 @@ int xfs_trans_free_extent(struct xfs_trans *,
struct xfs_efd_log_item *, xfs_fsblock_t,
xfs_extlen_t, struct xfs_owner_info *);
int xfs_trans_commit(struct xfs_trans *);
-int xfs_trans_roll(struct xfs_trans **, struct xfs_inode *);
+int xfs_trans_roll(struct xfs_trans **);
+int xfs_trans_roll_inode(struct xfs_trans **, struct xfs_inode *);
void xfs_trans_cancel(xfs_trans_t *);
int xfs_trans_ail_init(struct xfs_mount *);
void xfs_trans_ail_destroy(struct xfs_mount *);
diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c
index dab8daa676f9..daa7615497f9 100644
--- a/fs/xfs/xfs_trans_inode.c
+++ b/fs/xfs/xfs_trans_inode.c
@@ -134,3 +134,17 @@ xfs_trans_log_inode(
flags |= ip->i_itemp->ili_last_fields;
ip->i_itemp->ili_fields |= flags;
}
+
+int
+xfs_trans_roll_inode(
+ struct xfs_trans **tpp,
+ struct xfs_inode *ip)
+{
+ int error;
+
+ xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
+ error = xfs_trans_roll(tpp);
+ if (!error)
+ xfs_trans_ijoin(*tpp, ip, 0);
+ return error;
+}
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] xfs: rename xfs_defer_join to xfs_defer_ijoin
2017-08-28 7:44 explicitly join inodes to deferred operations V2 (this time for real) Christoph Hellwig
2017-08-28 7:44 ` [PATCH 1/3] xfs: refactor xfs_trans_roll Christoph Hellwig
@ 2017-08-28 7:44 ` Christoph Hellwig
2017-08-28 7:44 ` [PATCH 3/3] xfs: remove the ip argument to xfs_defer_finish Christoph Hellwig
2 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2017-08-28 7:44 UTC (permalink / raw)
To: linux-xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_bmap.c | 2 +-
fs/xfs/libxfs/xfs_defer.c | 4 ++--
fs/xfs/libxfs/xfs_defer.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index c09c16b1ad3b..dcefadd4fc3a 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -6452,7 +6452,7 @@ __xfs_bmap_add(
bi->bi_whichfork = whichfork;
bi->bi_bmap = *bmap;
- error = xfs_defer_join(dfops, bi->bi_owner);
+ error = xfs_defer_ijoin(dfops, bi->bi_owner);
if (error) {
kmem_free(bi);
return error;
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 4ea2f068d95c..6c0da24c68c9 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -281,7 +281,7 @@ xfs_defer_has_unfinished_work(
* to xfs_defer_finish().
*/
int
-xfs_defer_join(
+xfs_defer_ijoin(
struct xfs_defer_ops *dop,
struct xfs_inode *ip)
{
@@ -324,7 +324,7 @@ xfs_defer_finish(
trace_xfs_defer_finish((*tp)->t_mountp, dop);
- xfs_defer_join(dop, ip);
+ xfs_defer_ijoin(dop, ip);
/* Until we run out of pending work to finish... */
while (xfs_defer_has_unfinished_work(dop)) {
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index f6e93ef0bffe..70c944b21a2a 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -77,7 +77,7 @@ int xfs_defer_finish(struct xfs_trans **tp, struct xfs_defer_ops *dop,
void xfs_defer_cancel(struct xfs_defer_ops *dop);
void xfs_defer_init(struct xfs_defer_ops *dop, xfs_fsblock_t *fbp);
bool xfs_defer_has_unfinished_work(struct xfs_defer_ops *dop);
-int xfs_defer_join(struct xfs_defer_ops *dop, struct xfs_inode *ip);
+int xfs_defer_ijoin(struct xfs_defer_ops *dop, struct xfs_inode *ip);
/* Description of a deferred type. */
struct xfs_defer_op_type {
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] xfs: remove the ip argument to xfs_defer_finish
2017-08-28 7:44 explicitly join inodes to deferred operations V2 (this time for real) Christoph Hellwig
2017-08-28 7:44 ` [PATCH 1/3] xfs: refactor xfs_trans_roll Christoph Hellwig
2017-08-28 7:44 ` [PATCH 2/3] xfs: rename xfs_defer_join to xfs_defer_ijoin Christoph Hellwig
@ 2017-08-28 7:44 ` Christoph Hellwig
2 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2017-08-28 7:44 UTC (permalink / raw)
To: linux-xfs
And instead require callers to explicitly join the inode using
xfs_defer_ijoin. Also consolidate the defer error handling in
a few places using a goto label.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_attr.c | 140 +++++++++++++++++++++-------------------
fs/xfs/libxfs/xfs_attr_remote.c | 35 +++++-----
fs/xfs/libxfs/xfs_bmap.c | 4 +-
fs/xfs/libxfs/xfs_defer.c | 8 +--
fs/xfs/libxfs/xfs_defer.h | 3 +-
fs/xfs/libxfs/xfs_refcount.c | 2 +-
fs/xfs/xfs_bmap_item.c | 2 +-
fs/xfs/xfs_bmap_util.c | 10 +--
fs/xfs/xfs_dquot.c | 2 +-
fs/xfs/xfs_inode.c | 13 ++--
fs/xfs/xfs_iomap.c | 6 +-
fs/xfs/xfs_refcount_item.c | 2 +-
fs/xfs/xfs_reflink.c | 11 ++--
fs/xfs/xfs_rtalloc.c | 2 +-
fs/xfs/xfs_symlink.c | 5 +-
15 files changed, 129 insertions(+), 116 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index bafa0f6bfafa..6249c92671de 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -328,13 +328,12 @@ xfs_attr_set(
*/
xfs_defer_init(args.dfops, args.firstblock);
error = xfs_attr_shortform_to_leaf(&args);
- if (!error)
- error = xfs_defer_finish(&args.trans, args.dfops, dp);
- if (error) {
- args.trans = NULL;
- xfs_defer_cancel(&dfops);
- goto out;
- }
+ if (error)
+ goto out_defer_cancel;
+ xfs_defer_ijoin(args.dfops, dp);
+ error = xfs_defer_finish(&args.trans, args.dfops);
+ if (error)
+ goto out_defer_cancel;
/*
* Commit the leaf transformation. We'll need another (linked)
@@ -373,6 +372,9 @@ xfs_attr_set(
return error;
+out_defer_cancel:
+ xfs_defer_cancel(&dfops);
+ args.trans = NULL;
out:
if (args.trans)
xfs_trans_cancel(args.trans);
@@ -593,13 +595,12 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
*/
xfs_defer_init(args->dfops, args->firstblock);
error = xfs_attr3_leaf_to_node(args);
- if (!error)
- error = xfs_defer_finish(&args->trans, args->dfops, dp);
- if (error) {
- args->trans = NULL;
- xfs_defer_cancel(args->dfops);
- return error;
- }
+ if (error)
+ goto out_defer_cancel;
+ xfs_defer_ijoin(args->dfops, dp);
+ error = xfs_defer_finish(&args->trans, args->dfops);
+ if (error)
+ goto out_defer_cancel;
/*
* Commit the current trans (including the inode) and start
@@ -684,14 +685,12 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
xfs_defer_init(args->dfops, args->firstblock);
error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
/* bp is gone due to xfs_da_shrink_inode */
- if (!error)
- error = xfs_defer_finish(&args->trans,
- args->dfops, dp);
- if (error) {
- args->trans = NULL;
- xfs_defer_cancel(args->dfops);
- return error;
- }
+ if (error)
+ goto out_defer_cancel;
+ xfs_defer_ijoin(args->dfops, dp);
+ error = xfs_defer_finish(&args->trans, args->dfops);
+ if (error)
+ goto out_defer_cancel;
}
/*
@@ -706,6 +705,10 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
error = xfs_attr3_leaf_clearflag(args);
}
return error;
+out_defer_cancel:
+ xfs_defer_cancel(args->dfops);
+ args->trans = NULL;
+ return error;
}
/*
@@ -747,15 +750,18 @@ xfs_attr_leaf_removename(xfs_da_args_t *args)
xfs_defer_init(args->dfops, args->firstblock);
error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
/* bp is gone due to xfs_da_shrink_inode */
- if (!error)
- error = xfs_defer_finish(&args->trans, args->dfops, dp);
- if (error) {
- args->trans = NULL;
- xfs_defer_cancel(args->dfops);
- return error;
- }
+ if (error)
+ goto out_defer_cancel;
+ xfs_defer_ijoin(args->dfops, dp);
+ error = xfs_defer_finish(&args->trans, args->dfops);
+ if (error)
+ goto out_defer_cancel;
}
return 0;
+out_defer_cancel:
+ xfs_defer_cancel(args->dfops);
+ args->trans = NULL;
+ return error;
}
/*
@@ -872,14 +878,12 @@ xfs_attr_node_addname(xfs_da_args_t *args)
state = NULL;
xfs_defer_init(args->dfops, args->firstblock);
error = xfs_attr3_leaf_to_node(args);
- if (!error)
- error = xfs_defer_finish(&args->trans,
- args->dfops, dp);
- if (error) {
- args->trans = NULL;
- xfs_defer_cancel(args->dfops);
- goto out;
- }
+ if (error)
+ goto out_defer_cancel;
+ xfs_defer_ijoin(args->dfops, dp);
+ error = xfs_defer_finish(&args->trans, args->dfops);
+ if (error)
+ goto out_defer_cancel;
/*
* Commit the node conversion and start the next
@@ -900,13 +904,12 @@ xfs_attr_node_addname(xfs_da_args_t *args)
*/
xfs_defer_init(args->dfops, args->firstblock);
error = xfs_da3_split(state);
- if (!error)
- error = xfs_defer_finish(&args->trans, args->dfops, dp);
- if (error) {
- args->trans = NULL;
- xfs_defer_cancel(args->dfops);
- goto out;
- }
+ if (error)
+ goto out_defer_cancel;
+ xfs_defer_ijoin(args->dfops, dp);
+ error = xfs_defer_finish(&args->trans, args->dfops);
+ if (error)
+ goto out_defer_cancel;
} else {
/*
* Addition succeeded, update Btree hashvals.
@@ -999,14 +1002,12 @@ xfs_attr_node_addname(xfs_da_args_t *args)
if (retval && (state->path.active > 1)) {
xfs_defer_init(args->dfops, args->firstblock);
error = xfs_da3_join(state);
- if (!error)
- error = xfs_defer_finish(&args->trans,
- args->dfops, dp);
- if (error) {
- args->trans = NULL;
- xfs_defer_cancel(args->dfops);
- goto out;
- }
+ if (error)
+ goto out_defer_cancel;
+ xfs_defer_ijoin(args->dfops, dp);
+ error = xfs_defer_finish(&args->trans, args->dfops);
+ if (error)
+ goto out_defer_cancel;
}
/*
@@ -1032,6 +1033,10 @@ xfs_attr_node_addname(xfs_da_args_t *args)
if (error)
return error;
return retval;
+out_defer_cancel:
+ xfs_defer_cancel(args->dfops);
+ args->trans = NULL;
+ goto out;
}
/*
@@ -1122,13 +1127,12 @@ xfs_attr_node_removename(xfs_da_args_t *args)
if (retval && (state->path.active > 1)) {
xfs_defer_init(args->dfops, args->firstblock);
error = xfs_da3_join(state);
- if (!error)
- error = xfs_defer_finish(&args->trans, args->dfops, dp);
- if (error) {
- args->trans = NULL;
- xfs_defer_cancel(args->dfops);
- goto out;
- }
+ if (error)
+ goto out_defer_cancel;
+ xfs_defer_ijoin(args->dfops, dp);
+ error = xfs_defer_finish(&args->trans, args->dfops);
+ if (error)
+ goto out_defer_cancel;
/*
* Commit the Btree join operation and start a new trans.
*/
@@ -1156,14 +1160,12 @@ xfs_attr_node_removename(xfs_da_args_t *args)
xfs_defer_init(args->dfops, args->firstblock);
error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
/* bp is gone due to xfs_da_shrink_inode */
- if (!error)
- error = xfs_defer_finish(&args->trans,
- args->dfops, dp);
- if (error) {
- args->trans = NULL;
- xfs_defer_cancel(args->dfops);
- goto out;
- }
+ if (error)
+ goto out_defer_cancel;
+ xfs_defer_ijoin(args->dfops, dp);
+ error = xfs_defer_finish(&args->trans, args->dfops);
+ if (error)
+ goto out_defer_cancel;
} else
xfs_trans_brelse(args->trans, bp);
}
@@ -1172,6 +1174,10 @@ xfs_attr_node_removename(xfs_da_args_t *args)
out:
xfs_da_state_free(state);
return error;
+out_defer_cancel:
+ xfs_defer_cancel(args->dfops);
+ args->trans = NULL;
+ goto out;
}
/*
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index 433c36714e40..d56caf037ca0 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -467,13 +467,12 @@ xfs_attr_rmtval_set(
error = xfs_bmapi_write(args->trans, dp, (xfs_fileoff_t)lblkno,
blkcnt, XFS_BMAPI_ATTRFORK, args->firstblock,
args->total, &map, &nmap, args->dfops);
- if (!error)
- error = xfs_defer_finish(&args->trans, args->dfops, dp);
- if (error) {
- args->trans = NULL;
- xfs_defer_cancel(args->dfops);
- return error;
- }
+ if (error)
+ goto out_defer_cancel;
+ xfs_defer_ijoin(args->dfops, dp);
+ error = xfs_defer_finish(&args->trans, args->dfops);
+ if (error)
+ goto out_defer_cancel;
ASSERT(nmap == 1);
ASSERT((map.br_startblock != DELAYSTARTBLOCK) &&
@@ -539,6 +538,10 @@ xfs_attr_rmtval_set(
}
ASSERT(valuelen == 0);
return 0;
+out_defer_cancel:
+ xfs_defer_cancel(args->dfops);
+ args->trans = NULL;
+ return error;
}
/*
@@ -609,14 +612,12 @@ xfs_attr_rmtval_remove(
error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt,
XFS_BMAPI_ATTRFORK, 1, args->firstblock,
args->dfops, &done);
- if (!error)
- error = xfs_defer_finish(&args->trans, args->dfops,
- args->dp);
- if (error) {
- args->trans = NULL;
- xfs_defer_cancel(args->dfops);
- return error;
- }
+ if (error)
+ goto out_defer_cancel;
+ xfs_defer_ijoin(args->dfops, args->dp);
+ error = xfs_defer_finish(&args->trans, args->dfops);
+ if (error)
+ goto out_defer_cancel;
/*
* Close out trans and start the next one in the chain.
@@ -626,4 +627,8 @@ xfs_attr_rmtval_remove(
return error;
}
return 0;
+out_defer_cancel:
+ xfs_defer_cancel(args->dfops);
+ args->trans = NULL;
+ return error;
}
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index dcefadd4fc3a..28ac796abe45 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1196,7 +1196,7 @@ xfs_bmap_add_attrfork(
xfs_log_sb(tp);
}
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto bmap_cancel;
error = xfs_trans_commit(tp);
@@ -6402,7 +6402,7 @@ xfs_bmap_split_extent(
if (error)
goto out;
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out;
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 6c0da24c68c9..072ebfe1d6ae 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -277,8 +277,7 @@ xfs_defer_has_unfinished_work(
/*
* Add this inode to the deferred op. Each joined inode is relogged
- * each time we roll the transaction, in addition to any inode passed
- * to xfs_defer_finish().
+ * each time we roll the transaction.
*/
int
xfs_defer_ijoin(
@@ -310,8 +309,7 @@ xfs_defer_ijoin(
int
xfs_defer_finish(
struct xfs_trans **tp,
- struct xfs_defer_ops *dop,
- struct xfs_inode *ip)
+ struct xfs_defer_ops *dop)
{
struct xfs_defer_pending *dfp;
struct list_head *li;
@@ -324,8 +322,6 @@ xfs_defer_finish(
trace_xfs_defer_finish((*tp)->t_mountp, dop);
- xfs_defer_ijoin(dop, ip);
-
/* Until we run out of pending work to finish... */
while (xfs_defer_has_unfinished_work(dop)) {
/* Log intents for work items sitting in the intake. */
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index 70c944b21a2a..d4f046dd44bd 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -72,8 +72,7 @@ struct xfs_defer_ops {
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,
- struct xfs_inode *ip);
+int xfs_defer_finish(struct xfs_trans **tp, struct xfs_defer_ops *dop);
void xfs_defer_cancel(struct xfs_defer_ops *dop);
void xfs_defer_init(struct xfs_defer_ops *dop, xfs_fsblock_t *fbp);
bool xfs_defer_has_unfinished_work(struct xfs_defer_ops *dop);
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index 45b1c3b4e047..9d5406b4f663 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -1679,7 +1679,7 @@ xfs_refcount_recover_cow_leftovers(
xfs_bmap_add_free(mp, &dfops, fsb,
rr->rr_rrec.rc_blockcount, NULL);
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_defer;
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 88073910fa5d..dd136f7275e4 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -502,7 +502,7 @@ xfs_bui_recover(
}
/* Finish transaction, free inodes. */
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto err_dfops;
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 93e955262d07..f7f04aebcade 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1136,7 +1136,7 @@ xfs_alloc_file_space(
/*
* Complete the transaction
*/
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto error0;
@@ -1202,7 +1202,8 @@ xfs_unmap_extent(
if (error)
goto out_bmap_cancel;
- error = xfs_defer_finish(&tp, &dfops, ip);
+ xfs_defer_ijoin(&dfops, ip);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_bmap_cancel;
@@ -1496,7 +1497,7 @@ xfs_shift_file_space(
if (error)
goto out_bmap_cancel;
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_bmap_cancel;
@@ -1777,7 +1778,8 @@ xfs_swap_extent_rmap(
if (error)
goto out_defer;
- error = xfs_defer_finish(tpp, &dfops, ip);
+ xfs_defer_ijoin(&dfops, ip);
+ error = xfs_defer_finish(tpp, &dfops);
if (error)
goto out_defer;
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index fd2ef8c2c9a7..cd82429d8df7 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -383,7 +383,7 @@ xfs_qm_dqalloc(
xfs_trans_bhold(tp, bp);
- error = xfs_defer_finish(tpp, &dfops, NULL);
+ error = xfs_defer_finish(tpp, &dfops);
if (error)
goto error1;
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 5b4eda44f39f..97644604a21a 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1285,7 +1285,7 @@ xfs_create(
*/
xfs_qm_vop_create_dqattach(tp, ip, udqp, gdqp, pdqp);
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_bmap_cancel;
@@ -1513,7 +1513,7 @@ xfs_link(
if (mp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
xfs_trans_set_sync(tp);
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error) {
xfs_defer_cancel(&dfops);
goto error_return;
@@ -1607,7 +1607,8 @@ xfs_itruncate_extents(
* Duplicate the transaction that has the permanent
* reservation and commit the old transaction.
*/
- error = xfs_defer_finish(&tp, &dfops, ip);
+ xfs_defer_ijoin(&dfops, ip);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_bmap_cancel;
@@ -1855,7 +1856,7 @@ xfs_inactive_ifree(
* Just ignore errors at this point. There is nothing we can do except
* to try to keep going. Make sure it's not a silent error.
*/
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error) {
xfs_notice(mp, "%s: xfs_defer_finish returned error %d",
__func__, error);
@@ -2637,7 +2638,7 @@ xfs_remove(
if (mp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
xfs_trans_set_sync(tp);
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_bmap_cancel;
@@ -2723,7 +2724,7 @@ xfs_finish_rename(
if (tp->t_mountp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
xfs_trans_set_sync(tp);
- error = xfs_defer_finish(&tp, dfops, NULL);
+ error = xfs_defer_finish(&tp, dfops);
if (error) {
xfs_defer_cancel(dfops);
xfs_trans_cancel(tp);
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 813394c62849..1b625d050441 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -274,7 +274,7 @@ xfs_iomap_write_direct(
/*
* Complete the transaction
*/
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_bmap_cancel;
@@ -784,7 +784,7 @@ xfs_iomap_write_allocate(
if (error)
goto trans_cancel;
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto trans_cancel;
@@ -906,7 +906,7 @@ xfs_iomap_write_unwritten(
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
}
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto error_on_bmapi_transaction;
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index 96fe209b5eb6..8f2e2fac4255 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -525,7 +525,7 @@ xfs_cui_recover(
}
xfs_refcount_finish_one_cleanup(tp, rcur, error);
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto abort_defer;
set_bit(XFS_CUI_RECOVERED, &cuip->cui_flags);
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index f45fbf0db9bb..3246815c24d6 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -464,7 +464,7 @@ xfs_reflink_allocate_cow(
goto out_bmap_cancel;
/* Finish up. */
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_bmap_cancel;
@@ -602,7 +602,8 @@ xfs_reflink_cancel_cow_blocks(
-(long)del.br_blockcount);
/* Roll the transaction */
- error = xfs_defer_finish(tpp, &dfops, ip);
+ xfs_defer_ijoin(&dfops, ip);
+ error = xfs_defer_finish(tpp, &dfops);
if (error) {
xfs_defer_cancel(&dfops);
break;
@@ -791,7 +792,8 @@ xfs_reflink_end_cow(
/* Remove the mapping from the CoW fork. */
xfs_bmap_del_extent_cow(ip, &idx, &got, &del);
- error = xfs_defer_finish(&tp, &dfops, ip);
+ xfs_defer_ijoin(&dfops, ip);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_defer;
next_extent:
@@ -1152,7 +1154,8 @@ xfs_reflink_remap_extent(
next_extent:
/* Process all the deferred stuff. */
- error = xfs_defer_finish(&tp, &dfops, ip);
+ xfs_defer_ijoin(&dfops, ip);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_defer;
}
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 91472193643b..488719d43ca8 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -810,7 +810,7 @@ xfs_growfs_rt_alloc(
/*
* Free any blocks freed up in the transaction, then commit.
*/
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_bmap_cancel;
error = xfs_trans_commit(tp);
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 23a50d7aa46a..68d3ca2c4968 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -378,7 +378,7 @@ xfs_symlink(
xfs_trans_set_sync(tp);
}
- error = xfs_defer_finish(&tp, &dfops, NULL);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_bmap_cancel;
@@ -497,7 +497,8 @@ xfs_inactive_symlink_rmt(
/*
* Commit the first transaction. This logs the EFI and the inode.
*/
- error = xfs_defer_finish(&tp, &dfops, ip);
+ xfs_defer_ijoin(&dfops, ip);
+ error = xfs_defer_finish(&tp, &dfops);
if (error)
goto error_bmap_cancel;
/*
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] xfs: refactor xfs_trans_roll
2017-08-28 7:44 ` [PATCH 1/3] xfs: refactor xfs_trans_roll Christoph Hellwig
@ 2017-08-28 17:33 ` Darrick J. Wong
0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2017-08-28 17:33 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Aug 28, 2017 at 09:44:56AM +0200, Christoph Hellwig wrote:
> Split xfs_trans_roll into a low-level helper that just rolls the
> actual transaction and a new higher level xfs_trans_roll_inode
> that takes care of logging and rejoining the inode. This gets
> rid of the NULL inode case, and allows to simplify the special
> cases in the deferred operation code.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok, will test...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_attr.c | 16 ++++++++--------
> fs/xfs/libxfs/xfs_attr_leaf.c | 6 +++---
> fs/xfs/libxfs/xfs_attr_remote.c | 4 ++--
> fs/xfs/libxfs/xfs_defer.c | 23 +++++++++--------------
> fs/xfs/xfs_attr_inactive.c | 6 +++---
> fs/xfs/xfs_inode.c | 4 ++--
> fs/xfs/xfs_trans.c | 28 +++++-----------------------
> fs/xfs/xfs_trans.h | 3 ++-
> fs/xfs/xfs_trans_inode.c | 14 ++++++++++++++
> 9 files changed, 48 insertions(+), 56 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index de7b9bd30bec..bafa0f6bfafa 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -341,7 +341,7 @@ xfs_attr_set(
> * transaction to add the new attribute to the leaf.
> */
>
> - error = xfs_trans_roll(&args.trans, dp);
> + error = xfs_trans_roll_inode(&args.trans, dp);
> if (error)
> goto out;
>
> @@ -605,7 +605,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
> * Commit the current trans (including the inode) and start
> * a new one.
> */
> - error = xfs_trans_roll(&args->trans, dp);
> + error = xfs_trans_roll_inode(&args->trans, dp);
> if (error)
> return error;
>
> @@ -620,7 +620,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
> * Commit the transaction that added the attr name so that
> * later routines can manage their own transactions.
> */
> - error = xfs_trans_roll(&args->trans, dp);
> + error = xfs_trans_roll_inode(&args->trans, dp);
> if (error)
> return error;
>
> @@ -697,7 +697,7 @@ xfs_attr_leaf_addname(xfs_da_args_t *args)
> /*
> * Commit the remove and start the next trans in series.
> */
> - error = xfs_trans_roll(&args->trans, dp);
> + error = xfs_trans_roll_inode(&args->trans, dp);
>
> } else if (args->rmtblkno > 0) {
> /*
> @@ -885,7 +885,7 @@ xfs_attr_node_addname(xfs_da_args_t *args)
> * Commit the node conversion and start the next
> * trans in the chain.
> */
> - error = xfs_trans_roll(&args->trans, dp);
> + error = xfs_trans_roll_inode(&args->trans, dp);
> if (error)
> goto out;
>
> @@ -925,7 +925,7 @@ xfs_attr_node_addname(xfs_da_args_t *args)
> * Commit the leaf addition or btree split and start the next
> * trans in the chain.
> */
> - error = xfs_trans_roll(&args->trans, dp);
> + error = xfs_trans_roll_inode(&args->trans, dp);
> if (error)
> goto out;
>
> @@ -1012,7 +1012,7 @@ xfs_attr_node_addname(xfs_da_args_t *args)
> /*
> * Commit and start the next trans in the chain.
> */
> - error = xfs_trans_roll(&args->trans, dp);
> + error = xfs_trans_roll_inode(&args->trans, dp);
> if (error)
> goto out;
>
> @@ -1132,7 +1132,7 @@ xfs_attr_node_removename(xfs_da_args_t *args)
> /*
> * Commit the Btree join operation and start a new trans.
> */
> - error = xfs_trans_roll(&args->trans, dp);
> + error = xfs_trans_roll_inode(&args->trans, dp);
> if (error)
> goto out;
> }
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index c6c15e5717e4..5c16db86b38f 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -2608,7 +2608,7 @@ xfs_attr3_leaf_clearflag(
> /*
> * Commit the flag value change and start the next trans in series.
> */
> - return xfs_trans_roll(&args->trans, args->dp);
> + return xfs_trans_roll_inode(&args->trans, args->dp);
> }
>
> /*
> @@ -2659,7 +2659,7 @@ xfs_attr3_leaf_setflag(
> /*
> * Commit the flag value change and start the next trans in series.
> */
> - return xfs_trans_roll(&args->trans, args->dp);
> + return xfs_trans_roll_inode(&args->trans, args->dp);
> }
>
> /*
> @@ -2777,7 +2777,7 @@ xfs_attr3_leaf_flipflags(
> /*
> * Commit the flag value change and start the next trans in series.
> */
> - error = xfs_trans_roll(&args->trans, args->dp);
> + error = xfs_trans_roll_inode(&args->trans, args->dp);
>
> return error;
> }
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> index 5236d8e45146..433c36714e40 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.c
> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> @@ -484,7 +484,7 @@ xfs_attr_rmtval_set(
> /*
> * Start the next trans in the chain.
> */
> - error = xfs_trans_roll(&args->trans, dp);
> + error = xfs_trans_roll_inode(&args->trans, dp);
> if (error)
> return error;
> }
> @@ -621,7 +621,7 @@ xfs_attr_rmtval_remove(
> /*
> * Close out trans and start the next one in the chain.
> */
> - error = xfs_trans_roll(&args->trans, args->dp);
> + error = xfs_trans_roll_inode(&args->trans, args->dp);
> if (error)
> return error;
> }
> diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
> index 5c2929f94bd3..4ea2f068d95c 100644
> --- a/fs/xfs/libxfs/xfs_defer.c
> +++ b/fs/xfs/libxfs/xfs_defer.c
> @@ -240,23 +240,19 @@ xfs_defer_trans_abort(
> STATIC int
> xfs_defer_trans_roll(
> struct xfs_trans **tp,
> - struct xfs_defer_ops *dop,
> - struct xfs_inode *ip)
> + struct xfs_defer_ops *dop)
> {
> int i;
> int error;
>
> - /* Log all the joined inodes except the one we passed in. */
> - for (i = 0; i < XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++) {
> - if (dop->dop_inodes[i] == ip)
> - continue;
> + /* Log all the joined inodes. */
> + for (i = 0; i < XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++)
> xfs_trans_log_inode(*tp, dop->dop_inodes[i], XFS_ILOG_CORE);
> - }
>
> trace_xfs_defer_trans_roll((*tp)->t_mountp, dop);
>
> /* Roll the transaction. */
> - error = xfs_trans_roll(tp, ip);
> + error = xfs_trans_roll(tp);
> if (error) {
> trace_xfs_defer_trans_roll_error((*tp)->t_mountp, dop, error);
> xfs_defer_trans_abort(*tp, dop, error);
> @@ -264,12 +260,9 @@ xfs_defer_trans_roll(
> }
> dop->dop_committed = true;
>
> - /* Rejoin the joined inodes except the one we passed in. */
> - for (i = 0; i < XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++) {
> - if (dop->dop_inodes[i] == ip)
> - continue;
> + /* Rejoin the joined inodes. */
> + for (i = 0; i < XFS_DEFER_OPS_NR_INODES && dop->dop_inodes[i]; i++)
> xfs_trans_ijoin(*tp, dop->dop_inodes[i], 0);
> - }
>
> return error;
> }
> @@ -331,13 +324,15 @@ xfs_defer_finish(
>
> trace_xfs_defer_finish((*tp)->t_mountp, dop);
>
> + xfs_defer_join(dop, ip);
> +
> /* Until we run out of pending work to finish... */
> while (xfs_defer_has_unfinished_work(dop)) {
> /* Log intents for work items sitting in the intake. */
> xfs_defer_intake_work(*tp, dop);
>
> /* Roll the transaction. */
> - error = xfs_defer_trans_roll(tp, dop, ip);
> + error = xfs_defer_trans_roll(tp, dop);
> if (error)
> goto out;
>
> diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
> index be0b79d8900f..ebd66b19fbfc 100644
> --- a/fs/xfs/xfs_attr_inactive.c
> +++ b/fs/xfs/xfs_attr_inactive.c
> @@ -97,7 +97,7 @@ xfs_attr3_leaf_freextent(
> /*
> * Roll to next transaction.
> */
> - error = xfs_trans_roll(trans, dp);
> + error = xfs_trans_roll_inode(trans, dp);
> if (error)
> return error;
> }
> @@ -308,7 +308,7 @@ xfs_attr3_node_inactive(
> /*
> * Atomically commit the whole invalidate stuff.
> */
> - error = xfs_trans_roll(trans, dp);
> + error = xfs_trans_roll_inode(trans, dp);
> if (error)
> return error;
> }
> @@ -375,7 +375,7 @@ xfs_attr3_root_inactive(
> /*
> * Commit the invalidate and start the next transaction.
> */
> - error = xfs_trans_roll(trans, dp);
> + error = xfs_trans_roll_inode(trans, dp);
>
> return error;
> }
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index ff48f0096810..5b4eda44f39f 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1055,7 +1055,7 @@ xfs_dir_ialloc(
> tp->t_flags &= ~(XFS_TRANS_DQ_DIRTY);
> }
>
> - code = xfs_trans_roll(&tp, NULL);
> + code = xfs_trans_roll(&tp);
> if (committed != NULL)
> *committed = 1;
>
> @@ -1611,7 +1611,7 @@ xfs_itruncate_extents(
> if (error)
> goto out_bmap_cancel;
>
> - error = xfs_trans_roll(&tp, ip);
> + error = xfs_trans_roll_inode(&tp, ip);
> if (error)
> goto out;
> }
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 2011620008de..a87f657f59c9 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -1035,25 +1035,18 @@ xfs_trans_cancel(
> */
> int
> xfs_trans_roll(
> - struct xfs_trans **tpp,
> - struct xfs_inode *dp)
> + struct xfs_trans **tpp)
> {
> - struct xfs_trans *trans;
> + struct xfs_trans *trans = *tpp;
> struct xfs_trans_res tres;
> int error;
>
> /*
> - * Ensure that the inode is always logged.
> - */
> - trans = *tpp;
> - if (dp)
> - xfs_trans_log_inode(trans, dp, XFS_ILOG_CORE);
> -
> - /*
> * Copy the critical parameters from one trans to the next.
> */
> tres.tr_logres = trans->t_log_res;
> tres.tr_logcount = trans->t_log_count;
> +
> *tpp = xfs_trans_dup(trans);
>
> /*
> @@ -1067,10 +1060,8 @@ xfs_trans_roll(
> if (error)
> return error;
>
> - trans = *tpp;
> -
> /*
> - * Reserve space in the log for th next transaction.
> + * Reserve space in the log for the next transaction.
> * This also pushes items in the "AIL", the list of logged items,
> * out to disk if they are taking up space at the tail of the log
> * that we want to use. This requires that either nothing be locked
> @@ -1078,14 +1069,5 @@ xfs_trans_roll(
> * the prior and the next transactions.
> */
> tres.tr_logflags = XFS_TRANS_PERM_LOG_RES;
> - error = xfs_trans_reserve(trans, &tres, 0, 0);
> - /*
> - * Ensure that the inode is in the new transaction and locked.
> - */
> - if (error)
> - return error;
> -
> - if (dp)
> - xfs_trans_ijoin(trans, dp, 0);
> - return 0;
> + return xfs_trans_reserve(*tpp, &tres, 0, 0);
> }
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index 7d627721e4b3..b25d3d22e289 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -228,7 +228,8 @@ int xfs_trans_free_extent(struct xfs_trans *,
> struct xfs_efd_log_item *, xfs_fsblock_t,
> xfs_extlen_t, struct xfs_owner_info *);
> int xfs_trans_commit(struct xfs_trans *);
> -int xfs_trans_roll(struct xfs_trans **, struct xfs_inode *);
> +int xfs_trans_roll(struct xfs_trans **);
> +int xfs_trans_roll_inode(struct xfs_trans **, struct xfs_inode *);
> void xfs_trans_cancel(xfs_trans_t *);
> int xfs_trans_ail_init(struct xfs_mount *);
> void xfs_trans_ail_destroy(struct xfs_mount *);
> diff --git a/fs/xfs/xfs_trans_inode.c b/fs/xfs/xfs_trans_inode.c
> index dab8daa676f9..daa7615497f9 100644
> --- a/fs/xfs/xfs_trans_inode.c
> +++ b/fs/xfs/xfs_trans_inode.c
> @@ -134,3 +134,17 @@ xfs_trans_log_inode(
> flags |= ip->i_itemp->ili_last_fields;
> ip->i_itemp->ili_fields |= flags;
> }
> +
> +int
> +xfs_trans_roll_inode(
> + struct xfs_trans **tpp,
> + struct xfs_inode *ip)
> +{
> + int error;
> +
> + xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
> + error = xfs_trans_roll(tpp);
> + if (!error)
> + xfs_trans_ijoin(*tpp, ip, 0);
> + return error;
> +}
> --
> 2.11.0
>
> --
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-08-28 17:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-28 7:44 explicitly join inodes to deferred operations V2 (this time for real) Christoph Hellwig
2017-08-28 7:44 ` [PATCH 1/3] xfs: refactor xfs_trans_roll Christoph Hellwig
2017-08-28 17:33 ` Darrick J. Wong
2017-08-28 7:44 ` [PATCH 2/3] xfs: rename xfs_defer_join to xfs_defer_ijoin Christoph Hellwig
2017-08-28 7:44 ` [PATCH 3/3] xfs: remove the ip argument to xfs_defer_finish Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.