patch move_chechpoint_to_ailtesting

author: Carlos Maiolino <cem@kernel.org> 2025-02-20 13:32:44 +0100
committer: Carlos Maiolino <cem@kernel.org> 2025-02-20 13:32:44 +0100
commit: 90430e39422c2efc34b3dbe3e27a9260a54eba39 (patch)
tree: 8ab669640ccd2c11280fd4570c7c8648e35c8d79
parent: 53652c0c79b90ae49ef5dfd113f83111d7df8671 (diff)
download: xfs-linux-testing.tar.gz
3 files changed, 194 insertions, 195 deletions
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index b797586958c998..7e47cb74c6a31a 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -689,193 +689,6 @@ xlog_cil_insert_items(
 	}
 }
 
-static inline void
-xlog_cil_ail_insert_batch(
-	struct xfs_ail		*ailp,
-	struct xfs_ail_cursor	*cur,
-	struct xfs_log_item	**log_items,
-	int			nr_items,
-	xfs_lsn_t		commit_lsn)
-{
-	int	i;
-
-	spin_lock(&ailp->ail_lock);
-	/* xfs_trans_ail_update_bulk drops ailp->ail_lock */
-	xfs_trans_ail_update_bulk(ailp, cur, log_items, nr_items, commit_lsn);
-
-	for (i = 0; i < nr_items; i++) {
-		struct xfs_log_item *lip = log_items[i];
-
-		if (lip->li_ops->iop_unpin)
-			lip->li_ops->iop_unpin(lip, 0);
-	}
-}
-
-/*
- * Take the checkpoint's log vector chain of items and insert the attached log
- * items into the AIL. This uses bulk insertion techniques to minimise AIL lock
- * traffic.
- *
- * The AIL tracks log items via the start record LSN of the checkpoint,
- * not the commit record LSN. This is because we can pipeline multiple
- * checkpoints, and so the start record of checkpoint N+1 can be
- * written before the commit record of checkpoint N. i.e:
- *
- *   start N			commit N
- *	+-------------+------------+----------------+
- *		  start N+1			commit N+1
- *
- * The tail of the log cannot be moved to the LSN of commit N when all
- * the items of that checkpoint are written back, because then the
- * start record for N+1 is no longer in the active portion of the log
- * and recovery will fail/corrupt the filesystem.
- *
- * Hence when all the log items in checkpoint N are written back, the
- * tail of the log most now only move as far forwards as the start LSN
- * of checkpoint N+1.
- *
- * If we are called with the aborted flag set, it is because a log write during
- * a CIL checkpoint commit has failed. In this case, all the items in the
- * checkpoint have already gone through iop_committed and iop_committing, which
- * means that checkpoint commit abort handling is treated exactly the same as an
- * iclog write error even though we haven't started any IO yet. Hence in this
- * case all we need to do is iop_committed processing, followed by an
- * iop_unpin(aborted) call.
- *
- * The AIL cursor is used to optimise the insert process. If commit_lsn is not
- * at the end of the AIL, the insert cursor avoids the need to walk the AIL to
- * find the insertion point on every xfs_log_item_batch_insert() call. This
- * saves a lot of needless list walking and is a net win, even though it
- * slightly increases that amount of AIL lock traffic to set it up and tear it
- * down.
- */
-static void
-xlog_cil_ail_insert(
-	struct xlog_chkpt	*ctx,
-	bool			aborted)
-{
-#define LOG_ITEM_BATCH_SIZE	32
-	struct xfs_ail		*ailp = ctx->cil->xc_log->l_ailp;
-	struct xfs_log_item	*log_items[LOG_ITEM_BATCH_SIZE];
-	struct xfs_log_vec	*lv;
-	struct xfs_ail_cursor	cur;
-	xfs_lsn_t		old_head;
-	int			i = 0;
-
-	/*
-	 * Update the AIL head LSN with the commit record LSN of this
-	 * checkpoint. As iclogs are always completed in order, this should
-	 * always be the same (as iclogs can contain multiple commit records) or
-	 * higher LSN than the current head. We do this before insertion of the
-	 * items so that log space checks during insertion will reflect the
-	 * space that this checkpoint has already consumed.  We call
-	 * xfs_ail_update_finish() so that tail space and space-based wakeups
-	 * will be recalculated appropriately.
-	 */
-	ASSERT(XFS_LSN_CMP(ctx->commit_lsn, ailp->ail_head_lsn) >= 0 ||
-			aborted);
-	spin_lock(&ailp->ail_lock);
-	xfs_trans_ail_cursor_last(ailp, &cur, ctx->start_lsn);
-	old_head = ailp->ail_head_lsn;
-	ailp->ail_head_lsn = ctx->commit_lsn;
-	/* xfs_ail_update_finish() drops the ail_lock */
-	xfs_ail_update_finish(ailp, NULLCOMMITLSN);
-
-	/*
-	 * We move the AIL head forwards to account for the space used in the
-	 * log before we remove that space from the grant heads. This prevents a
-	 * transient condition where reservation space appears to become
-	 * available on return, only for it to disappear again immediately as
-	 * the AIL head update accounts in the log tail space.
-	 */
-	smp_wmb();	/* paired with smp_rmb in xlog_grant_space_left */
-	xlog_grant_return_space(ailp->ail_log, old_head, ailp->ail_head_lsn);
-
-	/* unpin all the log items */
-	list_for_each_entry(lv, &ctx->lv_chain, lv_list) {
-		struct xfs_log_item	*lip = lv->lv_item;
-		xfs_lsn_t		item_lsn;
-
-		if (aborted)
-			set_bit(XFS_LI_ABORTED, &lip->li_flags);
-
-		if (lip->li_ops->flags & XFS_ITEM_RELEASE_WHEN_COMMITTED) {
-			lip->li_ops->iop_release(lip);
-			continue;
-		}
-
-		if (lip->li_ops->iop_committed)
-			item_lsn = lip->li_ops->iop_committed(lip,
-					ctx->start_lsn);
-		else
-			item_lsn = ctx->start_lsn;
-
-		/* item_lsn of -1 means the item needs no further processing */
-		if (XFS_LSN_CMP(item_lsn, (xfs_lsn_t)-1) == 0)
-			continue;
-
-		/*
-		 * if we are aborting the operation, no point in inserting the
-		 * object into the AIL as we are in a shutdown situation.
-		 */
-		if (aborted) {
-			ASSERT(xlog_is_shutdown(ailp->ail_log));
-			if (lip->li_ops->iop_unpin)
-				lip->li_ops->iop_unpin(lip, 1);
-			continue;
-		}
-
-		if (item_lsn != ctx->start_lsn) {
-
-			/*
-			 * Not a bulk update option due to unusual item_lsn.
-			 * Push into AIL immediately, rechecking the lsn once
-			 * we have the ail lock. Then unpin the item. This does
-			 * not affect the AIL cursor the bulk insert path is
-			 * using.
-			 */
-			spin_lock(&ailp->ail_lock);
-			if (XFS_LSN_CMP(item_lsn, lip->li_lsn) > 0)
-				xfs_trans_ail_update(ailp, lip, item_lsn);
-			else
-				spin_unlock(&ailp->ail_lock);
-			if (lip->li_ops->iop_unpin)
-				lip->li_ops->iop_unpin(lip, 0);
-			continue;
-		}
-
-		/* Item is a candidate for bulk AIL insert.  */
-		log_items[i++] = lv->lv_item;
-		if (i >= LOG_ITEM_BATCH_SIZE) {
-			xlog_cil_ail_insert_batch(ailp, &cur, log_items,
-					LOG_ITEM_BATCH_SIZE, ctx->start_lsn);
-			i = 0;
-		}
-	}
-
-	/* make sure we insert the remainder! */
-	if (i)
-		xlog_cil_ail_insert_batch(ailp, &cur, log_items, i,
-				ctx->start_lsn);
-
-	spin_lock(&ailp->ail_lock);
-	xfs_trans_ail_cursor_done(&cur);
-	spin_unlock(&ailp->ail_lock);
-}
-
-static void
-xlog_cil_free_logvec(
-	struct list_head	*lv_chain)
-{
-	struct xfs_log_vec	*lv;
-
-	while (!list_empty(lv_chain)) {
-		lv = list_first_entry(lv_chain, struct xfs_log_vec, lv_list);
-		list_del_init(&lv->lv_list);
-		kvfree(lv);
-	}
-}
-
 /*
  * Mark all items committed and clear busy extents. We free the log vector
  * chains in a separate pass so that we unpin the log items as quickly as
@@ -912,20 +725,29 @@ xlog_cil_committed(
 		spin_unlock(&ctx->cil->xc_push_lock);
 	}
 
-	xlog_cil_ail_insert(ctx, abort);
+	/*
+	 * Remove the checkpoint context from the committing list while we still
+	 * owns it. I.e. before we transfer it to the AIL code.
+	 *
+	 * XXX: By code analysis, removing the context from the committing list
+	 *	doesn't seem to have any bad impact on it, and we prevent messing
+	 *	around with the context after the transfer.
+	 *	I don't think it's a good idea to do this in the AIL code, as I'd
+	 *	rather keep all cil push lock usage self contained within cil code.
+	 *	Otherwise, a helper for the AIL to call in after all the transfer
+	 *	has been done looks pertinent.
+	 */
+	spin_lock(&ctx->cil->xc_push_lock);
+	list_del(&ctx->committing);
+	spin_unlock(&ctx->cil->xc_push_lock);
+
+	xfs_trans_ail_chkpt_transfer(ctx, abort);
 
 	xfs_extent_busy_sort(&extents->extent_list);
 	xfs_extent_busy_clear(&extents->extent_list,
 			      xfs_has_discard(mp) && !abort);
 
-	spin_lock(&ctx->cil->xc_push_lock);
-	list_del(&ctx->committing);
-	spin_unlock(&ctx->cil->xc_push_lock);
-
-	xlog_cil_free_logvec(&ctx->lv_chain);
 	xfs_discard_extents(mp, extents);
-
-	kfree(ctx);
 }
 
 void
diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 0fcb1828e598fb..6a896c63c5d411 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -863,6 +863,178 @@ xfs_trans_ail_insert(
 	xfs_trans_ail_update_bulk(ailp, NULL, &lip, 1, lsn);
 }
 
+static inline void
+xfs_trans_ail_insert_batch(
+	struct xfs_ail		*ailp,
+	struct xfs_ail_cursor	*cur,
+	struct xfs_log_item	**log_items,
+	int			nr_items,
+	xfs_lsn_t		commit_lsn)
+{
+	int	i;
+
+	spin_lock(&ailp->ail_lock);
+	xfs_trans_ail_update_bulk(ailp, cur, log_items, nr_items, commit_lsn);
+
+	for (i = 0; i < nr_items; i++) {
+		struct xfs_log_item *lip = log_items[i];
+
+		if (lip->li_ops->iop_unpin)
+			lip->li_ops->iop_unpin(lip, 0);
+	}
+}
+
+/*
+ * Take the checkpoint's context from CIL and transfer the log items
+ * (linked through their log vectors) to the AIL.
+ *
+ * Once it's called, the checkpoint context can't be access by the CIL
+ * anymore.
+ *
+ * This uses bulk insertion techniques to minimise AIL lock traffic.
+ *
+ * The AIL tracks log items via the start record LSN of the checkpoint,
+ * not the commit record LSN. This is because we can pipeline multiple
+ * checkpoints, and so the start record of checkpoint N+1 can be written
+ * before the commit record of checkpoint N. i.e:
+ *
+ *   start N                   commit N
+ *     +-------------+------------+----------------+
+ *               start N+1                     commit N+1
+ *
+ * The tail of the log cannot be moved to the LSN of commit N when all
+ * the items of that checkpoint are written back, because then, the
+ * start record for N+1 is no longer in the active portion of the log
+ * and recovery will fail/corrupt the filesystem.
+ *
+ * Hence when all the log items in checkpoint N are written back, the
+ * tail of the log must now only move as far forwards as the start LSN
+ * of checkpoint N+1.
+ *
+ * If we are called with the aborted flag set, it is because a log write during
+ * a CIL checkpoint commit has failed. In this case, all the items in the
+ * checkpoint have already gone through iop_committed and iop_committing, which
+ * means that checkpoint commit abort handling is treated exactly the same as an
+ * iclog write error, even though we haven't started any IO yet. Hence in this
+ * case, all we need to do is iop_committed processing, followed by an
+ * iop_unpin(aborted) call.
+ *
+ * The AIL cursor is used to optimise the insert process. If commit_lsn is not
+ * at the end of the AIL, the insert cursor avoids the need to walk the AIL to
+ * find the insertion point on every xfs_trans_ail_insert_batch() call. This
+ * saves a lot of needless list walking and is a net win, even though it
+ * slightly increases the amount of AIL lock traffic to set it up and tear it
+ * down.
+ */
+void
+xfs_trans_ail_chkpt_transfer(
+	struct xlog_chkpt	*chkpt,
+	bool			aborted)
+{
+# define LOG_ITEM_BATCH_SIZE	32
+	struct xfs_ail		*ailp = chkpt->cil->xc_log->l_ailp;
+	struct xfs_log_item	*log_items[LOG_ITEM_BATCH_SIZE];
+	struct xfs_log_vec	*lv;
+	struct xfs_ail_cursor	cur;
+	xfs_lsn_t		old_head;
+	int			i = 0;
+
+	ASSERT(XFS_LSN_CMP(chkpt->commit_lsn, ailp->ail_head_lsn) >= 0 ||
+			   aborted);
+
+	spin_lock(&ailp->ail_lock);
+	xfs_trans_ail_cursor_last(ailp, &cur, chkpt->start_lsn);
+	old_head = ailp->ail_head_lsn;
+	ailp->ail_head_lsn = chkpt->commit_lsn;
+
+	/* xfs_ail_update_finish() dropts the ail_lock */
+	xfs_ail_update_finish(ailp, NULLCOMMITLSN);
+
+	/* XXX: If we are returning the space here, where we took it? */
+	smp_wmb();	/* paired with smp_rmb in xlog_grant_space_left */
+	xlog_grant_return_space(ailp->ail_log, old_head, ailp->ail_head_lsn);
+
+	/* unpin all the log items with log vectors in this checkpoint */
+	list_for_each_entry(lv, &chkpt->lv_chain, lv_list) {
+		struct xfs_log_item	*lip = lv->lv_item;
+		xfs_lsn_t		item_lsn;
+
+		if (aborted)
+			set_bit(XFS_LI_ABORTED, &lip->li_flags);
+
+		if (lip->li_ops->flags & XFS_ITEM_RELEASE_WHEN_COMMITTED) {
+			lip->li_ops->iop_release(lip);
+			continue;
+		}
+
+		if (lip->li_ops->iop_committed)
+			item_lsn = lip->li_ops->iop_committed(lip,
+					      chkpt->start_lsn);
+		else
+			item_lsn = chkpt->start_lsn;
+
+		if (XFS_LSN_CMP(item_lsn, (xfs_lsn_t)-1) == 0)
+		    continue;
+
+		if (aborted) {
+			ASSERT(xlog_is_shutdown(ailp->ail_log));
+			if (lip->li_ops->iop_unpin)
+				lip->li_ops->iop_unpin(lip, 1);
+			continue;
+		}
+
+		if (item_lsn != chkpt->start_lsn) {
+			spin_lock(&ailp->ail_lock);
+			if (XFS_LSN_CMP(item_lsn, lip->li_lsn) > 0)
+				xfs_trans_ail_update(ailp, lip, item_lsn);
+			else
+				spin_unlock(&ailp->ail_lock);
+
+			if (lip->li_ops->iop_unpin)
+				lip->li_ops->iop_unpin(lip, 0);
+
+			continue;
+		}
+
+		log_items[i++] = lv->lv_item;
+		if (i >= LOG_ITEM_BATCH_SIZE) {
+			xfs_trans_ail_insert_batch(ailp, &cur, log_items,
+					LOG_ITEM_BATCH_SIZE, chkpt->start_lsn);
+			i = 0;
+		}
+	}
+
+	if (i)
+		xfs_trans_ail_insert_batch(ailp, &cur, log_items, i,
+				chkpt->start_lsn);
+
+	spin_lock(&ailp->ail_lock);
+	xfs_trans_ail_cursor_done(&cur);
+	spin_unlock(&ailp->ail_lock);
+
+	/*
+	 * We are done with the checkpoint context, clean up the lv_chain.
+	 *
+	 * This opencode the old xlog_cil_free_logvec()
+	 *
+	 * Reusing lv pointer here as has no usage anymore, instead of
+	 * defining a new pointer just for it
+	 *
+	 * XXX: We are freeing the log vector here, is this safe? Don't AIL code
+	 *	uses it to push the item to the log?
+	 */
+
+	while (!list_empty(&chkpt->lv_chain)) {
+	       lv = list_first_entry(&chkpt->lv_chain, struct xfs_log_vec,
+				lv_list);
+
+		list_del_init(&lv->lv_list);
+		kvfree(lv);
+	}
+
+	kfree(chkpt);
+}
+
 /*
  * Delete one log item from the AIL.
  *
diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h
index bd841df93021ff..7c26b6c592e5e8 100644
--- a/fs/xfs/xfs_trans_priv.h
+++ b/fs/xfs/xfs_trans_priv.h
@@ -12,6 +12,7 @@ struct xfs_mount;
 struct xfs_trans;
 struct xfs_ail;
 struct xfs_log_vec;
+struct xlog_chkpt;
 
 
 void	xfs_trans_init(struct xfs_mount *);
@@ -73,6 +74,7 @@ void	xfs_trans_ail_update_bulk(struct xfs_ail *ailp,
 				struct xfs_ail_cursor *cur,
 				struct xfs_log_item **log_items, int nr_items,
 				xfs_lsn_t lsn) __releases(ailp->ail_lock);
+
 /*
  * Return a pointer to the first item in the AIL.  If the AIL is empty, then
  * return NULL.
@@ -102,6 +104,9 @@ void xfs_ail_update_finish(struct xfs_ail *ailp, xfs_lsn_t old_lsn)
 			__releases(ailp->ail_lock);
 void xfs_trans_ail_delete(struct xfs_log_item *lip, int shutdown_type);
 
+/* Move the checkpoint context into the AIL code */
+void xfs_trans_ail_chkpt_transfer(struct xlog_chkpt *chkpt, bool aborted);
+
 static inline void xfs_ail_push(struct xfs_ail *ailp)
 {
 	wake_up_process(ailp->ail_task);
author	Carlos Maiolino <cem@kernel.org>	2025-02-20 13:32:44 +0100
committer	Carlos Maiolino <cem@kernel.org>	2025-02-20 13:32:44 +0100
commit	90430e39422c2efc34b3dbe3e27a9260a54eba39 (patch)
tree	8ab669640ccd2c11280fd4570c7c8648e35c8d79
parent	53652c0c79b90ae49ef5dfd113f83111d7df8671 (diff)
download	xfs-linux-testing.tar.gz