On 11/23/21 4:28 PM, Darrick J. Wong wrote:
On Tue, Nov 16, 2021 at 09:13:33PM -0700, Allison Henderson wrote:
If the first operation in a string of defer ops has no intents,
then there is no reason to commit it before running the first call
to xfs_defer_finish_one(). This allows the defer ops to be used
effectively for non-intent based operations without requiring an
unnecessary extra transaction commit when first called.
This fixes a regression in per-attribute modification transaction
count when delayed attributes are not being used.
Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_defer.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index 6dac8d6b8c21..51574f0371b5 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -187,7 +187,7 @@ static const struct xfs_defer_op_type *defer_op_types[] = {
[XFS_DEFER_OPS_TYPE_AGFL_FREE] = &xfs_agfl_free_defer_type,
};
-static void
+static bool
xfs_defer_create_intent(
struct xfs_trans *tp,
struct xfs_defer_pending *dfp,
@@ -198,6 +198,7 @@ xfs_defer_create_intent(
if (!dfp->dfp_intent)
dfp->dfp_intent = ops->create_intent(tp, &dfp->dfp_work,
dfp->dfp_count, sort);
+ return dfp->dfp_intent;
Nit: This ought to be "return dfp->dfp_intent != NULL" to reinforce that
we're returning whether or not the dfp has an associated log item vs.
returning the actual log item.
Sure, I can add that in
}
/*
@@ -205,16 +206,18 @@ xfs_defer_create_intent(
* associated extents, then add the entire intake list to the end of
* the pending list.
*/
-STATIC void
+STATIC bool
xfs_defer_create_intents(
struct xfs_trans *tp)
{
struct xfs_defer_pending *dfp;
+ bool ret = false;
list_for_each_entry(dfp, &tp->t_dfops, dfp_list) {
trace_xfs_defer_create_intent(tp->t_mountp, dfp);
- xfs_defer_create_intent(tp, dfp, true);
+ ret |= xfs_defer_create_intent(tp, dfp, true);
}
+ return ret;
}
/* Abort all the intents that were committed. */
@@ -488,7 +491,7 @@ int
xfs_defer_finish_noroll(
struct xfs_trans **tp)
{
- struct xfs_defer_pending *dfp;
+ struct xfs_defer_pending *dfp = NULL;
int error = 0;
LIST_HEAD(dop_pending);
@@ -507,17 +510,19 @@ xfs_defer_finish_noroll(
* of time that any one intent item can stick around in memory,
* pinning the log tail.
*/
- xfs_defer_create_intents(*tp);
+ bool has_intents = xfs_defer_create_intents(*tp);
list_splice_init(&(*tp)->t_dfops, &dop_pending);
- error = xfs_defer_trans_roll(tp);
- if (error)
- goto out_shutdown;
+ if (has_intents || dfp) {
I can't help but wonder if it would be simpler to make the xattr code
walk through the delattr state machine until there's actually an intent
to log? I forget, which patch in this series actually sets up this
situation?
xfs_attr_set_iter is the function that figures out if we need to go
around again. That function was merged as part of the refactoring
subseries. From here, I think it gets called through the
xfs_defer_finish_one function below, it's just a line or two below where
the diff cuts off. The *_iter routine needs a place to stash away the
state machine and other such things it needs to keep track of once the
operation starts, which in this case is the item.
I'll fiddle with some ideas, it might be possible to use the state of
the attr fork to figure out a chunk of cases that might not need to be
logged, and then just return null in the create_intent call back.
Atomic extent swapping sort of has a similar situation in that the first
transaction logs the inode core update and the swapext intent item, and
that's it.
Thanks for the reviews!
Allison
--D
+ error = xfs_defer_trans_roll(tp);
+ if (error)
+ goto out_shutdown;
- /* Possibly relog intent items to keep the log moving. */
- error = xfs_defer_relog(tp, &dop_pending);
- if (error)
- goto out_shutdown;
+ /* Possibly relog intent items to keep the log moving. */
+ error = xfs_defer_relog(tp, &dop_pending);
+ if (error)
+ goto out_shutdown;
+ }
dfp = list_first_entry(&dop_pending, struct xfs_defer_pending,
dfp_list);
--
2.25.1
