[PATCH 1/4] xfs: use a cursor for bulk AIL insertion

[PATCH 1/4] xfs: use a cursor for bulk AIL insertion

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

Delayed logging can insert tens of thousands of log items into the
AIL at the same LSN. When the committing of log commit records
occur, we can get insertions occurring at an LSN that is not at the
end of the AIL. If there are thousands of items in the AIL on the
tail LSN, each insertion has to walk the AIL to find the correct
place to insert the new item into the AIL. This can consume large
amounts of CPU time and block other operations from occurring while
the traversals are in progress.

To avoid this repeated walk, use a AIL cursor to record
where we should be inserting the new items into the AIL without
having to repeat the walk. The cursor infrastructure already
provides this functionality for push walks, so is a simple extension
of existing code. While this will not avoid the initial walk, it
will avoid repeating it tens of thousands of times during a single
checkpoint commit.

This version includes logic improvements from Christoph Hellwig.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_trans.c      |   27 ++++++++++--
 fs/xfs/xfs_trans_ail.c  |  109 ++++++++++++++++++++++++++++++++++++++---------
 fs/xfs/xfs_trans_priv.h |   10 +++-
 3 files changed, 118 insertions(+), 28 deletions(-)

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index c83f63b..efc147f 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -1426,6 +1426,7 @@ xfs_trans_committed(
 static inline void
 xfs_log_item_batch_insert(
 	struct xfs_ail		*ailp,
+	struct xfs_ail_cursor	*cur,
 	struct xfs_log_item	**log_items,
 	int			nr_items,
 	xfs_lsn_t		commit_lsn)
@@ -1434,7 +1435,7 @@ xfs_log_item_batch_insert(
 
 	spin_lock(&ailp->xa_lock);
 	/* xfs_trans_ail_update_bulk drops ailp->xa_lock */
-	xfs_trans_ail_update_bulk(ailp, log_items, nr_items, commit_lsn);
+	xfs_trans_ail_update_bulk(ailp, cur, log_items, nr_items, commit_lsn);
 
 	for (i = 0; i < nr_items; i++)
 		IOP_UNPIN(log_items[i], 0);
@@ -1452,6 +1453,13 @@ xfs_log_item_batch_insert(
  * 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.
  */
 void
 xfs_trans_committed_bulk(
@@ -1463,8 +1471,13 @@ xfs_trans_committed_bulk(
 #define LOG_ITEM_BATCH_SIZE	32
 	struct xfs_log_item	*log_items[LOG_ITEM_BATCH_SIZE];
 	struct xfs_log_vec	*lv;
+	struct xfs_ail_cursor	cur;
 	int			i = 0;
 
+	spin_lock(&ailp->xa_lock);
+	xfs_trans_ail_cursor_last(ailp, &cur, commit_lsn);
+	spin_unlock(&ailp->xa_lock);
+
 	/* unpin all the log items */
 	for (lv = log_vector; lv; lv = lv->lv_next ) {
 		struct xfs_log_item	*lip = lv->lv_item;
@@ -1493,7 +1506,9 @@ xfs_trans_committed_bulk(
 			/*
 			 * 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.
+			 * 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->xa_lock);
 			if (XFS_LSN_CMP(item_lsn, lip->li_lsn) > 0)
@@ -1507,7 +1522,7 @@ xfs_trans_committed_bulk(
 		/* Item is a candidate for bulk AIL insert.  */
 		log_items[i++] = lv->lv_item;
 		if (i >= LOG_ITEM_BATCH_SIZE) {
-			xfs_log_item_batch_insert(ailp, log_items,
+			xfs_log_item_batch_insert(ailp, &cur, log_items,
 					LOG_ITEM_BATCH_SIZE, commit_lsn);
 			i = 0;
 		}
@@ -1515,7 +1530,11 @@ xfs_trans_committed_bulk(
 
 	/* make sure we insert the remainder! */
 	if (i)
-		xfs_log_item_batch_insert(ailp, log_items, i, commit_lsn);
+		xfs_log_item_batch_insert(ailp, &cur, log_items, i, commit_lsn);
+
+	spin_lock(&ailp->xa_lock);
+	xfs_trans_ail_cursor_done(ailp, &cur);
+	spin_unlock(&ailp->xa_lock);
 }
 
 /*
diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 5fc2380..9a69dc0 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -272,9 +272,9 @@ xfs_trans_ail_cursor_clear(
 }
 
 /*
- * Return the item in the AIL with the current lsn.
- * Return the current tree generation number for use
- * in calls to xfs_trans_next_ail().
+ * Initialise the cursor to the first item in the AIL with the given @lsn.
+ * This searches the list from lowest LSN to highest. Pass a @lsn of zero
+ * to initialise the cursor to the first item in the AIL.
  */
 xfs_log_item_t *
 xfs_trans_ail_cursor_first(
@@ -300,31 +300,97 @@ out:
 }
 
 /*
- * splice the log item list into the AIL at the given LSN.
+ * Initialise the cursor to the last item in the AIL with the given @lsn.
+ * This searches the list from highest LSN to lowest. If there is no item with
+ * the value of @lsn, then it sets the cursor to the last item with an LSN lower
+ * than @lsn.
+ */
+static struct xfs_log_item *
+__xfs_trans_ail_cursor_last(
+	struct xfs_ail		*ailp,
+	xfs_lsn_t		lsn)
+{
+	xfs_log_item_t		*lip;
+
+	list_for_each_entry_reverse(lip, &ailp->xa_ail, li_ail) {
+		if (XFS_LSN_CMP(lip->li_lsn, lsn) <= 0)
+			return lip;
+	}
+	return NULL;
+}
+
+/*
+ * Initialise the cursor to the last item in the AIL with the given @lsn.
+ * This searches the list from highest LSN to lowest.
+ */
+struct xfs_log_item *
+xfs_trans_ail_cursor_last(
+	struct xfs_ail		*ailp,
+	struct xfs_ail_cursor	*cur,
+	xfs_lsn_t		lsn)
+{
+	xfs_trans_ail_cursor_init(ailp, cur);
+	cur->item = __xfs_trans_ail_cursor_last(ailp, lsn);
+	return cur->item;
+}
+
+/*
+ * splice the log item list into the AIL at the given LSN. We splice to the
+ * tail of the given LSN to maintain insert order for push traversals. The
+ * cursor is optional, allowing repeated updates to the same LSN to avoid
+ * repeated traversals.
  */
 static void
 xfs_ail_splice(
-	struct xfs_ail  *ailp,
-	struct list_head *list,
-	xfs_lsn_t       lsn)
+	struct xfs_ail		*ailp,
+	struct xfs_ail_cursor	*cur,
+	struct list_head	*list,
+	xfs_lsn_t		lsn)
 {
-	xfs_log_item_t  *next_lip;
+	struct xfs_log_item	*lip = cur ? cur->item : NULL;
+	struct xfs_log_item	*next_lip;
 
-	/* If the list is empty, just insert the item.  */
-	if (list_empty(&ailp->xa_ail)) {
-		list_splice(list, &ailp->xa_ail);
-		return;
+	/*
+	 * Get a new cursor if we don't have a placeholder or the existing one
+	 * has been invalidated.
+	 */
+	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.  */
+			if (cur)
+				cur->item = NULL;
+			list_splice(list, &ailp->xa_ail);
+			return;
+		}
 	}
 
-	list_for_each_entry_reverse(next_lip, &ailp->xa_ail, li_ail) {
-		if (XFS_LSN_CMP(next_lip->li_lsn, lsn) <= 0)
-			break;
+	/*
+	 * Our cursor points to the item we want to insert _after_, so we have
+	 * to update the cursor to point to the end of the list we are splicing
+	 * in so that it points to the correct location for the next splice.
+	 * i.e. before the splice
+	 *
+	 *  lsn -> lsn -> lsn + x -> lsn + x ...
+	 *          ^
+	 *          | cursor points here
+	 *
+	 * After the splice we have:
+	 *
+	 *  lsn -> lsn -> lsn -> lsn -> .... -> lsn -> lsn + x -> lsn + x ...
+	 *          ^                            ^
+	 *          | cursor points here         | needs to move here
+	 *
+	 * So we set the cursor to the last item in the list to be spliced
+	 * before we execute the splice, resulting in the cursor pointing to
+	 * the correct item after the splice occurs.
+	 */
+	if (cur) {
+		next_lip = list_entry(list->prev, struct xfs_log_item, li_ail);
+		cur->item = next_lip;
 	}
-
-	ASSERT(&next_lip->li_ail == &ailp->xa_ail ||
-	       XFS_LSN_CMP(next_lip->li_lsn, lsn) <= 0);
-
-	list_splice_init(list, &next_lip->li_ail);
+	list_splice(list, &lip->li_ail);
 }
 
 /*
@@ -645,6 +711,7 @@ xfs_trans_unlocked_item(
 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->xa_lock)
@@ -674,7 +741,7 @@ xfs_trans_ail_update_bulk(
 		list_add(&lip->li_ail, &tmp);
 	}
 
-	xfs_ail_splice(ailp, &tmp, lsn);
+	xfs_ail_splice(ailp, cur, &tmp, lsn);
 
 	if (!mlip_changed) {
 		spin_unlock(&ailp->xa_lock);
diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h
index 6b164e9..c0cb408 100644
--- a/fs/xfs/xfs_trans_priv.h
+++ b/fs/xfs/xfs_trans_priv.h
@@ -82,6 +82,7 @@ struct xfs_ail {
 extern struct workqueue_struct	*xfs_ail_wq;	/* AIL workqueue */
 
 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->xa_lock);
 static inline void
@@ -90,7 +91,7 @@ xfs_trans_ail_update(
 	struct xfs_log_item	*lip,
 	xfs_lsn_t		lsn) __releases(ailp->xa_lock)
 {
-	xfs_trans_ail_update_bulk(ailp, &lip, 1, lsn);
+	xfs_trans_ail_update_bulk(ailp, NULL, &lip, 1, lsn);
 }
 
 void	xfs_trans_ail_delete_bulk(struct xfs_ail *ailp,
@@ -111,10 +112,13 @@ xfs_lsn_t		xfs_ail_min_lsn(struct xfs_ail *ailp);
 void			xfs_trans_unlocked_item(struct xfs_ail *,
 					xfs_log_item_t *);
 
-struct xfs_log_item	*xfs_trans_ail_cursor_first(struct xfs_ail *ailp,
+struct xfs_log_item *	xfs_trans_ail_cursor_first(struct xfs_ail *ailp,
 					struct xfs_ail_cursor *cur,
 					xfs_lsn_t lsn);
-struct xfs_log_item	*xfs_trans_ail_cursor_next(struct xfs_ail *ailp,
+struct xfs_log_item *	xfs_trans_ail_cursor_last(struct xfs_ail *ailp,
+					struct xfs_ail_cursor *cur,
+					xfs_lsn_t lsn);
+struct xfs_log_item *	xfs_trans_ail_cursor_next(struct xfs_ail *ailp,
 					struct xfs_ail_cursor *cur);
 void			xfs_trans_ail_cursor_done(struct xfs_ail *ailp,
 					struct xfs_ail_cursor *cur);
-- 
1.7.5.1

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

Re: [PATCH 1/4] xfs: use a cursor for bulk AIL insertion

[Christoph Hellwig]

Looks good,

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

btw, patch 0 is missing from me, did anyone else get it?

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

Re: [PATCH 1/4] xfs: use a cursor for bulk AIL insertion

[Dave Chinner]

On Mon, Jul 18, 2011 at 10:01:15PM -0400, Christoph Hellwig wrote:
> Looks good,
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> btw, patch 0 is missing from me, did anyone else get it?

Doesn't look like it came through on the list. I just checked my
logs and it was accepted by my ISP, so I guess it got eaten
somewhere along the way to the mailing list...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 1/4] xfs: use a cursor for bulk AIL insertion

[Alex Elder]

On Mon, 2011-07-18 at 13:40 +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Delayed logging can insert tens of thousands of log items into the
> AIL at the same LSN. When the committing of log commit records
> occur, we can get insertions occurring at an LSN that is not at the
> end of the AIL. If there are thousands of items in the AIL on the
> tail LSN, each insertion has to walk the AIL to find the correct
> place to insert the new item into the AIL. This can consume large
> amounts of CPU time and block other operations from occurring while
> the traversals are in progress.
> 
> To avoid this repeated walk, use a AIL cursor to record
> where we should be inserting the new items into the AIL without
> having to repeat the walk. The cursor infrastructure already
> provides this functionality for push walks, so is a simple extension
> of existing code. While this will not avoid the initial walk, it
> will avoid repeating it tens of thousands of times during a single
> checkpoint commit.
> 
> This version includes logic improvements from Christoph Hellwig.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

I think there's a case that can be improved (though
it isn't wrong as-is), and assuming I'm right, I have
provided a modified splice function (not tested), below.

But if you don't want to change anything, this code
looks OK to me, so:

Reviewed-by: Alex Elder <aelder@sgi.com>

. . .

> +
> +/*
> + * splice the log item list into the AIL at the given LSN. We splice to the
> + * tail of the given LSN to maintain insert order for push traversals. The
> + * cursor is optional, allowing repeated updates to the same LSN to avoid
> + * repeated traversals.
>   */
>  static void
>  xfs_ail_splice(
> -	struct xfs_ail  *ailp,
> -	struct list_head *list,
> -	xfs_lsn_t       lsn)
> +	struct xfs_ail		*ailp,
> +	struct xfs_ail_cursor	*cur,
> +	struct list_head	*list,
> +	xfs_lsn_t		lsn)
>  {
> -	xfs_log_item_t  *next_lip;
> +	struct xfs_log_item	*lip = cur ? cur->item : NULL;
> +	struct xfs_log_item	*next_lip;
>  
> -	/* If the list is empty, just insert the item.  */
> -	if (list_empty(&ailp->xa_ail)) {
> -		list_splice(list, &ailp->xa_ail);
> -		return;
> +	/*
> +	 * Get a new cursor if we don't have a placeholder or the existing one
> +	 * has been invalidated.
> +	 */
> +	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.  */
> +			if (cur)
> +				cur->item = NULL;

If the AIL was empty, I think we still want to
make the cursor point to the end of the list that's
being spliced in, don't we?

> +			list_splice(list, &ailp->xa_ail);
> +			return;
> +		}
>  	}
>  
> -	list_for_each_entry_reverse(next_lip, &ailp->xa_ail, li_ail) {
> -		if (XFS_LSN_CMP(next_lip->li_lsn, lsn) <= 0)
> -			break;
> +	/*
> +	 * Our cursor points to the item we want to insert _after_, so we have
> +	 * to update the cursor to point to the end of the list we are splicing
> +	 * in so that it points to the correct location for the next splice.
> +	 * i.e. before the splice
> +	 *
> +	 *  lsn -> lsn -> lsn + x -> lsn + x ...
> +	 *          ^
> +	 *          | cursor points here
> +	 *
> +	 * After the splice we have:
> +	 *
> +	 *  lsn -> lsn -> lsn -> lsn -> .... -> lsn -> lsn + x -> lsn + x ...
> +	 *          ^                            ^
> +	 *          | cursor points here         | needs to move here
> +	 *
> +	 * So we set the cursor to the last item in the list to be spliced
> +	 * before we execute the splice, resulting in the cursor pointing to
> +	 * the correct item after the splice occurs.
> +	 */
> +	if (cur) {
> +		next_lip = list_entry(list->prev, struct xfs_log_item, li_ail);
> +		cur->item = next_lip;
>  	}
> -
> -	ASSERT(&next_lip->li_ail == &ailp->xa_ail ||
> -	       XFS_LSN_CMP(next_lip->li_lsn, lsn) <= 0);
> -
> -	list_splice_init(list, &next_lip->li_ail);
> +	list_splice(list, &lip->li_ail);
>  }
>  
>  /*


So assuming my comment above is right, how about this:

static void
xfs_ail_splice(
        struct xfs_ail          *ailp,
        struct xfs_ail_cursor   *cur,
        struct list_head        *list,
        xfs_lsn_t               lsn)
{
        struct xfs_log_item     *lip;

        /*
         * Use the cursor to determine the insertion point if one is
         * provided.
         */
        lip = cur ? cur->item : NULL;
        if (!lip || (__psint_t) lip & 1)
                lip = __xfs_trans_ail_cursor_last(ailp, lsn);

        /*
         * If a cursor is provided, we know we're processing the AIL
         * in lsn order, and future items to be spliced in will
         * follow the last one being inserted now.  Update the
         * cursor to point to that last item, now while we have a
         * reliable pointer to it.
         */
        if (cur)
                cur->item = list_entry(list->prev, struct xfs_log_item,
li_ail);

        /*
         * Finally perform the splice.  Unless the AIL was empty,
         * lip points to the item in the AIL _after_ which the new
         * items should go.  If lip is null the AIL was empty, so
         * the new items go at the head of the AIL.
         */
        if (lip)
                list_splice(list, &lip->li_ail);
        else
                list_splice(list, &ailp->xa_ail);
}


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

Re: [PATCH 1/4] xfs: use a cursor for bulk AIL insertion

[Dave Chinner]

On Tue, Jul 19, 2011 at 06:03:44PM -0500, Alex Elder wrote:
> On Mon, 2011-07-18 at 13:40 +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Delayed logging can insert tens of thousands of log items into the
> > AIL at the same LSN. When the committing of log commit records
> > occur, we can get insertions occurring at an LSN that is not at the
> > end of the AIL. If there are thousands of items in the AIL on the
> > tail LSN, each insertion has to walk the AIL to find the correct
> > place to insert the new item into the AIL. This can consume large
> > amounts of CPU time and block other operations from occurring while
> > the traversals are in progress.
> > 
> > To avoid this repeated walk, use a AIL cursor to record
> > where we should be inserting the new items into the AIL without
> > having to repeat the walk. The cursor infrastructure already
> > provides this functionality for push walks, so is a simple extension
> > of existing code. While this will not avoid the initial walk, it
> > will avoid repeating it tens of thousands of times during a single
> > checkpoint commit.
> > 
> > This version includes logic improvements from Christoph Hellwig.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> 
> I think there's a case that can be improved (though
> it isn't wrong as-is), and assuming I'm right, I have
> provided a modified splice function (not tested), below.
> 
> But if you don't want to change anything, this code
> looks OK to me, so:
> 
> Reviewed-by: Alex Elder <aelder@sgi.com>
> 
> . . .
> 
> > +
> > +/*
> > + * splice the log item list into the AIL at the given LSN. We splice to the
> > + * tail of the given LSN to maintain insert order for push traversals. The
> > + * cursor is optional, allowing repeated updates to the same LSN to avoid
> > + * repeated traversals.
> >   */
> >  static void
> >  xfs_ail_splice(
> > -	struct xfs_ail  *ailp,
> > -	struct list_head *list,
> > -	xfs_lsn_t       lsn)
> > +	struct xfs_ail		*ailp,
> > +	struct xfs_ail_cursor	*cur,
> > +	struct list_head	*list,
> > +	xfs_lsn_t		lsn)
> >  {
> > -	xfs_log_item_t  *next_lip;
> > +	struct xfs_log_item	*lip = cur ? cur->item : NULL;
> > +	struct xfs_log_item	*next_lip;
> >  
> > -	/* If the list is empty, just insert the item.  */
> > -	if (list_empty(&ailp->xa_ail)) {
> > -		list_splice(list, &ailp->xa_ail);
> > -		return;
> > +	/*
> > +	 * Get a new cursor if we don't have a placeholder or the existing one
> > +	 * has been invalidated.
> > +	 */
> > +	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.  */
> > +			if (cur)
> > +				cur->item = NULL;
> 
> If the AIL was empty, I think we still want to
> make the cursor point to the end of the list that's
> being spliced in, don't we?

Not sure - we do get different LSNs in the same commit and the
insertions are interleaved. Hence I was defensive here and revalidate
the cursor on the next use of it.

> > +		cur->item = next_lip;
> >  	}
> > -
> > -	ASSERT(&next_lip->li_ail == &ailp->xa_ail ||
> > -	       XFS_LSN_CMP(next_lip->li_lsn, lsn) <= 0);
> > -
> > -	list_splice_init(list, &next_lip->li_ail);
> > +	list_splice(list, &lip->li_ail);
> >  }
> >  
> >  /*
> 
> 
> So assuming my comment above is right, how about this:
> 
> static void
> xfs_ail_splice(
>         struct xfs_ail          *ailp,
>         struct xfs_ail_cursor   *cur,
>         struct list_head        *list,
>         xfs_lsn_t               lsn)
> {
>         struct xfs_log_item     *lip;
> 
>         /*
>          * Use the cursor to determine the insertion point if one is
>          * provided.
>          */
>         lip = cur ? cur->item : NULL;
>         if (!lip || (__psint_t) lip & 1)
>                 lip = __xfs_trans_ail_cursor_last(ailp, lsn);
> 
>         /*
>          * If a cursor is provided, we know we're processing the AIL
>          * in lsn order, and future items to be spliced in will
>          * follow the last one being inserted now.  Update the
>          * cursor to point to that last item, now while we have a
>          * reliable pointer to it.
>          */
>         if (cur)
>                 cur->item = list_entry(list->prev, struct xfs_log_item,
> li_ail);
> 
>         /*
>          * Finally perform the splice.  Unless the AIL was empty,
>          * lip points to the item in the AIL _after_ which the new
>          * items should go.  If lip is null the AIL was empty, so
>          * the new items go at the head of the AIL.
>          */
>         if (lip)
>                 list_splice(list, &lip->li_ail);
>         else
>                 list_splice(list, &ailp->xa_ail);
> }


Looks cleaner, but I'll need to test it. Right now all my test
resources are busy with non-mainline stuff, so it's going to be next
week sometime before I can do this.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 1/4] xfs: use a cursor for bulk AIL insertion

[Christoph Hellwig]

On Wed, Jul 20, 2011 at 09:36:44AM +1000, Dave Chinner wrote:
> Looks cleaner, but I'll need to test it. Right now all my test
> resources are busy with non-mainline stuff, so it's going to be next
> week sometime before I can do this.

Given that the merge window is about to open how about putting in the
current version for now and doing a cleanup patch on top later on?

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

Re: [PATCH 1/4] xfs: use a cursor for bulk AIL insertion

[Dave Chinner]

On Tue, Jul 19, 2011 at 08:15:31PM -0400, Christoph Hellwig wrote:
> On Wed, Jul 20, 2011 at 09:36:44AM +1000, Dave Chinner wrote:
> > Looks cleaner, but I'll need to test it. Right now all my test
> > resources are busy with non-mainline stuff, so it's going to be next
> > week sometime before I can do this.
> 
> Given that the merge window is about to open how about putting in the
> current version for now and doing a cleanup patch on top later on?

I'm happy with that....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

Re: [PATCH 1/4] xfs: use a cursor for bulk AIL insertion

[Alex Elder]

On Tue, 2011-07-19 at 20:15 -0400, Christoph Hellwig wrote:
> On Wed, Jul 20, 2011 at 09:36:44AM +1000, Dave Chinner wrote:
> > Looks cleaner, but I'll need to test it. Right now all my test
> > resources are busy with non-mainline stuff, so it's going to be next
> > week sometime before I can do this.
> 
> Given that the merge window is about to open how about putting in the
> current version for now and doing a cleanup patch on top later on?
> 

Will do.

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

Re: [PATCH 1/4] xfs: use a cursor for bulk AIL insertion

[Alex Elder]

On Wed, 2011-07-20 at 09:36 +1000, Dave Chinner wrote:
> On Tue, Jul 19, 2011 at 06:03:44PM -0500, Alex Elder wrote:
> > On Mon, 2011-07-18 at 13:40 +1000, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > Delayed logging can insert tens of thousands of log items into the
> > > AIL at the same LSN. When the committing of log commit records
> > > occur, we can get insertions occurring at a
. . .
> _after_ which the new
> >          * items should go.  If lip is null the AIL was empty, so
> >          * the new items go at the head of the AIL.
> >          */
> >         if (lip)
> >                 list_splice(list, &lip->li_ail);
> >         else
> >                 list_splice(list, &ailp->xa_ail);
> > }
> 
> 
> Looks cleaner, but I'll need to test it. Right now all my test
> resources are busy with non-mainline stuff, so it's going to be next
> week sometime before I can do this.

I'll actually try to test this today.	-Alex

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

[PATCH 2/4] xfs: remove confusing ail cursor wrapper

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

xfs_trans_ail_cursor_set() doesn't set the cursor to the current log
item, it sets it to the next item. There is already a function for
doing this - xfs_trans_ail_cursor_next() - and the _set function is
simply a two line wrapper.  Remove it and open code the setting of
the cursor in the two locations that call it to remove the
confusion.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_trans_ail.c |   50 ++++++++++++++++++-----------------------------
 1 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 9a69dc0..7908c79 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -189,20 +189,6 @@ xfs_trans_ail_cursor_init(
 }
 
 /*
- * Set the cursor to the next item, because when we look
- * up the cursor the current item may have been freed.
- */
-STATIC void
-xfs_trans_ail_cursor_set(
-	struct xfs_ail		*ailp,
-	struct xfs_ail_cursor	*cur,
-	struct xfs_log_item	*lip)
-{
-	if (lip)
-		cur->item = xfs_ail_next(ailp, lip);
-}
-
-/*
  * 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.
@@ -216,7 +202,8 @@ xfs_trans_ail_cursor_next(
 
 	if ((__psint_t)lip & 1)
 		lip = xfs_ail_min(ailp);
-	xfs_trans_ail_cursor_set(ailp, cur, lip);
+	if (lip)
+		cur->item = xfs_ail_next(ailp, lip);
 	return lip;
 }
 
@@ -272,9 +259,10 @@ xfs_trans_ail_cursor_clear(
 }
 
 /*
- * Initialise the cursor to the first item in the AIL with the given @lsn.
- * This searches the list from lowest LSN to highest. Pass a @lsn of zero
- * to initialise the cursor to the first item in the AIL.
+ * Find the first item in the AIL with the given @lsn by searching in ascending
+ * LSN order and initialise the cursor to point to the next item for a
+ * ascending traversal.  Pass a @lsn of zero to initialise the cursor to the
+ * first item in the AIL. Returns NULL if the list is empty.
  */
 xfs_log_item_t *
 xfs_trans_ail_cursor_first(
@@ -285,26 +273,24 @@ xfs_trans_ail_cursor_first(
 	xfs_log_item_t		*lip;
 
 	xfs_trans_ail_cursor_init(ailp, cur);
-	lip = xfs_ail_min(ailp);
-	if (lsn == 0)
+
+	if (lsn == 0) {
+		lip = xfs_ail_min(ailp);
 		goto out;
+	}
 
 	list_for_each_entry(lip, &ailp->xa_ail, li_ail) {
 		if (XFS_LSN_CMP(lip->li_lsn, lsn) >= 0)
 			goto out;
 	}
-	lip = NULL;
+	return NULL;
+
 out:
-	xfs_trans_ail_cursor_set(ailp, cur, lip);
+	if (lip)
+		cur->item = xfs_ail_next(ailp, lip);
 	return lip;
 }
 
-/*
- * Initialise the cursor to the last item in the AIL with the given @lsn.
- * This searches the list from highest LSN to lowest. If there is no item with
- * the value of @lsn, then it sets the cursor to the last item with an LSN lower
- * than @lsn.
- */
 static struct xfs_log_item *
 __xfs_trans_ail_cursor_last(
 	struct xfs_ail		*ailp,
@@ -320,8 +306,10 @@ __xfs_trans_ail_cursor_last(
 }
 
 /*
- * Initialise the cursor to the last item in the AIL with the given @lsn.
- * This searches the list from highest LSN to lowest.
+ * Find the last item in the AIL with the given @lsn by searching in descending
+ * LSN order and initialise the cursor to point to that item.  If there is no
+ * item with the value of @lsn, then it sets the cursor to the last item with an
+ * LSN lower than @lsn.  Returns NULL if the list is empty.
  */
 struct xfs_log_item *
 xfs_trans_ail_cursor_last(
@@ -335,7 +323,7 @@ xfs_trans_ail_cursor_last(
 }
 
 /*
- * splice the log item list into the AIL at the given LSN. We splice to the
+ * Splice the log item list into the AIL at the given LSN. We splice to the
  * tail of the given LSN to maintain insert order for push traversals. The
  * cursor is optional, allowing repeated updates to the same LSN to avoid
  * repeated traversals.
-- 
1.7.5.1

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

Re: [PATCH 2/4] xfs: remove confusing ail cursor wrapper

[Alex Elder]

On Mon, 2011-07-18 at 13:40 +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> xfs_trans_ail_cursor_set() doesn't set the cursor to the current log
> item, it sets it to the next item. There is already a function for
> doing this - xfs_trans_ail_cursor_next() - and the _set function is
> simply a two line wrapper.  Remove it and open code the setting of
> the cursor in the two locations that call it to remove the
> confusion.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

I already reviewed this but I do see you did a tiny change
or two.  In any case, this (still) looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

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

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

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. This allows us to
treat the AIL push cursor as a generic cursor rather than as a
special case, further simplifying the code.

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@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_trans_ail.c  |   77 ++++++++++++++++-------------------------------
 fs/xfs/xfs_trans_priv.h |    4 +-
 2 files changed, 28 insertions(+), 53 deletions(-)

diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 7908c79..8cd329b 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 item 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,12 @@ 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;
+	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 (indicated by a lip of 1), restart the traversal.
  */
 struct xfs_log_item *
 xfs_trans_ail_cursor_next(
@@ -208,40 +199,25 @@ 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
- * from the cursor.
+ * 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 low bit of the cursor item pointer so we can
+ * distinguish between an invalidation and the end of the list when getting the
+ * next item from the cursor.
  */
 STATIC void
 xfs_trans_ail_cursor_clear(
@@ -250,8 +226,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);
@@ -405,7 +380,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;
 	xfs_log_item_t		*lip;
 	xfs_lsn_t		lsn;
 	xfs_lsn_t		target;
@@ -417,13 +392,12 @@ 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);
+	lip = xfs_trans_ail_cursor_first(ailp, &cur, ailp->xa_last_pushed_lsn);
 	if (!lip || XFS_FORCED_SHUTDOWN(mp)) {
 		/*
 		 * AIL is empty or our push has reached the end.
 		 */
-		xfs_trans_ail_cursor_done(ailp, cur);
+		xfs_trans_ail_cursor_done(ailp, &cur);
 		spin_unlock(&ailp->xa_lock);
 		goto out_done;
 	}
@@ -511,12 +485,12 @@ xfs_ail_worker(
 		if (stuck > 100)
 			break;
 
-		lip = xfs_trans_ail_cursor_next(ailp, cur);
+		lip = xfs_trans_ail_cursor_next(ailp, &cur);
 		if (lip == NULL)
 			break;
 		lsn = lip->li_lsn;
 	}
-	xfs_trans_ail_cursor_done(ailp, cur);
+	xfs_trans_ail_cursor_done(ailp, &cur);
 	spin_unlock(&ailp->xa_lock);
 
 	if (flush_log) {
@@ -848,6 +822,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..212946b 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,7 @@ 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;
 	spinlock_t		xa_lock;
 	struct delayed_work	xa_work;
 	xfs_lsn_t		xa_last_pushed_lsn;
-- 
1.7.5.1

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

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

[Alex Elder]

On Mon, 2011-07-18 at 13:40 +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> 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. This allows us to
> treat the AIL push cursor as a generic cursor rather than as a
> special case, further simplifying the code.
> 
> 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@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

You didn't fix one thing I mentioned before in a comment.
I can fix that for you if you like.  Other than that, this
looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>

> ---
>  fs/xfs/xfs_trans_ail.c  |   77 ++++++++++++++++-------------------------------
>  fs/xfs/xfs_trans_priv.h |    4 +-
>  2 files changed, 28 insertions(+), 53 deletions(-)
> 
> diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> index 7908c79..8cd329b 100644
> --- a/fs/xfs/xfs_trans_ail.c
> +++ b/fs/xfs/xfs_trans_ail.c

. . .

> @@ -181,17 +177,12 @@ xfs_trans_ail_cursor_init(

. . .

> - * 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 (indicated by a lip of 1), restart the traversal.
 * was invalidated (indicated by its item pointer's low bit being set),
 * restart the traversal.

>   */
>  struct xfs_log_item *
>  xfs_trans_ail_cursor_next(

. . .

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

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

[Dave Chinner]

On Tue, Jul 19, 2011 at 06:03:56PM -0500, Alex Elder wrote:
> On Mon, 2011-07-18 at 13:40 +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > 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. This allows us to
> > treat the AIL push cursor as a generic cursor rather than as a
> > special case, further simplifying the code.
> > 
> > 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@redhat.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> You didn't fix one thing I mentioned before in a comment.
> I can fix that for you if you like.  Other than that, this
> looks good.
> 
> Reviewed-by: Alex Elder <aelder@sgi.com>
> 
> > ---
> >  fs/xfs/xfs_trans_ail.c  |   77 ++++++++++++++++-------------------------------
> >  fs/xfs/xfs_trans_priv.h |    4 +-
> >  2 files changed, 28 insertions(+), 53 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> > index 7908c79..8cd329b 100644
> > --- a/fs/xfs/xfs_trans_ail.c
> > +++ b/fs/xfs/xfs_trans_ail.c
> 
> . . .
> 
> > @@ -181,17 +177,12 @@ xfs_trans_ail_cursor_init(
> 
> . . .
> 
> > - * 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 (indicated by a lip of 1), restart the traversal.
>  * was invalidated (indicated by its item pointer's low bit being set),
>  * restart the traversal.

I thought I fixed that. Hmmmm, I did fix a comment that said the
same thing - just it wasn't the one you noticed. :/ I'll fix it up
when I redo the splice code again.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

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

[Alex Elder]

On Wed, 2011-07-20 at 09:43 +1000, Dave Chinner wrote:
> On Tue, Jul 19, 2011 at 06:03:56PM -0500, Alex Elder wrote:
> > On Mon, 2011-07-18 at 13:40 +1000, Dave Chinne
. . .
> > > + * Get the next item in the traversal and advance the cursor.  If the cursor
> > > + * was invalidated (indicated by a lip of 1), restart the traversal.
> >  * was invalidated (indicated by its item pointer's low bit being set),
> >  * restart the traversal.
> 
> I thought I fixed that. Hmmmm, I did fix a comment that said the
> same thing - just it wasn't the one you noticed. :/ I'll fix it up
> when I redo the splice code again.

Don't worry about it.  I'll change it (as I did here)
in my own copy and will use that (unless you give me
something newer before I'm ready to publish it).

You are welcome to update it later, of course.

					-Alex

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

[PATCH 4/4] xfs: add size update tracepoint to IO completion

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

For improving insight into IO completion behaviour.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Alex Elder <aelder@sgi.com>
---
 fs/xfs/linux-2.6/xfs_aops.c  |    1 +
 fs/xfs/linux-2.6/xfs_trace.h |   12 ++++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_aops.c b/fs/xfs/linux-2.6/xfs_aops.c
index f865350..63e971e 100644
--- a/fs/xfs/linux-2.6/xfs_aops.c
+++ b/fs/xfs/linux-2.6/xfs_aops.c
@@ -181,6 +181,7 @@ xfs_setfilesize(
 
 	isize = xfs_ioend_new_eof(ioend);
 	if (isize) {
+		trace_xfs_setfilesize(ip, ioend->io_offset, ioend->io_size);
 		ip->i_d.di_size = isize;
 		xfs_mark_inode_dirty(ip);
 	}
diff --git a/fs/xfs/linux-2.6/xfs_trace.h b/fs/xfs/linux-2.6/xfs_trace.h
index 3bdfcb9..fda0708 100644
--- a/fs/xfs/linux-2.6/xfs_trace.h
+++ b/fs/xfs/linux-2.6/xfs_trace.h
@@ -998,7 +998,8 @@ DECLARE_EVENT_CLASS(xfs_simple_io_class,
 	TP_STRUCT__entry(
 		__field(dev_t, dev)
 		__field(xfs_ino_t, ino)
-		__field(loff_t, size)
+		__field(loff_t, isize)
+		__field(loff_t, disize)
 		__field(loff_t, new_size)
 		__field(loff_t, offset)
 		__field(size_t, count)
@@ -1006,16 +1007,18 @@ DECLARE_EVENT_CLASS(xfs_simple_io_class,
 	TP_fast_assign(
 		__entry->dev = VFS_I(ip)->i_sb->s_dev;
 		__entry->ino = ip->i_ino;
-		__entry->size = ip->i_d.di_size;
+		__entry->isize = ip->i_size;
+		__entry->disize = ip->i_d.di_size;
 		__entry->new_size = ip->i_new_size;
 		__entry->offset = offset;
 		__entry->count = count;
 	),
-	TP_printk("dev %d:%d ino 0x%llx size 0x%llx new_size 0x%llx "
+	TP_printk("dev %d:%d ino 0x%llx isize 0x%llx disize 0x%llx new_size 0x%llx "
 		  "offset 0x%llx count %zd",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->ino,
-		  __entry->size,
+		  __entry->isize,
+		  __entry->disize,
 		  __entry->new_size,
 		  __entry->offset,
 		  __entry->count)
@@ -1028,6 +1031,7 @@ DEFINE_EVENT(xfs_simple_io_class, name,	\
 DEFINE_SIMPLE_IO_EVENT(xfs_delalloc_enospc);
 DEFINE_SIMPLE_IO_EVENT(xfs_unwritten_convert);
 DEFINE_SIMPLE_IO_EVENT(xfs_get_blocks_notfound);
+DEFINE_SIMPLE_IO_EVENT(xfs_setfilesize);
 
 DECLARE_EVENT_CLASS(xfs_itrunc_class,
 	TP_PROTO(struct xfs_inode *ip, xfs_fsize_t new_size),
-- 
1.7.5.1

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