xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/5] xfs: use a cursor for bulk AIL insertion
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 4 Jul 2011 17:20:56 -0400
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1309757260-5484-3-git-send-email-david@xxxxxxxxxxxxx>
References: <1309757260-5484-1-git-send-email-david@xxxxxxxxxxxxx> <1309757260-5484-3-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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

<Prev in Thread] Current Thread [Next in Thread>