public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: xfs@oss.sgi.com
Subject: [PATCH 4/7] XFS: Don't use log forces when busy extents are allocated
Date: Wed,  8 Oct 2008 09:09:34 +1100	[thread overview]
Message-ID: <1223417377-8679-5-git-send-email-david@fromorbit.com> (raw)
In-Reply-To: <1223417377-8679-1-git-send-email-david@fromorbit.com>

Even though we try to avoid busy extents, there are still situations
where we can't avoid them (such as allocation from the AGFL). To
minimise the overhead of such allocation behaviour, we mark the
transaction doing the allocation as synchronous rather than
triggering a log force to get the previous freeing transaction on
disk.

The synchronous transaction provides the same guarantees as a
synchronous log force because it ensures that all the transactions
are on disk. i.e. it preserves the free->allocate order of the
extent correctly in recovery.

The big advantage to this method comes from the fact that we don't
hold the AGF locked during the writeback of the log buffers during
transaction commit. Hence we can be doing further allocations while
the synchronous transaction is committing, unlike the current
synchronous log force case.

Signed-off-by: Dave Chinner <david@fromorbit.com>
---
 fs/xfs/xfs_ag.h    |    1 -
 fs/xfs/xfs_alloc.c |   93 ++++++++++++++++++++--------------------------------
 2 files changed, 36 insertions(+), 58 deletions(-)

diff --git a/fs/xfs/xfs_ag.h b/fs/xfs/xfs_ag.h
index e7aaa1c..7048d3d 100644
--- a/fs/xfs/xfs_ag.h
+++ b/fs/xfs/xfs_ag.h
@@ -164,7 +164,6 @@ struct xfs_busy_extent {
 	xfs_agnumber_t	agno;
 	xfs_agblock_t	bno;
 	xfs_extlen_t	length;
-	struct xfs_trans *tp;	/* transaction that did the free */
 };
 
 /*
diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
index 77dc18e..f9d092e 100644
--- a/fs/xfs/xfs_alloc.c
+++ b/fs/xfs/xfs_alloc.c
@@ -142,7 +142,6 @@ xfs_alloc_mark_busy(
 	busyp->agno = agno;
 	busyp->bno = bno;
 	busyp->length = len;
-	busyp->tp = tp;
 
 	pag = xfs_perag_get(tp->t_mountp, agno);
 	spin_lock(&pag->pagb_lock);
@@ -155,11 +154,10 @@ xfs_alloc_mark_busy(
 
 /*
  * Search for a busy extent within the range of the extent we are about to
- * allocate.  You need to be holding the busy extent tree lock when calling
- * __xfs_alloc_busy_search().
+ * allocate.
  */
 static struct xfs_busy_extent *
-__xfs_alloc_busy_search(
+xfs_alloc_busy_search(
 	struct xfs_trans	*tp,
 	xfs_agnumber_t		agno,
 	xfs_agblock_t		bno,
@@ -171,6 +169,7 @@ __xfs_alloc_busy_search(
 
 	uend = bno + len - 1;
 	pag = xfs_perag_get(tp->t_mountp, agno);
+	spin_lock(&pag->pagb_lock);
 	rbp = pag->pagb_tree.rb_node;
 	while (rbp) {
 		struct xfs_busy_extent	*busyp;
@@ -183,12 +182,14 @@ __xfs_alloc_busy_search(
 			rbp = rbp->rb_right;
 		else {
 			/* (start1,length1) within (start2, length2) */
-			TRACE_BUSYSEARCH("xfs_alloc_search_busy",
+			TRACE_BUSYSEARCH("xfs_alloc_busy_search",
 					 "found1", agno, bno, len, tp);
+			spin_unlock(&pag->pagb_lock);
 			xfs_perag_put(pag);
 			return busyp;
 		}
 	}
+	spin_unlock(&pag->pagb_lock);
 	xfs_perag_put(pag);
 	return NULL;
 }
@@ -200,13 +201,12 @@ xfs_alloc_clear_busy(
 {
 	struct xfs_perag	*pag;
 
-	pag = xfs_perag_get(tp->t_mountp, busyp->agno);
-	spin_lock(&pag->pagb_lock);
-
 	/* check that the busyp is still in the rbtree */
-	ASSERT(__xfs_alloc_busy_search(tp, busyp->agno, busyp->bno,
+	ASSERT(xfs_alloc_busy_search(tp, busyp->agno, busyp->bno,
 						busyp->length) == busyp);
 
+	pag = xfs_perag_get(tp->t_mountp, busyp->agno);
+	spin_lock(&pag->pagb_lock);
 	TRACE_UNBUSY("xfs_alloc_clear_busy", "found", busyp->agno, busyp->bno,
 							busyp->length, tp);
 	rb_erase(&busyp->rb_node, &pag->pagb_tree);
@@ -217,41 +217,6 @@ xfs_alloc_clear_busy(
 }
 
 /*
- * If we find the extent in the busy list, force the log out to get the
- * extent out of the busy list so the caller can use it straight away.
- */
-STATIC void
-xfs_alloc_search_busy(
-	struct xfs_trans	*tp,
-	xfs_agnumber_t		agno,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len)
-{
-	struct xfs_perag	*pag;
-	struct xfs_busy_extent	*busyp;
-	xfs_lsn_t		lsn;
-
-	pag = xfs_perag_get(tp->t_mountp, agno);
-	spin_lock(&pag->pagb_lock);
-	busyp = __xfs_alloc_busy_search(tp, agno, bno, len);
-	if (!busyp) {
-		TRACE_BUSYSEARCH("xfs_alloc_search_busy", "not-found", agno, bno, len, tp);
-		spin_unlock(&pag->pagb_lock);
-		xfs_perag_put(pag);
-		return;
-	}
-	/*
-	 * A block was found, force the log through the LSN of the
-	 * transaction that freed the block
-	 */
-	TRACE_BUSYSEARCH("xfs_alloc_search_busy", "found", agno, bno, len, tp);
-	lsn = busyp->tp->t_commit_lsn;
-	spin_unlock(&pag->pagb_lock);
-	xfs_log_force(tp->t_mountp, lsn, XFS_LOG_FORCE|XFS_LOG_SYNC);
-	xfs_perag_put(pag);
-}
-
-/*
  * Internal functions.
  */
 
@@ -852,9 +817,16 @@ xfs_alloc_ag_vextent(
 			TRACE_MODAGF(NULL, agf, XFS_AGF_FREEBLKS);
 			xfs_alloc_log_agf(args->tp, args->agbp,
 						XFS_AGF_FREEBLKS);
-			/* search the busylist for these blocks */
-			xfs_alloc_search_busy(args->tp, args->agno,
-					args->agbno, args->len);
+			/*
+			 * Search the busylist for these blocks and mark the
+			 * transaction as synchronous if blocks are found. This
+			 * avoids the need to block in due to a synchronous log
+			 * force to ensure correct ordering as the synchronous
+			 * transaction will guarantee that for us.
+			 */
+			if (xfs_alloc_busy_search(args->tp, args->agno,
+						args->agbno, args->len))
+				xfs_trans_set_sync(args->tp);
 		}
 		if (!args->isfl)
 			xfs_trans_mod_sb(args->tp,
@@ -914,7 +886,7 @@ xfs_alloc_ag_vextent_exact(
 	error = xfs_alloc_get_rec(bno_cur, &fbno, &flen, &i);
 	if (error)
 		goto error0;
-	if (__xfs_alloc_busy_search(args->tp, args->agno, fbno, flen))
+	if (xfs_alloc_busy_search(args->tp, args->agno, fbno, flen))
 		goto not_found;
 	XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
 	ASSERT(fbno <= args->agbno);
@@ -1009,7 +981,7 @@ xfs_alloc_find_best_extent(
 		XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
 		xfs_alloc_compute_aligned(*sbno, *slen, args->alignment,
 					args->minlen, &bno, slena);
-		if (__xfs_alloc_busy_search(args->tp, args->agno, bno, *slena)) {
+		if (xfs_alloc_busy_search(args->tp, args->agno, bno, *slena)) {
 			/* just freed - skip this one */
 			goto next_record;
 		}
@@ -1189,7 +1161,7 @@ xfs_alloc_ag_vextent_near(
 					args->minlen, &ltbnoa, &ltlena);
 			if (ltlena < args->minlen)
 				continue;
-			if (__xfs_alloc_busy_search(args->tp, args->agno, ltbnoa, ltlena))
+			if (xfs_alloc_busy_search(args->tp, args->agno, ltbnoa, ltlena))
 				continue;
 			args->len = XFS_EXTLEN_MIN(ltlena, args->maxlen);
 			xfs_alloc_fix_len(args);
@@ -1311,7 +1283,7 @@ xfs_alloc_ag_vextent_near(
 			xfs_alloc_compute_aligned(ltbno, ltlen, args->alignment,
 					args->minlen, &ltbnoa, &ltlena);
 			if (ltlena >= args->minlen &&
-			    !__xfs_alloc_busy_search(args->tp, args->agno, ltbnoa, ltlena))
+			    !xfs_alloc_busy_search(args->tp, args->agno, ltbnoa, ltlena))
 				break;
 			/*
 			 * clear the length so we don't accidentally use this
@@ -1334,7 +1306,7 @@ xfs_alloc_ag_vextent_near(
 			xfs_alloc_compute_aligned(gtbno, gtlen, args->alignment,
 					args->minlen, &gtbnoa, &gtlena);
 			if (gtlena >= args->minlen &&
-			    !__xfs_alloc_busy_search(args->tp, args->agno, gtbnoa, gtlena))
+			    !xfs_alloc_busy_search(args->tp, args->agno, gtbnoa, gtlena))
 				break;
 			gtlena = 0;
 			if ((error = xfs_btree_increment(bno_cur_gt, 0, &i)))
@@ -1513,7 +1485,7 @@ restart:
 				goto error0;
 			XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
 			if (!check_busy ||
-			    !__xfs_alloc_busy_search(args->tp, args->agno, fbno, flen))
+			    !xfs_alloc_busy_search(args->tp, args->agno, fbno, flen))
 				break;
 			error = xfs_btree_increment(cnt_cur, 0, &i);
 			if (error)
@@ -1566,7 +1538,7 @@ restart:
 			 * a better choice than other extents due to the log
 			 * force penalty of using them.
 			 */
-			if (__xfs_alloc_busy_search(args->tp, args->agno, fbno, flen))
+			if (xfs_alloc_busy_search(args->tp, args->agno, fbno, flen))
 				continue;
 			xfs_alloc_compute_aligned(fbno, flen, args->alignment,
 				args->minlen, &rbno, &rlen);
@@ -2267,10 +2239,17 @@ xfs_alloc_get_freelist(
 	 * and remain there until the freeing transaction is committed to
 	 * disk.  Now that we have allocated blocks, this list must be
 	 * searched to see if a block is being reused.  If one is, then
-	 * the freeing transaction must be pushed to disk NOW by forcing
-	 * to disk all iclogs up that transaction's LSN.
+	 * the freeing transaction must be pushed to disk before this
+	 * transaction.
+	 *
+	 * We do this by setting the current transaction
+	 * to a sync transaction which guarantees that the freeing transaction
+	 * is on disk before this transaction. This is done instead of a
+	 * synchronous log force here so that we don't sit and wait with
+	 * the AGF locked in the transaction during the log force.
 	 */
-	xfs_alloc_search_busy(tp, be32_to_cpu(agf->agf_seqno), bno, 1);
+	if (xfs_alloc_busy_search(tp, be32_to_cpu(agf->agf_seqno), bno, 1))
+		xfs_trans_set_sync(tp);
 	return 0;
 }
 
-- 
1.5.6.5

  parent reply	other threads:[~2008-10-07 22:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-07 22:09 [RFC, PATCH 0/7] XFS: dynamic busy extent tracking Dave Chinner
2008-10-07 22:09 ` [PATCH 1/7] XFS: rename xfs_get_perag Dave Chinner
2008-10-08 18:41   ` Christoph Hellwig
2008-10-07 22:09 ` [PATCH 2/7] XFS: replace fixed size busy extent array with an rbtree Dave Chinner
2008-10-08 18:49   ` Christoph Hellwig
2008-10-09  0:06     ` Dave Chinner
2008-10-07 22:09 ` [PATCH 3/7] XFS: Don't immediately reallocate busy extents Dave Chinner
2008-10-07 22:09 ` Dave Chinner [this message]
2008-10-07 22:09 ` [PATCH 5/7] XFS: Do not classify freed allocation btree blocks as busy Dave Chinner
2008-10-07 22:09 ` [PATCH 6/7] XFS: Avoid busy extent ranges rather than the entire extent Dave Chinner
2008-10-07 22:09 ` [PATCH 7/7] XFS: Simplify transaction busy extent tracking Dave Chinner
2008-10-09 18:17 ` [RFC, PATCH 0/7] XFS: dynamic " Martin Steigerwald
2008-10-09 22:33   ` Dave Chinner
2008-10-10  7:11     ` Martin Steigerwald

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1223417377-8679-5-git-send-email-david@fromorbit.com \
    --to=david@fromorbit.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox