[PATCH, RFC] mkfs.xfs: go into multidisk mode when geometry is on cmdline

[PATCH, RFC] mkfs.xfs: go into multidisk mode when geometry is on cmdline

[Eric Sandeen]

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

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

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?

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index be931e3..42a7b5b 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH, RFC] mkfs.xfs: go into multidisk mode when geometry is on cmdline

[Christoph Hellwig]

On Tue, Nov 20, 2012 at 10:34:29AM -0600, Eric Sandeen wrote:
> 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?

> 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.

> +	} 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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH V2] mkfs.xfs: go into multidisk mode when geometry is on cmdline

[Eric Sandeen]

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

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

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

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index be931e3..f320022 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH V3] mkfs.xfs: go into multidisk mode when geometry is on cmdline

[Eric Sandeen]

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

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

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

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index be931e3..577880b 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH V3] mkfs.xfs: go into multidisk mode when geometry is on cmdline

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Eric Sandeen]

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

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

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

diff --git a/291 b/291
new file mode 100755
index 0000000..045d2a0
--- /dev/null
+++ b/291
@@ -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
+owner=sandeen@redhat.com
+
+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
diff --git a/291.out b/291.out
new file mode 100644
index 0000000..fd702c0
--- /dev/null
+++ b/291.out
@@ -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
diff --git a/group b/group
index dacdb56..2ddb047 100644
--- a/group
+++ b/group
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Christoph Hellwig]

On Thu, Dec 06, 2012 at 04:02:16PM -0600, Eric Sandeen wrote:
> Ensure that when mkfs.xfs is invoked with commandline geometry, it
> triggers multidisk mode and creates more AGs.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

> 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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Eric Sandeen]

On 12/8/12 6:00 AM, Christoph Hellwig wrote:
> On Thu, Dec 06, 2012 at 04:02:16PM -0600, Eric Sandeen wrote:
>> Ensure that when mkfs.xfs is invoked with commandline geometry, it
>> triggers multidisk mode and creates more AGs.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> 
> Looks good,
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks

>> 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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Dave Chinner]

On Sat, Dec 08, 2012 at 10:27:47AM -0600, Eric Sandeen wrote:
> On 12/8/12 6:00 AM, Christoph Hellwig wrote:
> > On Thu, Dec 06, 2012 at 04:02:16PM -0600, Eric Sandeen wrote:
> >> Ensure that when mkfs.xfs is invoked with commandline geometry, it
> >> triggers multidisk mode and creates more AGs.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > 
> > Looks good,
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Thanks
> 
> >> 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.

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.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Eric Sandeen]

On 12/9/12 5:51 PM, Dave Chinner wrote:
> On Sat, Dec 08, 2012 at 10:27:47AM -0600, Eric Sandeen wrote:
>> On 12/8/12 6:00 AM, Christoph Hellwig wrote:
>>> On Thu, Dec 06, 2012 at 04:02:16PM -0600, Eric Sandeen wrote:
>>>> Ensure that when mkfs.xfs is invoked with commandline geometry, it
>>>> triggers multidisk mode and creates more AGs.
>>>>
>>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>>
>>> Looks good,
>>>
>>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>>
>> Thanks
>>
>>>> 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.
> 
> 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

> Cheers,
> 
> Dave.
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs