[Cluster-devel] [PATCH 0/5] mkfs.gfs2 device topology-related fixes

[Cluster-devel] [PATCH 0/5] mkfs.gfs2 device topology-related fixes

[Andrew Price]

This patch set improves handling of various combinations of device topology
values/io limits. It addresses two main problems: misaligned devices and
devices which report strange optimal/minumum values which can not be used for
resource group alignment. Some tests which failed before this patch set are
added in patch 5.

Andrew Price (5):
  mkfs.gfs2: Add an extended option for device topology testing
  mkfs.gfs2: Warn when device is misaligned
  mkfs.gfs2: Disable rgrp alignment when dev topology is unsuitable
  mkfs.gfs2: Disregard device topology if zero-sized sectors reported
  gfs2-utils tests: Add tests for device topology handling in mkfs.gfs2

 gfs2/mkfs/main_mkfs.c | 65 ++++++++++++++++++++++++++++++++++++++++++---------
 tests/mkfs.at         |  8 +++++++
 2 files changed, 62 insertions(+), 11 deletions(-)

-- 
2.9.3

[Cluster-devel] [PATCH 1/5] mkfs.gfs2: Add an extended option for device topology testing

[Andrew Price]

This adds an -o test_topology=v1:v2:v3:v4:v5 extended option which can
be used to test how mkfs.gfs2 behaves with different device
characteristics. The values it accepts correspond to:

1. alignment_offset
2. logical_sector_size
3. minimum_io_size
4. optimal_io_size
5. physical_sector_size

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 363878c..a2d2ba9 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -256,6 +256,33 @@ static unsigned parse_bool(struct mkfs_opts *opts, const char *key, const char *
 	exit(-1);
 }
 
+static int parse_topology(struct mkfs_opts *opts, char *str)
+{
+	char *opt;
+	unsigned i = 0;
+	unsigned long *topol[5] = {
+		&opts->dev.alignment_offset,
+		&opts->dev.logical_sector_size,
+		&opts->dev.minimum_io_size,
+		&opts->dev.optimal_io_size,
+		&opts->dev.physical_sector_size
+	};
+
+	while ((opt = strsep(&str, ":")) != NULL) {
+		if (i > 4) {
+			fprintf(stderr, "Too many topology values.\n");
+			return 1;
+		}
+		*topol[i] = parse_ulong(opts, "test_topology", opt);
+		i++;
+	}
+	if (i < 5) {
+		fprintf(stderr, "Too few topology values.\n");
+		return 1;
+	}
+	return 0;
+}
+
 static void opt_parse_extended(char *str, struct mkfs_opts *opts)
 {
 	char *opt;
@@ -274,6 +301,10 @@ static void opt_parse_extended(char *str, struct mkfs_opts *opts)
 			opts->got_swidth = 1;
 		} else if (strcmp("align", key) == 0) {
 			opts->align = parse_bool(opts, "align", val);
+		} else if (strcmp("test_topology", key) == 0) {
+			if (parse_topology(opts, val) != 0)
+				exit(-1);
+			opts->dev.got_topol = 1;
 		} else if (strcmp("help", key) == 0) {
 			print_ext_opts();
 			exit(0);
@@ -854,7 +885,7 @@ static int probe_contents(struct mkfs_dev *dev)
 	return 0;
 }
 
-static void open_dev(struct mkfs_dev *dev)
+static void open_dev(struct mkfs_dev *dev, int withprobe)
 {
 	int error;
 
@@ -882,9 +913,7 @@ static void open_dev(struct mkfs_dev *dev)
 		fprintf(stderr, _("'%s' is not a block device or regular file\n"), dev->path);
 		exit(1);
 	}
-
-	error = probe_contents(dev);
-	if (error)
+	if (withprobe && (probe_contents(dev) != 0))
 		exit(1);
 }
 
@@ -905,7 +934,7 @@ int main(int argc, char *argv[])
 	opts_get(argc, argv, &opts);
 	opts_check(&opts);
 
-	open_dev(&opts.dev);
+	open_dev(&opts.dev, !opts.dev.got_topol);
 	bsize = choose_blocksize(&opts);
 
 	if (S_ISREG(opts.dev.stat.st_mode)) {
-- 
2.9.3

[Cluster-devel] [PATCH 2/5] mkfs.gfs2: Warn when device is misaligned

[Andrew Price]

Normally one layer of the I/O stack will adjust for a non-zero
alignment_offset value and expose it to upper layers as zero. If a
misalignment is not accounted for, mkfs.gfs2 will see a non-zero
alignment_offset and should report the misalignment to the user as it
could harm performance.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index a2d2ba9..db83d1c 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -495,7 +495,11 @@ static unsigned choose_blocksize(struct mkfs_opts *opts)
 		printf("optimal_io_size: %lu\n", dev->optimal_io_size);
 		printf("physical_sector_size: %lu\n", dev->physical_sector_size);
 	}
-
+	if (dev->got_topol && dev->alignment_offset != 0) {
+		fprintf(stderr,
+		  _("Warning: device is not properly aligned. This may harm performance.\n"));
+		dev->physical_sector_size = dev->logical_sector_size;
+	}
 	if (!opts->got_bsize && dev->got_topol) {
 		if (dev->optimal_io_size <= getpagesize() &&
 		    dev->optimal_io_size >= dev->minimum_io_size)
-- 
2.9.3

[Cluster-devel] [PATCH 3/5] mkfs.gfs2: Disable rgrp alignment when dev topology is unsuitable

[Andrew Price]

If optimal_io_size is not a multiple of minimum_io_size then the values
are not reliable swidth and sunit values, so disable rgrp stripe
alignment in that case.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index db83d1c..e2c4a22 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -629,8 +629,14 @@ static lgfs2_rgrps_t rgs_init(struct mkfs_opts *opts, struct gfs2_sbd *sdp)
 			al_off = opts->sunit / sdp->bsize;
 		}
 	} else if (opts->align) {
-		if ((opts->dev.minimum_io_size > opts->dev.physical_sector_size) &&
-		    (opts->dev.optimal_io_size > opts->dev.physical_sector_size)) {
+		if (opts->dev.optimal_io_size <= opts->dev.physical_sector_size ||
+		    opts->dev.minimum_io_size <= opts->dev.physical_sector_size ||
+		    (opts->dev.optimal_io_size % opts->dev.minimum_io_size) != 0) {
+			/* If optimal_io_size is not a multiple of minimum_io_size then
+			   the values are not reliable swidth and sunit values, so disable
+			   rgrp alignment */
+			opts->align = 0;
+		} else {
 			al_base = opts->dev.optimal_io_size / sdp->bsize;
 			al_off = opts->dev.minimum_io_size / sdp->bsize;
 		}
-- 
2.9.3

[Cluster-devel] [PATCH 4/5] mkfs.gfs2: Disregard device topology if zero-sized sectors reported

[Andrew Price]

Guard against physical_sector_size or logical_sector_size being zero.
Normally this isn't a problem as the kernel will report these values as
the device sector size but the checks are useful for when zero values
are passed to mkfs.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/mkfs/main_mkfs.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index e2c4a22..0801a4b 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -136,6 +136,7 @@ struct mkfs_opts {
 	unsigned got_lockproto:1;
 	unsigned got_locktable:1;
 	unsigned got_device:1;
+	unsigned got_topol:1;
 
 	unsigned override:1;
 	unsigned quiet:1;
@@ -304,7 +305,8 @@ static void opt_parse_extended(char *str, struct mkfs_opts *opts)
 		} else if (strcmp("test_topology", key) == 0) {
 			if (parse_topology(opts, val) != 0)
 				exit(-1);
-			opts->dev.got_topol = 1;
+			opts->got_topol = (opts->dev.logical_sector_size != 0 &&
+	                                   opts->dev.physical_sector_size != 0);
 		} else if (strcmp("help", key) == 0) {
 			print_ext_opts();
 			exit(0);
@@ -487,20 +489,21 @@ static unsigned choose_blocksize(struct mkfs_opts *opts)
 	unsigned int x;
 	unsigned int bsize = opts->bsize;
 	struct mkfs_dev *dev = &opts->dev;
+	int got_topol = (dev->got_topol || opts->got_topol);
 
-	if (dev->got_topol && opts->debug) {
+	if (got_topol && opts->debug) {
 		printf("alignment_offset: %lu\n", dev->alignment_offset);
 		printf("logical_sector_size: %lu\n", dev->logical_sector_size);
 		printf("minimum_io_size: %lu\n", dev->minimum_io_size);
 		printf("optimal_io_size: %lu\n", dev->optimal_io_size);
 		printf("physical_sector_size: %lu\n", dev->physical_sector_size);
 	}
-	if (dev->got_topol && dev->alignment_offset != 0) {
+	if (got_topol && dev->alignment_offset != 0) {
 		fprintf(stderr,
 		  _("Warning: device is not properly aligned. This may harm performance.\n"));
 		dev->physical_sector_size = dev->logical_sector_size;
 	}
-	if (!opts->got_bsize && dev->got_topol) {
+	if (!opts->got_bsize && got_topol) {
 		if (dev->optimal_io_size <= getpagesize() &&
 		    dev->optimal_io_size >= dev->minimum_io_size)
 			bsize = dev->optimal_io_size;
@@ -887,7 +890,8 @@ static int probe_contents(struct mkfs_dev *dev)
 			dev->minimum_io_size = blkid_topology_get_minimum_io_size(tp);
 			dev->optimal_io_size = blkid_topology_get_optimal_io_size(tp);
 			dev->physical_sector_size = blkid_topology_get_physical_sector_size(tp);
-			dev->got_topol = 1;
+			dev->got_topol = (dev->logical_sector_size != 0 &&
+	                                  dev->physical_sector_size != 0);
 		}
 	}
 
@@ -944,7 +948,7 @@ int main(int argc, char *argv[])
 	opts_get(argc, argv, &opts);
 	opts_check(&opts);
 
-	open_dev(&opts.dev, !opts.dev.got_topol);
+	open_dev(&opts.dev, !opts.got_topol);
 	bsize = choose_blocksize(&opts);
 
 	if (S_ISREG(opts.dev.stat.st_mode)) {
-- 
2.9.3

[Cluster-devel] [PATCH 5/5] gfs2-utils tests: Add tests for device topology handling in mkfs.gfs2

[Andrew Price]

Add some tests that exercise mkfs.gfs2 against various device i/o limits
based on odd values seen in the wild.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 tests/mkfs.at | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/mkfs.at b/tests/mkfs.at
index d3521c8..c026a76 100644
--- a/tests/mkfs.at
+++ b/tests/mkfs.at
@@ -101,3 +101,11 @@ AT_CHECK([$GFS_MKFS -p lock_dlm -t "quite_long_cluster_name_test_here:intec34p"
 AT_CHECK([$GFS_MKFS -p lock_dlm -t "financial_cluster:this_time_we_test_fs_naming_len" $GFS_TGT], 255, [ignore], [ignore])
 GFS_FSCK_CHECK([$GFS_MKFS -p lock_dlm -t "a_really_long_named_cluster_here:concurrently_lets_check_fs_len" $GFS_TGT])
 AT_CLEANUP
+
+AT_SETUP([Device i/o limits handling])
+AT_KEYWORDS(mkfs.gfs2 mkfs)
+AT_CHECK([$GFS_MKFS -p lock_nolock -o test_topology=0:0:0:0:0 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -o test_topology=7168:512:0:33553920:512 $GFS_TGT], 0, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_nolock -o test_topology=7168:512:8192:33553920:512 $GFS_TGT], 0, [ignore], [Warning: device is not properly aligned. This may harm performance.
+])
+AT_CLEANUP
-- 
2.9.3

[Cluster-devel] [PATCH 0/5] mkfs.gfs2 device topology-related fixes

[Bob Peterson]

----- Original Message -----
| This patch set improves handling of various combinations of device topology
| values/io limits. It addresses two main problems: misaligned devices and
| devices which report strange optimal/minumum values which can not be used for
| resource group alignment. Some tests which failed before this patch set are
| added in patch 5.
| 
| Andrew Price (5):
|   mkfs.gfs2: Add an extended option for device topology testing
|   mkfs.gfs2: Warn when device is misaligned
|   mkfs.gfs2: Disable rgrp alignment when dev topology is unsuitable
|   mkfs.gfs2: Disregard device topology if zero-sized sectors reported
|   gfs2-utils tests: Add tests for device topology handling in mkfs.gfs2
| 
|  gfs2/mkfs/main_mkfs.c | 65
|  ++++++++++++++++++++++++++++++++++++++++++---------
|  tests/mkfs.at         |  8 +++++++
|  2 files changed, 62 insertions(+), 11 deletions(-)
| 
| --
| 2.9.3
| 
Hi,

Series ACK

Regards,

Bob Peterson
Red Hat File Systems