Skip to contentSkip to conversation

mkfs.xfs: go into multidisk mode when geometry is on cmdline

RFC PATCH Superseded Eric Sandeen 3 participants 10 messages Nov 20, 2012 – Dec 10, 2012

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 mkfs.xfs: go into multidisk mode when geometry is on cmdline

In the course of some other investigations, I found that calc_default_ag_geometry() doesn't go into "multidisk" mode unless stripe geometry is *detected* (i.e. by the blkid routines).

Specifying a geometry on the cmdline is *not* sufficient, because we test (ft.dsunit | ft.dswidth) which are not set by the cmdline options.

If we move the AG calculations to after we have set dsunit & dswdith, then we'll pick up either cmdline-specified or blkid-detected geometry, and go into "multidisk" mode for AG size/count calculations in both cases.

So now for a ~5T fs, for example, we'd make several more AGs:

# truncate --size=5t fsfile
# mkfs.xfs -N -d su=128k,sw=8 fsfile | grep agcount
meta-data=fsfile                 isize=256    agcount=5, agsize=268435424 blks
# mkfs/mkfs.xfs -N -d su=128k,sw=8 fsfile | grep agcount
meta-data=fsfile                 isize=256    agcount=32, agsize=41943008 blks

---

TBH, though, is there ever any value in treating a very large fs as "!multidisk?" When would we ever want to pretend that a 16T filesystem doesn't have a lot of spindles behind it?

mkfs/xfs_mkfs.c +19 -19
index be931e3..42a7b5b 100644
@@ -2090,25 +2091,6 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
 		nbmblocks = 0;
 	}
 
-	if (dasize) {		/* User-specified AG size */
-		/*
-		 * Check specified agsize is a multiple of blocksize.
-		 */
-		if (agsize % blocksize) {
-			fprintf(stderr,
-		_("agsize (%lld) not a multiple of fs blk size (%d)\n"),
-				(long long)agsize, blocksize);
-			usage();
-		}
-		agsize /= blocksize;
-		agcount = dblocks / agsize + (dblocks % agsize != 0);
-
-	} else if (daflag)	/* User-specified AG count */
-		agsize = dblocks / agcount + (dblocks % agcount != 0);
-	else
-		calc_default_ag_geometry(blocklog, dblocks,
-				ft.dsunit | ft.dswidth, &agsize, &agcount);
-
 	if (!nodsflag) {
 		if (dsunit) {
 			if (ft.dsunit && ft.dsunit != dsunit) {
@@ -2132,6 +2114,25 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
 		}
 	} /* else dsunit & dswidth can't be set if nodsflag is set */
 
+	if (dasize) {		/* User-specified AG size */
+		/*
+		 * Check specified agsize is a multiple of blocksize.
+		 */
+		if (agsize % blocksize) {
+			fprintf(stderr,
+		_("agsize (%lld) not a multiple of fs blk size (%d)\n"),
+				(long long)agsize, blocksize);
+			usage();
+		}
+		agsize /= blocksize;
+		agcount = dblocks / agsize + (dblocks % agsize != 0);
+
+	} else if (daflag)	/* User-specified AG count */
+		agsize = dblocks / agcount + (dblocks % agcount != 0);
+	else
+		calc_default_ag_geometry(blocklog, dblocks,
+				dsunit | dswidth, &agsize, &agcount);
+
 	/*
 	 * If dsunit is a multiple of fs blocksize, then check that is a
 	 * multiple of the agsize too

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw
Quoted 8 lines · Eric Sandeen

So now for a ~5T fs, for example, we'd make several more AGs:

# truncate --size=5t fsfile
# mkfs.xfs -N -d su=128k,sw=8 fsfile | grep agcount
meta-data=fsfile                 isize=256    agcount=5, agsize=268435424 blks
# mkfs/mkfs.xfs -N -d su=128k,sw=8 fsfile | grep agcount
meta-data=fsfile                 isize=256    agcount=32, agsize=41943008 blks

Can you add this to xfstests, please?

Quoted 3 lines

TBH, though, is there ever any value in treating a very large fs as "!multidisk?" When would we ever want to pretend that a 16T filesystem doesn't have a lot of spindles behind it?

Not now, but I'm pretty sure we'll get there not too soon in the future.

mkfs/xfs_mkfs.c _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"), · line 2135

Quoted 6 lines
mkfs/xfs_mkfs.c +6 -0
+	} else if (daflag)	/* User-specified AG count */
+		agsize = dblocks / agcount + (dblocks % agcount != 0);
+	else
+		calc_default_ag_geometry(blocklog, dblocks,
+				dsunit | dswidth, &agsize, &agcount);
+

I know the code was just moved, but can you add braces to the else if constructs so that the whole if else if else has consistent bracing?

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#2 lore raw

PATCH v2 1/1 mkfs.xfs: go into multidisk mode when geometry is on cmdline

In the course of some other investigations, I found that calc_default_ag_geometry() doesn't go into "multidisk" mode unless stripe geometry is *detected* (i.e. by the blkid routines).

Specifying a geometry on the cmdline is *not* sufficient, because we test (ft.dsunit | ft.dswidth) which are not set by the cmdline options.

If we move the AG calculations to after we have set dsunit & dswdith, then we'll pick up either cmdline-specified or blkid-detected geometry, and go into "multidisk" mode for AG size/count calculations in both cases.

So now for a ~5T fs, for example, we'd make several more AGs:

# truncate --size=5t fsfile
# mkfs.xfs -N -d su=128k,sw=8 fsfile | grep agcount
meta-data=fsfile                 isize=256    agcount=5, agsize=268435424 blks
# mkfs/mkfs.xfs -N -d su=128k,sw=8 fsfile | grep agcount
meta-data=fsfile                 isize=256    agcount=32, agsize=41943008 blks

---

V2: Add braces to last else clause per hch's request

mkfs/xfs_mkfs.c +20 -19
index be931e3..f320022 100644
@@ -2090,25 +2090,6 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
 		nbmblocks = 0;
 	}
 
-	if (dasize) {		/* User-specified AG size */
-		/*
-		 * Check specified agsize is a multiple of blocksize.
-		 */
-		if (agsize % blocksize) {
-			fprintf(stderr,
-		_("agsize (%lld) not a multiple of fs blk size (%d)\n"),
-				(long long)agsize, blocksize);
-			usage();
-		}
-		agsize /= blocksize;
-		agcount = dblocks / agsize + (dblocks % agsize != 0);
-
-	} else if (daflag)	/* User-specified AG count */
-		agsize = dblocks / agcount + (dblocks % agcount != 0);
-	else
-		calc_default_ag_geometry(blocklog, dblocks,
-				ft.dsunit | ft.dswidth, &agsize, &agcount);
-
 	if (!nodsflag) {
 		if (dsunit) {
 			if (ft.dsunit && ft.dsunit != dsunit) {
@@ -2132,6 +2113,26 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
 		}
 	} /* else dsunit & dswidth can't be set if nodsflag is set */
 
+	if (dasize) {		/* User-specified AG size */
+		/*
+		 * Check specified agsize is a multiple of blocksize.
+		 */
+		if (agsize % blocksize) {
+			fprintf(stderr,
+		_("agsize (%lld) not a multiple of fs blk size (%d)\n"),
+				(long long)agsize, blocksize);
+			usage();
+		}
+		agsize /= blocksize;
+		agcount = dblocks / agsize + (dblocks % agsize != 0);
+
+	} else if (daflag) {	/* User-specified AG count */
+		agsize = dblocks / agcount + (dblocks % agcount != 0);
+	else {
+		calc_default_ag_geometry(blocklog, dblocks,
+				dsunit | dswidth, &agsize, &agcount);
+	}
+
 	/*
 	 * If dsunit is a multiple of fs blocksize, then check that is a
 	 * multiple of the agsize too

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#3 lore raw

PATCH v3 1/1 mkfs.xfs: go into multidisk mode when geometry is on cmdline

In the course of some other investigations, I found that calc_default_ag_geometry() doesn't go into "multidisk" mode unless stripe geometry is *detected* (i.e. by the blkid routines).

Specifying a geometry on the cmdline is *not* sufficient, because we test (ft.dsunit | ft.dswidth) which are not set by the cmdline options.

If we move the AG calculations to after we have set dsunit & dswdith, then we'll pick up either cmdline-specified or blkid-detected geometry, and go into "multidisk" mode for AG size/count calculations in both cases.

So now for a ~5T fs, for example, we'd make several more AGs:

# truncate --size=5t fsfile
# mkfs.xfs -N -d su=128k,sw=8 fsfile | grep agcount
meta-data=fsfile                 isize=256    agcount=5, agsize=268435424 blks
# mkfs/mkfs.xfs -N -d su=128k,sw=8 fsfile | grep agcount
meta-data=fsfile                 isize=256    agcount=32, agsize=41943008 blks

---

V2: Add braces to last else clause per hch's request
V3: Add another brace so it compiles, ugh.

mkfs/xfs_mkfs.c +20 -19
index be931e3..577880b 100644
@@ -2090,25 +2090,6 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
 		nbmblocks = 0;
 	}
 
-	if (dasize) {		/* User-specified AG size */
-		/*
-		 * Check specified agsize is a multiple of blocksize.
-		 */
-		if (agsize % blocksize) {
-			fprintf(stderr,
-		_("agsize (%lld) not a multiple of fs blk size (%d)\n"),
-				(long long)agsize, blocksize);
-			usage();
-		}
-		agsize /= blocksize;
-		agcount = dblocks / agsize + (dblocks % agsize != 0);
-
-	} else if (daflag)	/* User-specified AG count */
-		agsize = dblocks / agcount + (dblocks % agcount != 0);
-	else
-		calc_default_ag_geometry(blocklog, dblocks,
-				ft.dsunit | ft.dswidth, &agsize, &agcount);
-
 	if (!nodsflag) {
 		if (dsunit) {
 			if (ft.dsunit && ft.dsunit != dsunit) {
@@ -2132,6 +2113,26 @@ _("size %s specified for log subvolume is too large, maximum is %lld blocks\n"),
 		}
 	} /* else dsunit & dswidth can't be set if nodsflag is set */
 
+	if (dasize) {		/* User-specified AG size */
+		/*
+		 * Check specified agsize is a multiple of blocksize.
+		 */
+		if (agsize % blocksize) {
+			fprintf(stderr,
+		_("agsize (%lld) not a multiple of fs blk size (%d)\n"),
+				(long long)agsize, blocksize);
+			usage();
+		}
+		agsize /= blocksize;
+		agcount = dblocks / agsize + (dblocks % agsize != 0);
+
+	} else if (daflag) {	/* User-specified AG count */
+		agsize = dblocks / agcount + (dblocks % agcount != 0);
+	} else {
+		calc_default_ag_geometry(blocklog, dblocks,
+				dsunit | dswidth, &agsize, &agcount);
+	}
+
 	/*
 	 * If dsunit is a multiple of fs blocksize, then check that is a
 	 * multiple of the agsize too

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#5 lore raw

PATCH 1/1 xfstests: Ensure cmdline geometry puts mkfs.xfs into multidisk mode

In reply to Christoph Hellwig

Ensure that when mkfs.xfs is invoked with commandline geometry, it triggers multidisk mode and creates more AGs.

---

Maybe this should also test device-specified geometry, but not sure how to do that easily within xfstests, any ideas?

291 new file +63 -0
new file mode 100755
index 0000000..045d2a0
@@ -0,0 +1,63 @@
+#! /bin/bash
+# FS QA Test No. 291
+#
+# Ensure mkfs with stripe geometry goes into multidisk mode
+# which results in more AGs
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2012 Red Hat, Inc.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+# creator
[email protected]
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs xfs
+_supported_os IRIX Linux
+
+fsfile=$TEST_DIR/fsfile.$seq
+
+rm -f $fsfile
+$XFS_IO_PROG -f -c "truncate 256g" $fsfile
+
+echo "mkfs.xfs without geometry"
+mkfs.xfs -f $fsfile 2>&1 | sed -e "s:$fsfile:FILENAME:g" | grep meta-data
+echo "mkfs.xfs with cmdline geometry"
+mkfs.xfs -f -d su=16k,sw=5 $fsfile 2>&1 | sed -e "s:$fsfile:FILENAME:g" | grep meta-data
+
+# success, all done
+status=0
+exit
291.out new file +5 -0
new file mode 100644
index 0000000..fd702c0
@@ -0,0 +1,5 @@
+QA output created by 291
+mkfs.xfs without geometry
+meta-data=FILENAME   isize=256    agcount=4, agsize=16777216 blks
+mkfs.xfs with cmdline geometry
+meta-data=FILENAME   isize=256    agcount=16, agsize=4194304 blks
group +1 -0
index dacdb56..2ddb047 100644
@@ -409,3 +409,4 @@ deprecated
 288 auto quick ioctl trim
 289 auto quick
 290 auto rw prealloc quick ioctl
+291 auto mkfs quick

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v1 1/1
#6 lore raw

Re: [PATCH] xfstests: Ensure cmdline geometry puts mkfs.xfs into multidisk mode

Quoted 4 lines · Eric Sandeen

Ensure that when mkfs.xfs is invoked with commandline geometry, it triggers multidisk mode and creates more AGs.

Looks good,

Quoted 2 lines

Maybe this should also test device-specified geometry, but not sure how to do that easily within xfstests, any ideas?

That would involved scsi_debug so that we could set them up. We had several ideas how to use scsi debug for these sort of tests but never really bothered to do it. If you have some spare cycles I'd love to see this sort of tests.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#7 lore raw

Re: [PATCH] xfstests: Ensure cmdline geometry puts mkfs.xfs into multidisk mode

Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Eric Sandeen

Ensure that when mkfs.xfs is invoked with commandline geometry, it triggers multidisk mode and creates more AGs.

Looks good,

Thanks

Quoted 7 lines
Quoted 2 lines

Maybe this should also test device-specified geometry, but not sure how to do that easily within xfstests, any ideas?

That would involved scsi_debug so that we could set them up. We had several ideas how to use scsi debug for these sort of tests but never really bothered to do it. If you have some spare cycles I'd love to see this sort of tests.

scsi_debug won't report geometry on its own today; we'd need to assemble something out of scsi_debug luns I'm afraid, so it gets even more involved.

We do have a little bit of scsi_debug framework in xfstests today, though, so if scsi_debug grew the capability it'd be easy to add.

-Eric

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#8 lore raw

Re: [PATCH] xfstests: Ensure cmdline geometry puts mkfs.xfs into multidisk mode

Quoted 21 lines · Eric Sandeen
Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Eric Sandeen

Ensure that when mkfs.xfs is invoked with commandline geometry, it triggers multidisk mode and creates more AGs.

Looks good,

Thanks

Quoted 7 lines
Quoted 2 lines

Maybe this should also test device-specified geometry, but not sure how to do that easily within xfstests, any ideas?

That would involved scsi_debug so that we could set them up. We had several ideas how to use scsi debug for these sort of tests but never really bothered to do it. If you have some spare cycles I'd love to see this sort of tests.

Quoted 5 lines

scsi_debug won't report geometry on its own today; we'd need to assemble something out of scsi_debug luns I'm afraid, so it gets even more involved.

We do have a little bit of scsi_debug framework in xfstests today, though, so if scsi_debug grew the capability it'd be easy to add.

Does it really need scsi for this? To mkfs.xfs, all devices look the same, so if you want to make random geometries, wouldn't playing games with llopback devices and DM be sufficient?

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#9 lore raw

Re: [PATCH] xfstests: Ensure cmdline geometry puts mkfs.xfs into multidisk mode

Quoted 25 lines · Dave Chinner
Quoted 24 lines · Eric Sandeen
Quoted 9 lines · Christoph Hellwig
Quoted 4 lines · Eric Sandeen

Ensure that when mkfs.xfs is invoked with commandline geometry, it triggers multidisk mode and creates more AGs.

Looks good,

Thanks

Quoted 7 lines
Quoted 2 lines

Maybe this should also test device-specified geometry, but not sure how to do that easily within xfstests, any ideas?

That would involved scsi_debug so that we could set them up. We had several ideas how to use scsi debug for these sort of tests but never really bothered to do it. If you have some spare cycles I'd love to see this sort of tests.

scsi_debug won't report geometry on its own today; we'd need to assemble something out of scsi_debug luns I'm afraid, so it gets even more involved.

Quoted 6 lines
Quoted 2 lines

We do have a little bit of scsi_debug framework in xfstests today, though, so if scsi_debug grew the capability it'd be easy to add.

Does it really need scsi for this? To mkfs.xfs, all devices look the same, so if you want to make random geometries, wouldn't playing games with llopback devices and DM be sufficient?

Yeah that'd be fine too. If scsi_debug could just generate geometries that'd be a bit easier. But it doesn't.

-Eric

Quoted 4 lines

Cheers,

Dave.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs