xfsprogs: ignore stripe geom if sunit or swidth == physical sector size

Today, users doing a bare mkfs on storage with a large default stripe size may be surprised to get this warning:

log stripe unit (%d bytes) is too large (maximum is 256KiB log stripe unit adjusted to 32KiB

through no fault of their own. The fallback is appropriate and harmless, and there's no need to warn about this in the defaults case.

However, we keep the warning if a large log stripe unit was specified by the user on the commandline.

Signed-off-by Eric Sandeen <[email protected]>

---

mkfs/xfs_mkfs.c +5 -1

index a0fed31..66711cb 100644
@@ -946,6 +946,8 @@ main(
 	int			logversion;
 	int			lvflag;
 	int			lsflag;
+	int			lsuflag;
+	int			lsunitflag;
 	int			lsectorlog;
 	int			lsectorsize;
 	int			lslflag;

@@ -1004,7 +1006,7 @@ main(
 	sectorsize = lsectorsize = XFS_MIN_SECTORSIZE;
 	agsize = daflag = dasize = dblocks = 0;
 	ilflag = imflag = ipflag = isflag = 0;
-	liflag = laflag = lsflag = ldflag = lvflag = 0;
+	liflag = laflag = lsflag = lsuflag = lsunitflag = ldflag = lvflag = 0;
 	loginternal = 1;
 	logversion = 2;
 	logagno = logblocks = rtblocks = rtextblocks = 0;

@@ -1400,6 +1402,7 @@ main(
 						respec('l', lopts, L_SU);
 					lsu = cvtnum(
 						blocksize, sectorsize, value);
+					lsuflag = 1;
 					break;
 				case L_SUNIT:
 					if (!value || *value == '\0')

@@ -1412,6 +1415,7 @@ main(
 						usage();
 					}
 					lsunit = cvtnum(0, 0, value);
+					lsunitflag = 1;
 					break;
 				case L_NAME:
 				case L_DEV:

@@ -2379,11 +2383,15 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
 	}
 
 	if (logversion == 2 && (lsunit * blocksize) > 256 * 1024) {

mkfs/xfs_mkfs.c +0 -1

 		lsunit = (32 * 1024) >> blocklog;
-		fprintf(stderr, _("log stripe unit adjusted to 32KiB\n"));
 	}
 
 	min_logblocks = max_trans_res(crcs_enabled, dirversion,

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

Quoted 15 lines · Eric Sandeen

Today, users doing a bare mkfs on storage with a large default stripe size may be surprised to get this warning:

log stripe unit (%d bytes) is too large (maximum is 256KiB log stripe unit adjusted to 32KiB

through no fault of their own. The fallback is appropriate and harmless, and there's no need to warn about this in the defaults case.

However, we keep the warning if a large log stripe unit was specified by the user on the commandline.

Signed-off-by Eric Sandeen <[email protected]>

---

Quoted 4 lines

mkfs/xfs_mkfs.c +0 -0

@@ -2379,11 +2383,15 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
 	}
 
 	if (logversion == 2 && (lsunit * blocksize) > 256 * 1024) {

mkfs/xfs_mkfs.c an AG size that is one stripe unit smaller, for example %llu.\n"), · line 2393

Quoted 10 lines

mkfs/xfs_mkfs.c +7 -2

-		fprintf(stderr,
+		/* Warn only if specified on commandline */
+		if (lsuflag || lsunitflag) {
+			fprintf(stderr,
 	_("log stripe unit (%d bytes) is too large (maximum is 256KiB)\n"),
-			(lsunit * blocksize));
+				(lsunit * blocksize));
+			fprintf(stderr,
+	_("log stripe unit adjusted to 32KiB\n"));
+		}

Right above this particular hunk we have the possibility of lsunit inheriting a value from dsunit. If the latter is specified by the user, we don't print the message for an arguably user-specified lsunit. Hmm, do we care about that case?

Quoted 16 lines · Brian Foster

Quoted 15 lines · Eric Sandeen

Today, users doing a bare mkfs on storage with a large default stripe size may be surprised to get this warning:

log stripe unit (%d bytes) is too large (maximum is 256KiB log stripe unit adjusted to 32KiB

through no fault of their own. The fallback is appropriate and harmless, and there's no need to warn about this in the defaults case.

However, we keep the warning if a large log stripe unit was specified by the user on the commandline.

Signed-off-by Eric Sandeen <[email protected]>

---

...

Quoted 19 lines

Quoted 4 lines

mkfs/xfs_mkfs.c +0 -0

@@ -2379,11 +2383,15 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
 	}
 
 	if (logversion == 2 && (lsunit * blocksize) > 256 * 1024) {

mkfs/xfs_mkfs.c an AG size that is one stripe unit smaller, for example %llu.\n"), · line 2393

Quoted 10 lines

mkfs/xfs_mkfs.c +7 -2

-		fprintf(stderr,
+		/* Warn only if specified on commandline */
+		if (lsuflag || lsunitflag) {
+			fprintf(stderr,
 	_("log stripe unit (%d bytes) is too large (maximum is 256KiB)\n"),
-			(lsunit * blocksize));
+				(lsunit * blocksize));
+			fprintf(stderr,
+	_("log stripe unit adjusted to 32KiB\n"));
+		}

Right above this particular hunk we have the possibility of lsunit inheriting a value from dsunit. If the latter is specified by the user, we don't print the message for an arguably user-specified lsunit. Hmm, do we care about that case?

I don't think we do care. I think we only care if the user manually specified a log stripe unit and we're changing what they asked for.

I'd put "inheritance" in the "using default behaviors" case, and not warn about it.

-Eric

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

Today, this geometry:

# modprobe scsi_debug  opt_blks=2048 dev_size_mb=2048
# blockdev --getpbsz --getss --getiomin --getioopt  /dev/sdd
512
512
512
1048576

will result in a warning at mkfs time, like this:

# mkfs.xfs -f -d su=64k,sw=12 -l su=64k /dev/sdd
mkfs.xfs: Specified data stripe width 1536 is not the same as the volume stripe width 2048

because our geometry discovery thinks it looks like a valid striping setup which the commandline is overriding. However, a stripe unit of 512 really isn't indicative of a proper stripe geometry.

Prior to this patch, we reset only sunit *or* swidth, if either was equal to physical block size, but not necessarily both.

Change the heuristic so that if either the discovered sunit or the discovered swidth is physical block size, we reset *both* to zero and ignore the geom completely.

While we're at it, don't pass &dummy in for multiple arguments to blkid_get_topology(); that'll mean that inside the function, the last assignment wins, and could lead to unexpected results.

Reported-by Stan Hoeppner <[email protected]>

Signed-off-by Eric Sandeen <[email protected]>

---

mkfs/xfs_mkfs.c +6 -10

index 4546e35..a0fed31 100644
@@ -410,21 +410,27 @@ static void blkid_get_topology(
 	*lsectorsize = val;
 	val = blkid_topology_get_physical_sector_size(tp);
 	*psectorsize = val;
+	val = blkid_topology_get_minimum_io_size(tp);
+	*sunit = val;
+	val = blkid_topology_get_optimal_io_size(tp);
+	*swidth = val;
 
 	/*
-	 * Blkid reports the information in terms of bytes, but we want it in
-	 * terms of 512 bytes blocks (just to convert it to bytes later..)
-	 *
 	 * If the reported values are the same as the physical sector size
-	 * do not bother to report anything.  It will just cause warnings
+	 * do not bother to report anything.  It will only cause warnings
 	 * if people specify larger stripe units or widths manually.
 	 */
-	val = blkid_topology_get_minimum_io_size(tp);
-	if (val > *psectorsize)
-		*sunit = val >> 9;
-	val = blkid_topology_get_optimal_io_size(tp);
-	if (val > *psectorsize)
-		*swidth = val >> 9;
+	if (*sunit == *psectorsize || *swidth == *psectorsize) {

mkfs/xfs_mkfs.c +3 -3

 	if (blkid_topology_get_alignment_offset(tp) != 0) {
 		fprintf(stderr,

@@ -484,10 +490,10 @@ static void get_topology(
 	}
 
 	if (xi->rtname && !xi->risfile) {
-		int dummy;
+		int sunit, lsectorsize, psectorsize;
 
-		blkid_get_topology(xi->rtname, &dummy, &ft->rtswidth,
-				   &dummy, &dummy, force_overwrite);
+		blkid_get_topology(xi->rtname, &sunit, &ft->rtswidth,
+				   &lsectorsize, &psectorsize, force_overwrite);
 	}
 }
 #else /* ENABLE_BLKID */

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

Quoted 11 lines · Eric Sandeen

Today, this geometry:

# modprobe scsi_debug  opt_blks=2048 dev_size_mb=2048
# blockdev --getpbsz --getss --getiomin --getioopt  /dev/sdd
512
512
512
1048576

will result in a warning at mkfs time, like this:

Quoted 8 lines

# mkfs.xfs -f -d su=64k,sw=12 -l su=64k /dev/sdd
mkfs.xfs: Specified data stripe width 1536 is not the same as the volume stripe width 2048

because our geometry discovery thinks it looks like a valid striping setup which the commandline is overriding. However, a stripe unit of 512 really isn't indicative of a proper stripe geometry.

So the assumption is that the storage reports a non-physical block size for minimum and optimal I/O sizes for geometry detection. There was a real world scenario of this, right? Any idea of the configuration details (e.g., raid layout) that resulted in an increased optimal I/O size but not minimum I/O size?

This seems reasonable to me and the code looks fine, save a trailing whitespace instance below.

I'm just curious if there are any weird
corner cases where there's value in the reported optimal I/O size or the real world situation was just noise.

Quoted 21 lines · Brian Foster

Quoted 19 lines · Eric Sandeen

Today, this geometry:

# modprobe scsi_debug  opt_blks=2048 dev_size_mb=2048
# blockdev --getpbsz --getss --getiomin --getioopt  /dev/sdd
512
512
512
1048576

will result in a warning at mkfs time, like this:

# mkfs.xfs -f -d su=64k,sw=12 -l su=64k /dev/sdd
mkfs.xfs: Specified data stripe width 1536 is not the same as the volume stripe width 2048

because our geometry discovery thinks it looks like a valid striping setup which the commandline is overriding. However, a stripe unit of 512 really isn't indicative of a proper stripe geometry.

Quoted 5 lines

So the assumption is that the storage reports a non-physical block size for minimum and optimal I/O sizes for geometry detection. There was a real world scenario of this, right? Any idea of the configuration details (e.g., raid layout) that resulted in an increased optimal I/O size but not minimum I/O size?

Stan? :)

Quoted 28 lines · Eric Sandeen

Quoted 26 lines · Brian Foster

Quoted 11 lines · Eric Sandeen

Today, this geometry:

# modprobe scsi_debug  opt_blks=2048 dev_size_mb=2048
# blockdev --getpbsz --getss --getiomin --getioopt  /dev/sdd
512
512
512
1048576

will result in a warning at mkfs time, like this:

Quoted 8 lines

# mkfs.xfs -f -d su=64k,sw=12 -l su=64k /dev/sdd
mkfs.xfs: Specified data stripe width 1536 is not the same as the volume stripe width 2048

because our geometry discovery thinks it looks like a valid striping setup which the commandline is overriding. However, a stripe unit of 512 really isn't indicative of a proper stripe geometry.

So the assumption is that the storage reports a non-physical block size for minimum and optimal I/O sizes for geometry detection. There was a real world scenario of this, right? Any idea of the configuration details (e.g., raid layout) that resulted in an increased optimal I/O size but not minimum I/O size?

Quoted 1 line

Stan? :)

Yeah, it was pretty much what you pasted sans the log su, and it was a device-mapper device:

# mkfs.xfs -d su=64k,sw=12 /dev/dm-0

--
Stan

Quoted 31 lines · Stan Hoeppner

Quoted 29 lines · Eric Sandeen

Quoted 21 lines · Brian Foster

Quoted 11 lines · Eric Sandeen

Today, this geometry:

# modprobe scsi_debug  opt_blks=2048 dev_size_mb=2048
# blockdev --getpbsz --getss --getiomin --getioopt  /dev/sdd
512
512
512
1048576

will result in a warning at mkfs time, like this:

Quoted 8 lines

# mkfs.xfs -f -d su=64k,sw=12 -l su=64k /dev/sdd
mkfs.xfs: Specified data stripe width 1536 is not the same as the volume stripe width 2048

because our geometry discovery thinks it looks like a valid striping setup which the commandline is overriding. However, a stripe unit of 512 really isn't indicative of a proper stripe geometry.

Quoted 5 lines

So the assumption is that the storage reports a non-physical block size for minimum and optimal I/O sizes for geometry detection. There was a real world scenario of this, right? Any idea of the configuration details (e.g., raid layout) that resulted in an increased optimal I/O size but not minimum I/O size?

Stan? :)

Quoted 5 lines

Yeah, it was pretty much what you pasted sans the log su, and it was a device-mapper device:

# mkfs.xfs -d su=64k,sw=12 /dev/dm-0

What kind of device is dm-0? I use linear devices regularly and I don't see any special optimal I/O size reported:

Quoted 35 lines · Brian Foster

Quoted 34 lines · Stan Hoeppner

Quoted 22 lines · Eric Sandeen

Quoted 21 lines · Brian Foster

Quoted 19 lines · Eric Sandeen

Today, this geometry:

# modprobe scsi_debug  opt_blks=2048 dev_size_mb=2048
# blockdev --getpbsz --getss --getiomin --getioopt  /dev/sdd
512
512
512
1048576

will result in a warning at mkfs time, like this:

# mkfs.xfs -f -d su=64k,sw=12 -l su=64k /dev/sdd
mkfs.xfs: Specified data stripe width 1536 is not the same as the volume stripe width 2048

because our geometry discovery thinks it looks like a valid striping setup which the commandline is overriding. However, a stripe unit of 512 really isn't indicative of a proper stripe geometry.

Quoted 7 lines

Quoted 5 lines

So the assumption is that the storage reports a non-physical block size for minimum and optimal I/O sizes for geometry detection. There was a real world scenario of this, right? Any idea of the configuration details (e.g., raid layout) that resulted in an increased optimal I/O size but not minimum I/O size?

Stan? :)

Yeah, it was pretty much what you pasted sans the log su, and it was a device-mapper device:

Quoted 5 lines

Quoted 2 lines

# mkfs.xfs -d su=64k,sw=12 /dev/dm-0

What kind of device is dm-0? I use linear devices regularly and I don't see any special optimal I/O size reported:

It's a dm-multipath device. I pasted details up thread. Here, again:

# multipath -ll
3600c0ff0003630917954075401000000 dm-0 Tek,DH6554
size=44T features='0' hwhandler='0' wp=rw
|-+- policy='round-robin 0' prio=50 status=active
| `- 9:0:0:3 sdj 8:144 active ready running
`-+- policy='round-robin 0' prio=10 status=enabled
  `- 1:0:0:3 sdf 8:80  active ready running

# blockdev --getpbsz --getss --getiomin --getioopt  /dev/dm-0
512
512
512
1048576

# blockdev --getpbsz --getss --getiomin --getioopt  /dev/sdj
512
512
512
1048576

# blockdev --getpbsz --getss --getiomin --getioopt  /dev/sdf
512
512
512
1048576

Cheers,
Stan

Quoted 36 lines · Stan Hoeppner

Quoted 35 lines · Brian Foster

Quoted 34 lines · Stan Hoeppner

Quoted 22 lines · Eric Sandeen

Quoted 21 lines · Brian Foster

Quoted 19 lines · Eric Sandeen

Today, this geometry:

# modprobe scsi_debug  opt_blks=2048 dev_size_mb=2048
# blockdev --getpbsz --getss --getiomin --getioopt  /dev/sdd
512
512
512
1048576

will result in a warning at mkfs time, like this:

# mkfs.xfs -f -d su=64k,sw=12 -l su=64k /dev/sdd
mkfs.xfs: Specified data stripe width 1536 is not the same as the volume stripe width 2048

because our geometry discovery thinks it looks like a valid striping setup which the commandline is overriding. However, a stripe unit of 512 really isn't indicative of a proper stripe geometry.

Quoted 7 lines

Quoted 5 lines

So the assumption is that the storage reports a non-physical block size for minimum and optimal I/O sizes for geometry detection. There was a real world scenario of this, right? Any idea of the configuration details (e.g., raid layout) that resulted in an increased optimal I/O size but not minimum I/O size?

Stan? :)

Yeah, it was pretty much what you pasted sans the log su, and it was a device-mapper device:

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

# mkfs.xfs -d su=64k,sw=12 /dev/dm-0

What kind of device is dm-0? I use linear devices regularly and I don't see any special optimal I/O size reported:

It's a dm-multipath device. I pasted details up thread. Here, again:

Oh, I see. So this is just getting passed up from the lower level scsi devices. On a quick look, this data appears to come from the device via the "block limits VPD." Apparently that should be accessible via something like this (0xb0 from sd_read_block_limits()):

# sg_inq --page=0xb0 /dev/sdx

... but I don't have a device around that likes that command. It would be interesting to know what makes the underlying device set optimal I/O size as such, but that's just curiosity at this point. :)

Brian

Quoted 29 lines

# multipath -ll
3600c0ff0003630917954075401000000 dm-0 Tek,DH6554
size=44T features='0' hwhandler='0' wp=rw
|-+- policy='round-robin 0' prio=50 status=active
| `- 9:0:0:3 sdj 8:144 active ready running
`-+- policy='round-robin 0' prio=10 status=enabled
  `- 1:0:0:3 sdf 8:80  active ready running

# blockdev --getpbsz --getss --getiomin --getioopt  /dev/dm-0
512
512
512
1048576

# blockdev --getpbsz --getss --getiomin --getioopt  /dev/sdj
512
512
512
1048576

# blockdev --getpbsz --getss --getiomin --getioopt  /dev/sdf
512
512
512
1048576

Quoted 3 lines

Cheers,
Stan

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