Re: [PATCH 4/9] xfs: push the perag outwards in initial allocation

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

The logic looks good, and really helps to distinguis the lowspace
case:

Reviewed-by: Christoph Hellwig <hch@xxxxxx>

However I stll find the loop in xfs_bmap_btalloc_select_lengths a little
suboptimal.  I'd go for something like this incremental patch:


diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 3c250c89f42e92..c1da9e9cfe05f2 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -3268,35 +3268,31 @@ xfs_bmap_btalloc_select_lengths(
 			xfs_perag_rele(pag);
 			return error;
 		}
-		if (*blen >= args->maxlen + stripe_align) {
-			/*
-			 * We are going to target a different AG than the
-			 * incoming target, so we need to reset the target and
-			 * skip exact EOF allocation attempts.
-			 */
-			if (agno != startag) {
-				ap->blkno = XFS_AGB_TO_FSB(mp, agno, 0);
-				ap->aeof = false;
-			}
-			args->pag = pag;
-			break;
-		}
+
 		if (*blen > max_blen) {
 			max_blen = *blen;
 			max_blen_agno = agno;
+			if (*blen >= args->maxlen + stripe_align)
+				goto out;
 		}
 	}
 
-	if (max_blen >= *blen) {
-		ASSERT(args->pag == NULL);
-		if (max_blen_agno != startag) {
-			ap->blkno = XFS_AGB_TO_FSB(mp, max_blen_agno, 0);
-			ap->aeof = false;
-		}
-		*blen = max_blen;
-		args->pag = xfs_perag_grab(mp, max_blen_agno);
+	/*
+	 * We did not find a perfect fit, so pick the AG with the longest
+	 * available free space.
+	 */
+	*blen = max_blen;
+	pag = xfs_perag_grab(mp, max_blen_agno);
+out:
+	/*
+	 * If we are going to target a different AG than the incoming target,
+	 * reset the target and skip exact EOF allocation attempts.
+	 */
+	if (max_blen_agno != startag) {
+		ap->blkno = XFS_AGB_TO_FSB(mp, max_blen_agno, 0);
+		ap->aeof = false;
 	}
-
+	args->pag = pag;
 	args->minlen = xfs_bmap_select_minlen(ap, args, *blen);
 	return 0;
 }