Re: [PATCH 1/7] xfs: don't append work items to logged xfs_defer_pending objects

Christoph Hellwig
2023-11-25T05:04:23Z

This looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

To make the code nicer for the later addition of the barrier defer
ops I'd fold the hunk belw to split xfs_defer_try_append, but we could
also do that later:

index 6c283b30ea054a..7be2f9063e0ded 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -624,17 +624,12 @@ xfs_defer_cancel(
xfs_defer_cancel_list(mp, &tp->t_dfops);
}

-/*
- * Decide if we can add a deferred work item to the last dfops item attached
- * to the transaction.
- */
static inline struct xfs_defer_pending *
-xfs_defer_try_append(
+xfs_defer_find(
struct xfs_trans *tp,
- enum xfs_defer_ops_type type,
- const struct xfs_defer_op_type *ops)
+ enum xfs_defer_ops_type type)
{
- struct xfs_defer_pending *dfp = NULL;
+ struct xfs_defer_pending *dfp;

/* No dfops at all? */
if (list_empty(&tp->t_dfops))
@@ -646,16 +641,25 @@ xfs_defer_try_append(
/* Wrong type? */
if (dfp->dfp_type != type)
return NULL;
+ return dfp;
+}

+/*
+ * Decide if we can add a deferred work item to the last dfops item attached
+ * to the transaction.
+ */
+static inline bool
+xfs_defer_can_append(
+ struct xfs_defer_pending *dfp,
+ const struct xfs_defer_op_type *ops)
+{
/* Already logged? */
if (dfp->dfp_intent)
- return NULL;
-
+ return false;
/* Already full? */
if (ops->max_items && dfp->dfp_count >= ops->max_items)
return NULL;
-
- return dfp;
+ return true;
}

/* Add an item for later deferred processing. */
@@ -671,8 +675,8 @@ xfs_defer_add(
ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
BUILD_BUG_ON(ARRAY_SIZE(defer_op_types) != XFS_DEFER_OPS_TYPE_MAX);

- dfp = xfs_defer_try_append(tp, type, ops);
- if (!dfp) {
+ dfp = xfs_defer_find(tp, type);
+ if (!dfp || !xfs_defer_can_append(dfp, ops)) {
/* Create a new pending item at the end of the intake list. */
dfp = kmem_cache_zalloc(xfs_defer_pending_cache,
GFP_NOFS | __GFP_NOFAIL);