Re: [PATCH 2/5] xfs: use a cursor for bulk AIL insertion

[Christoph Hellwig <hch@infradead.org>]

FYI: the following patch implementing my suggested cleanups survived a
few rounds of xfsqa:


Index: xfs/fs/xfs/xfs_trans_ail.c
===================================================================
--- xfs.orig/fs/xfs/xfs_trans_ail.c	2011-07-04 16:04:08.932174424 +0200
+++ xfs/fs/xfs/xfs_trans_ail.c	2011-07-04 16:12:51.402677292 +0200
@@ -267,26 +267,14 @@ out:
 static struct xfs_log_item *
 __xfs_trans_ail_cursor_last(
 	struct xfs_ail		*ailp,
-	struct xfs_ail_cursor	*cur,
-	xfs_lsn_t		lsn,
-	bool			do_init)
+	xfs_lsn_t		lsn)
 {
-	xfs_log_item_t		*lip = NULL;
-
-	if (do_init)
-		xfs_trans_ail_cursor_init(ailp, cur);
+	struct xfs_log_item	*lip;
 
-	if (list_empty(&ailp->xa_ail)) 
-		goto out;
-
-	list_for_each_entry_reverse(lip, &ailp->xa_ail, li_ail) {
+	list_for_each_entry_reverse(lip, &ailp->xa_ail, li_ail)
 		if (XFS_LSN_CMP(lip->li_lsn, lsn) <= 0)
-			break;
-	}
-out:
-	if (cur)
-		cur->item = lip;
-	return lip;
+			return lip;
+	return NULL;
 }
 
 /*
@@ -300,7 +288,9 @@ xfs_trans_ail_cursor_last(
 	struct xfs_ail_cursor	*cur,
 	xfs_lsn_t		lsn)
 {
-	return __xfs_trans_ail_cursor_last(ailp, cur, lsn, true);
+	xfs_trans_ail_cursor_init(ailp, cur);
+	cur->item = __xfs_trans_ail_cursor_last(ailp, lsn);
+	return cur->item;
 }
 
 /*
@@ -319,27 +309,22 @@ xfs_ail_splice(
 	struct xfs_log_item	*lip = cur ? cur->item : NULL;
 	struct xfs_log_item	*next_lip;
 
-	do {
-		/* no placeholder, so get our insert location */
-		if (!lip)
-			lip = __xfs_trans_ail_cursor_last(ailp, cur,
-								lsn, false);
-
+	/*
+	 * Get a new cursor if we do not have a placeholder or an
+	 * invalidated one.
+	 */
+	if (!lip || (__psint_t)lip & 1) {
+		lip = __xfs_trans_ail_cursor_last(ailp, lsn);
 		if (!lip) {
 			/*
-			 * The list is empty, so just splice and return.  Our
-			 * cursor is already guaranteed to be up to date, so we
-			 * don't need to touch it here.
+			 * The list is empty, so just splice and return.
 			 */
+			if (cur)
+				cur->item = NULL;
 			list_splice(list, &ailp->xa_ail);
 			return;
 		}
-
-		/* The placeholder was invalidated, need to get a new cursor */
-		if ((__psint_t)lip & 1)
-			lip = NULL;
-
-	} while (lip == NULL);
+	}
 
 	/*
 	 * Our cursor points to the item we want to insert _after_, so we have

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs