xfsprogs patch for calc_default_ag_geometry

[Eric Sandeen <sandeen@xxxxxxxxxx>]

was just looking at this a little tonight.
I wonder if something like this makes things
any more correct and/or readable.

The patch looks a little ugly but once applied
I think the code makes more sense, it more closely
follows the original spirit of the function, while still
implementing the goal of different geometries for
single disks.

Most importantly it doesn't skip the tricky:

count = dblocks / blocks + (dblocks % blocks != 0);

which is probably what caused the regression.

Or maybe you guys already have your own fix :)

Index: xfsprogs-2.9.5/mkfs/xfs_mkfs.c
===================================================================
--- xfsprogs-2.9.5.orig/mkfs/xfs_mkfs.c
+++ xfsprogs-2.9.5/mkfs/xfs_mkfs.c
@@ -413,6 +413,8 @@ calc_default_ag_geometry(
         * First handle the extremes - the points at which we will
         * always use the maximum AG size, the points at which we
         * always use the minimum, and a "small-step" for 16-128Mb.
+        *
+        * These are chosen regardless of single- or multi-disk.
         */
        if (dblocks >= TERABYTES(32, blocklog)) {
                blocks = XFS_AG_MAX_BLOCKS(blocklog);
@@ -427,6 +429,9 @@ calc_default_ag_geometry(
        }
 
        /*
+        * Sizes in the middle.
+        *
+        * For multidisk:
         * For the remainder we choose an AG size based on the
         * number of data blocks available, trying to keep the
         * number of AGs relatively small (especially compared
@@ -436,34 +441,34 @@ calc_default_ag_geometry(
         * smaller counts at mkfs time.
         *
         * This scales us up smoothly between min/max AG sizes.
+        *
+        * For a single disk:
+        * Limit to 4 ags, unless quite large, then
+        * max out the AG size.
         */
 
-       if (!multidisk) {
-               if (dblocks >= TERABYTES(4, blocklog)) {
+       if (multidisk) {
+               if (dblocks > GIGABYTES(512, blocklog))
+                       shift = 5;
+               else if (dblocks > GIGABYTES(8, blocklog))
+                       shift = 4;
+               else if (dblocks >= MEGABYTES(128, blocklog))
+                       shift = 3;
+               else
+                       ASSERT(0);
+       } else {
+               if (dblocks < TERABYTES(4, blocklog))
+                       shift = 2;
+               else {
                         blocks = XFS_AG_MAX_BLOCKS(blocklog);
-                        goto done;
-                }
-                count = 4;
-
-                goto done;
-        }
-
-       if (dblocks > GIGABYTES(512, blocklog))
-               shift = 5;
-       else if (dblocks > GIGABYTES(8, blocklog))
-               shift = 4;
-       else if (dblocks >= MEGABYTES(128, blocklog))
-               shift = 3;
-       else
-               ASSERT(0);
-       blocks = dblocks >> shift;
+                       goto done;
+               }
+       }
 
+       blocks = dblocks >> shift;
 done:
-       ASSERT (count || blocks);
        if (!count)
                count = dblocks / blocks + (dblocks % blocks != 0);
-       if (!blocks)
-               blocks = dblocks / count;
 
        *agsize = blocks;
        *agcount = count;