xfs
[Top] [All Lists]

[PATCH 4/5] xfs: convert AIL cursors to use struct list_head

To: xfs@xxxxxxxxxxx
Subject: [PATCH 4/5] xfs: convert AIL cursors to use struct list_head
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 4 Jul 2011 15:27:39 +1000
In-reply-to: <1309757260-5484-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1309757260-5484-1-git-send-email-david@xxxxxxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>

The list of active AIL cursors uses a roll-your-own linked list with
special casing for the AIL push cursor. Simplify this code by
replacing the list with standard struct list_head lists, and use a
separate list_head to track the active cursors so that the code can
just treat the AIL push cursor (which is still embedded into the
struct xfs_ail) as a generic cursor.

Further, fix the duplicate push cursor initialisation that the
special case handling was hiding, and clean up all the comments
around the active cursor list handling.

Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
---
 fs/xfs/xfs_trans_ail.c  |   68 +++++++++++++++-------------------------------
 fs/xfs/xfs_trans_priv.h |    5 ++-
 2 files changed, 25 insertions(+), 48 deletions(-)

diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index de7a52a..3b5b5e4 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -165,15 +165,11 @@ xfs_ail_max_lsn(
 /*
  * AIL traversal cursor initialisation.
  *
- * The cursor keeps track of where our current traversal is up
- * to by tracking the next ƣtem in the list for us. However, for
- * this to be safe, removing an object from the AIL needs to invalidate
- * any cursor that points to it. hence the traversal cursor needs to
- * be linked to the struct xfs_ail so that deletion can search all the
- * active cursors for invalidation.
- *
- * We don't link the push cursor because it is embedded in the struct
- * xfs_ail and hence easily findable.
+ * The cursor keeps track of where our current traversal is up to by tracking
+ * the next ƣtem in the list for us. However, for this to be safe, removing an
+ * object from the AIL needs to invalidate any cursor that points to it. hence
+ * the traversal cursor needs to be linked to the struct xfs_ail so that
+ * deletion can search all the active cursors for invalidation.
  */
 STATIC void
 xfs_trans_ail_cursor_init(
@@ -181,17 +177,13 @@ xfs_trans_ail_cursor_init(
        struct xfs_ail_cursor   *cur)
 {
        cur->item = NULL;
-       if (cur == &ailp->xa_cursors)
-               return;
-
-       cur->next = ailp->xa_cursors.next;
-       ailp->xa_cursors.next = cur;
+       INIT_LIST_HEAD(&cur->list);
+       list_add_tail(&cur->list, &ailp->xa_cursors);
 }
 
 /*
- * Get the next item in the traversal and advance the cursor.
- * If the cursor was invalidated (inidicated by a lip of 1),
- * restart the traversal.
+ * Get the next item in the traversal and advance the cursor.  If the cursor
+ * was invalidated (inidicated by a lip of 1), restart the traversal.
  */
 struct xfs_log_item *
 xfs_trans_ail_cursor_next(
@@ -208,39 +200,24 @@ xfs_trans_ail_cursor_next(
 }
 
 /*
- * Now that the traversal is complete, we need to remove the cursor
- * from the list of traversing cursors. Avoid removing the embedded
- * push cursor, but use the fact it is always present to make the
- * list deletion simple.
+ * When the traversal is complete, we need to remove the cursor from the list
+ * of traversing cursors.
  */
 void
 xfs_trans_ail_cursor_done(
        struct xfs_ail          *ailp,
-       struct xfs_ail_cursor   *done)
+       struct xfs_ail_cursor   *cur)
 {
-       struct xfs_ail_cursor   *prev = NULL;
-       struct xfs_ail_cursor   *cur;
-
-       done->item = NULL;
-       if (done == &ailp->xa_cursors)
-               return;
-       prev = &ailp->xa_cursors;
-       for (cur = prev->next; cur; prev = cur, cur = prev->next) {
-               if (cur == done) {
-                       prev->next = cur->next;
-                       break;
-               }
-       }
-       ASSERT(cur);
+       cur->item = NULL;
+       list_del_init(&cur->list);
 }
 
 /*
- * Invalidate any cursor that is pointing to this item. This is
- * called when an item is removed from the AIL. Any cursor pointing
- * to this object is now invalid and the traversal needs to be
- * terminated so it doesn't reference a freed object. We set the
- * cursor item to a value of 1 so we can distinguish between an
- * invalidation and the end of the list when getting the next item
+ * Invalidate any cursor that is pointing to this item. This is called when an
+ * item is removed from the AIL. Any cursor pointing to this object is now
+ * invalid and the traversal needs to be terminated so it doesn't reference a
+ * freed object. We set the cursor item to a value of 1 so we can distinguish
+ * between an invalidation and the end of the list when getting the next item
  * from the cursor.
  */
 STATIC void
@@ -250,8 +227,7 @@ xfs_trans_ail_cursor_clear(
 {
        struct xfs_ail_cursor   *cur;
 
-       /* need to search all cursors */
-       for (cur = &ailp->xa_cursors; cur; cur = cur->next) {
+       list_for_each_entry(cur, &ailp->xa_cursors, list) {
                if (cur->item == lip)
                        cur->item = (struct xfs_log_item *)
                                        ((__psint_t)cur->item | 1);
@@ -416,7 +392,7 @@ xfs_ail_worker(
        struct xfs_ail          *ailp = container_of(to_delayed_work(work),
                                        struct xfs_ail, xa_work);
        xfs_mount_t             *mp = ailp->xa_mount;
-       struct xfs_ail_cursor   *cur = &ailp->xa_cursors;
+       struct xfs_ail_cursor   *cur = &ailp->xa_push_cursor;
        xfs_log_item_t          *lip;
        xfs_lsn_t               lsn;
        xfs_lsn_t               target;
@@ -428,7 +404,6 @@ xfs_ail_worker(
 
        spin_lock(&ailp->xa_lock);
        target = ailp->xa_target;
-       xfs_trans_ail_cursor_init(ailp, cur);
        lip = xfs_trans_ail_cursor_first(ailp, cur, ailp->xa_last_pushed_lsn);
        if (!lip || XFS_FORCED_SHUTDOWN(mp)) {
                /*
@@ -859,6 +834,7 @@ xfs_trans_ail_init(
 
        ailp->xa_mount = mp;
        INIT_LIST_HEAD(&ailp->xa_ail);
+       INIT_LIST_HEAD(&ailp->xa_cursors);
        spin_lock_init(&ailp->xa_lock);
        INIT_DELAYED_WORK(&ailp->xa_work, xfs_ail_worker);
        mp->m_ail = ailp;
diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h
index c0cb408..a394e2c 100644
--- a/fs/xfs/xfs_trans_priv.h
+++ b/fs/xfs/xfs_trans_priv.h
@@ -53,7 +53,7 @@ void  xfs_trans_committed_bulk(struct xfs_ail *ailp, struct 
xfs_log_vec *lv,
  * of the list to trigger traversal restarts.
  */
 struct xfs_ail_cursor {
-       struct xfs_ail_cursor   *next;
+       struct list_head        list;
        struct xfs_log_item     *item;
 };
 
@@ -66,7 +66,8 @@ struct xfs_ail {
        struct xfs_mount        *xa_mount;
        struct list_head        xa_ail;
        xfs_lsn_t               xa_target;
-       struct xfs_ail_cursor   xa_cursors;
+       struct list_head        xa_cursors;
+       struct xfs_ail_cursor   xa_push_cursor;
        spinlock_t              xa_lock;
        struct delayed_work     xa_work;
        xfs_lsn_t               xa_last_pushed_lsn;
-- 
1.7.5.1

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