public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 35/36] xfs: convert xfs_alloc_vextent_iterate_ags() to use perag walker
Date: Fri,  3 Dec 2021 11:01:10 +1100	[thread overview]
Message-ID: <20211203000111.2800982-36-david@fromorbit.com> (raw)
In-Reply-To: <20211203000111.2800982-1-david@fromorbit.com>

From: Dave Chinner <dchinner@redhat.com>

Now that the AG iteration code in the core allocation code has been
cleaned up, we can easily convert it to use a for_each_perag..()
variant to use active references and skip AGs that it can't get
active references on.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_ag.h    | 22 ++++++---
 fs/xfs/libxfs/xfs_alloc.c | 97 ++++++++++++++++++++-------------------
 2 files changed, 65 insertions(+), 54 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
index 23040a1094b9..2198166efa2f 100644
--- a/fs/xfs/libxfs/xfs_ag.h
+++ b/fs/xfs/libxfs/xfs_ag.h
@@ -244,6 +244,7 @@ xfs_perag_next_wrap(
 	struct xfs_perag	*pag,
 	xfs_agnumber_t		*agno,
 	xfs_agnumber_t		stop_agno,
+	xfs_agnumber_t		restart_agno,
 	xfs_agnumber_t		wrap_agno)
 {
 	struct xfs_mount	*mp = pag->pag_mount;
@@ -251,10 +252,11 @@ xfs_perag_next_wrap(
 	*agno = pag->pag_agno + 1;
 	xfs_perag_rele(pag);
 	while (*agno != stop_agno) {
-		if (*agno >= wrap_agno)
-			*agno = 0;
-		if (*agno == stop_agno)
-			break;
+		if (*agno >= wrap_agno) {
+			if (restart_agno >= stop_agno)
+				break;
+			*agno = restart_agno;
+		}
 
 		pag = xfs_perag_grab(mp, *agno);
 		if (pag)
@@ -265,14 +267,20 @@ xfs_perag_next_wrap(
 }
 
 /*
- * Iterate all AGs from start_agno through wrap_agno, then 0 through
+ * Iterate all AGs from start_agno through wrap_agno, then restart_agno through
  * (start_agno - 1).
  */
-#define for_each_perag_wrap_at(mp, start_agno, wrap_agno, agno, pag) \
+#define for_each_perag_wrap_range(mp, start_agno, restart_agno, wrap_agno, agno, pag) \
 	for ((agno) = (start_agno), (pag) = xfs_perag_grab((mp), (agno)); \
 		(pag) != NULL; \
 		(pag) = xfs_perag_next_wrap((pag), &(agno), (start_agno), \
-				(wrap_agno)))
+				(restart_agno), (wrap_agno)))
+/*
+ * Iterate all AGs from start_agno through wrap_agno, then 0 through
+ * (start_agno - 1).
+ */
+#define for_each_perag_wrap_at(mp, start_agno, wrap_agno, agno, pag) \
+	for_each_perag_wrap_range((mp), (start_agno), 0, (wrap_agno), (agno), (pag))
 
 /*
  * Iterate all AGs from start_agno through to the end of the filesystem, then 0
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index b15dee98090a..71bfb0c6150c 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -3132,6 +3132,7 @@ xfs_alloc_vextent_prepare_ag(
 	if (need_pag)
 		args->pag = xfs_perag_get(args->mp, args->agno);
 
+	args->agbp = NULL;
 	error = xfs_alloc_fix_freelist(args, 0);
 	if (error) {
 		trace_xfs_alloc_vextent_nofix(args);
@@ -3253,6 +3254,10 @@ xfs_alloc_vextent_this_ag(
  * transaction. This will result in an out-of-order locking of AGFs and hence
  * can cause deadlocks.
  *
+ * On return, args->pag may be left referenced if we finish before the "all
+ * failed" return point. The allocation finish still needs the perag, and
+ * so the caller will release it once they've finished the allocation.
+ *
  * XXX(dgc): when wrapping in potential deadlock scenarios, we could use
  * try-locks on the AGFs below the critical AG rather than skip them entirely.
  * We won't deadlock in that case, we'll just skip the AGFs we can't lock.
@@ -3265,67 +3270,65 @@ xfs_alloc_vextent_iterate_ags(
 	uint32_t		flags)
 {
 	struct xfs_mount	*mp = args->mp;
+	xfs_agnumber_t		restart_agno = 0;
+	xfs_agnumber_t		agno;
 	int			error = 0;
 
 	/*
-	 * Loop over allocation groups twice; first time with
-	 * trylock set, second time without.
+	 * If we already have allocated a block in this transaction, we don't
+	 * want to lock AGs whose number is below the start AG. This results in
+	 * out-of-order locking of AGF and deadlocks will result.
 	 */
-	args->agno = start_agno;
-	for (;;) {
+	if (args->tp->t_firstblock != NULLFSBLOCK)
+		restart_agno = start_agno;
+
+restart:
+	for_each_perag_wrap_range(mp, start_agno, restart_agno,
+			mp->m_sb.sb_agcount, agno, args->pag) {
+		args->agno = agno;
 		args->agbno = target_agbno;
-		args->pag = xfs_perag_get(args->mp, args->agno);
+		trace_printk("sag %u rag %u agno %u pag %u, agbno %u, agcnt %u",
+			start_agno, restart_agno, agno, args->pag->pag_agno,
+			target_agbno, mp->m_sb.sb_agcount);
+
 		error = xfs_alloc_vextent_prepare_ag(args);
 		if (error)
 			break;
-
-		if (args->agbp) {
-			/*
-			 * Allocation is supposed to succeed now, so break out
-			 * of the loop regardless of whether we succeed or not.
-			 */
-			if (args->agno == start_agno && target_agbno)
-				error = xfs_alloc_ag_vextent_near(args);
-			else
-				error = xfs_alloc_ag_vextent_size(args);
-			break;
+		if (!args->agbp) {
+			trace_xfs_alloc_vextent_loopfailed(args);
+			continue;
 		}
 
-		trace_xfs_alloc_vextent_loopfailed(args);
-
 		/*
-		* For the first allocation, we can try any AG to get
-		* space.  However, if we already have allocated a
-		* block, we don't want to try AGs whose number is below
-		* sagno. Otherwise, we may end up with out-of-order
-		* locking of AGF, which might cause deadlock.
-		*/
-		if (++(args->agno) == mp->m_sb.sb_agcount) {
-			if (args->tp->t_firstblock != NULLFSBLOCK)
-				args->agno = start_agno;
-			else
-				args->agno = 0;
-		}
-		/*
-		 * Reached the starting a.g., must either be done
-		 * or switch to non-trylock mode.
+		 * Allocation is supposed to succeed now, so break out of the
+		 * loop regardless of whether we succeed or not.
 		 */
-		if (args->agno == start_agno) {
-			if (flags == 0) {
-				args->agbno = NULLAGBLOCK;
-				trace_xfs_alloc_vextent_allfailed(args);
-				break;
-			}
-			flags = 0;
-		}
-		xfs_perag_put(args->pag);
+		if (args->agno == start_agno && target_agbno)
+			error = xfs_alloc_ag_vextent_near(args);
+		else
+			error = xfs_alloc_ag_vextent_size(args);
+		break;
+	}
+	if (error) {
+		xfs_perag_rele(args->pag);
 		args->pag = NULL;
+		return error;
 	}
+	if (args->agbp)
+		return 0;
+
 	/*
-	 * On success, perag is left referenced in args for the caller to clean
-	 * up after they've finished the allocation.
+	 * We didn't find an AG we can alloation from. If we were given
+	 * constraining flags by the caller, drop them and retry the allocation
+	 * without any constraints being set.
 	 */
-	return error;
+	if (flags) {
+		flags = 0;
+		goto restart;
+	}
+
+	trace_xfs_alloc_vextent_allfailed(args);
+	return 0;
 }
 
 /*
@@ -3367,7 +3370,7 @@ xfs_alloc_vextent_start_ag(
 	if (!error)
 		error = xfs_alloc_vextent_finish(args);
 	if (args->pag) {
-		xfs_perag_put(args->pag);
+		xfs_perag_rele(args->pag);
 		args->pag = NULL;
 	}
 
@@ -3408,7 +3411,7 @@ xfs_alloc_vextent_first_ag(
 	if (!error)
 		error = xfs_alloc_vextent_finish(args);
 	if (args->pag) {
-		xfs_perag_put(args->pag);
+		xfs_perag_rele(args->pag);
 		args->pag = NULL;
 	}
 	return error;
-- 
2.33.0


  parent reply	other threads:[~2021-12-03  0:01 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-03  0:00 [RFC] [PATCH 00/36] xfs: more work towards shrinking Dave Chinner
2021-12-03  0:00 ` [PATCH 01/36] xfs: make last AG grow/shrink perag centric Dave Chinner
2021-12-03  0:00 ` [PATCH 02/36] xfs: kill xfs_ialloc_pagi_init() Dave Chinner
2021-12-03  0:00 ` [PATCH 03/36] xfs: pass perag to xfs_ialloc_read_agi() Dave Chinner
2021-12-03  0:00 ` [PATCH 04/36] xfs: kill xfs_alloc_pagf_init() Dave Chinner
2021-12-03  0:00 ` [PATCH 05/36] xfs: pass perag to xfs_alloc_read_agf() Dave Chinner
2021-12-03  5:37   ` kernel test robot
2021-12-03  7:10   ` [RFC PATCH] xfs: xfs_reflink_find_shared can be static kernel test robot
2021-12-03  7:19   ` [PATCH 05/36] xfs: pass perag to xfs_alloc_read_agf() kernel test robot
2021-12-03  7:19   ` kernel test robot
2021-12-03  0:00 ` [PATCH 06/36] xfs: pass perag to xfs_read_agi Dave Chinner
2021-12-03  0:00 ` [PATCH 07/36] xfs: pass perag to xfs_read_agf Dave Chinner
2021-12-03  0:00 ` [PATCH 08/36] xfs: pass perag to xfs_alloc_get_freelist Dave Chinner
2021-12-03  0:00 ` [PATCH 09/36] xfs: pass perag to xfs_alloc_put_freelist Dave Chinner
2021-12-03  0:00 ` [PATCH 10/36] xfs: pass perag to xfs_alloc_read_agfl Dave Chinner
2021-12-03  0:00 ` [PATCH 11/36] xfs: Pre-calculate per-AG agbno geometry Dave Chinner
2021-12-03  0:00 ` [PATCH 12/36] xfs: Pre-calculate per-AG agino geometry Dave Chinner
2021-12-03  0:00 ` [PATCH 13/36] xfs: replace xfs_ag_block_count() with perag accesses Dave Chinner
2021-12-03  0:00 ` [PATCH 14/36] xfs: make is_log_ag() a first class helper Dave Chinner
2021-12-03  0:00 ` [PATCH 15/36] xfs: active perag reference counting Dave Chinner
2021-12-03  0:00 ` [PATCH 16/36] xfs: rework the perag trace points to be perag centric Dave Chinner
2021-12-03  0:00 ` [PATCH 17/36] xfs: convert xfs_imap() to take a perag Dave Chinner
2021-12-03  0:00 ` [PATCH 18/36] xfs: use active perag references for inode allocation Dave Chinner
2021-12-03  0:00 ` [PATCH 19/36] xfs: inobt can use perags in many more places than it does Dave Chinner
2021-12-03  0:00 ` [PATCH 20/36] xfs: convert xfs_ialloc_next_ag() to an atomic Dave Chinner
2021-12-03  0:00 ` [PATCH 21/36] xfs: perags need atomic operational state Dave Chinner
2021-12-03  0:00 ` [PATCH 22/36] xfs: introduce xfs_for_each_perag_wrap() Dave Chinner
2021-12-03  0:00 ` [PATCH 23/36] xfs: rework xfs_alloc_vextent() Dave Chinner
2021-12-03  0:00 ` [PATCH 24/36] xfs: use xfs_alloc_vextent_this_ag() in _iterate_ags() Dave Chinner
2021-12-03  0:01 ` [PATCH 25/36] xfs: combine __xfs_alloc_vextent_this_ag and xfs_alloc_ag_vextent Dave Chinner
2021-12-03  0:01 ` [PATCH 26/36] xfs: use xfs_alloc_vextent_this_ag() where appropriate Dave Chinner
2021-12-03  0:01 ` [PATCH 27/36] xfs: factor xfs_bmap_btalloc() Dave Chinner
2021-12-03  0:01 ` [PATCH 28/36] xfs: use xfs_alloc_vextent_first_ag() where appropriate Dave Chinner
2021-12-03  0:01 ` [PATCH 29/36] xfs: use xfs_alloc_vextent_start_bno() " Dave Chinner
2021-12-03  0:01 ` [PATCH 30/36] xfs: introduce xfs_alloc_vextent_near_bno() Dave Chinner
2021-12-03  0:01 ` [PATCH 31/36] xfs: introduce xfs_alloc_vextent_exact_bno() Dave Chinner
2021-12-03  0:01 ` [PATCH 32/36] xfs: introduce xfs_alloc_vextent_prepare() Dave Chinner
2021-12-03  0:01 ` [PATCH 33/36] xfs: move allocation accounting to xfs_alloc_vextent_set_fsbno() Dave Chinner
2021-12-03  0:01 ` [PATCH 34/36] xfs: fold xfs_alloc_ag_vextent() into callers Dave Chinner
2021-12-03  0:01 ` Dave Chinner [this message]
2021-12-03  0:01 ` [PATCH 36/36] xfs: convert trim to use for_each_perag_range Dave Chinner

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=20211203000111.2800982-36-david@fromorbit.com \
    --to=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /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