linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 30/42] mkfs: rework stripe calculations
Date: Wed, 30 Aug 2017 09:50:40 +1000	[thread overview]
Message-ID: <20170829235052.21050-31-david@fromorbit.com> (raw)
In-Reply-To: <20170829235052.21050-1-david@fromorbit.com>

From: Dave Chinner <dchinner@redhat.com>

The data and log stripe calculations a spaghettied all over the mkfs
code. This patch pulls all of the different chunks of code together
into calc_stripe_factors() and removes all the redundant/repeated
checks and calculations that are made.

Signed-Off-By: Dave Chinner <dchinner@redhat.com>
---
 mkfs/xfs_mkfs.c | 330 ++++++++++++++++++++++++++++++--------------------------
 1 file changed, 180 insertions(+), 150 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 7595e378ad44..fe380d2b388d 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -896,74 +896,6 @@ struct mkfs_default_params {
  */
 #define WHACK_SIZE (128 * 1024)
 
-/*
- * Convert lsu to lsunit for 512 bytes blocks and check validity of the values.
- */
-static void
-calc_stripe_factors(
-	int		dsu,
-	int		dsw,
-	int		dsectsz,
-	int		lsu,
-	int		lsectsz,
-	int		*dsunit,
-	int		*dswidth,
-	int		*lsunit)
-{
-	/* Handle data sunit/swidth options */
-	if ((*dsunit && !*dswidth) || (!*dsunit && *dswidth)) {
-		fprintf(stderr,
-			_("both data sunit and data swidth options "
-			"must be specified\n"));
-		usage();
-	}
-
-	if (dsu || dsw) {
-		if ((dsu && !dsw) || (!dsu && dsw)) {
-			fprintf(stderr,
-				_("both data su and data sw options "
-				"must be specified\n"));
-			usage();
-		}
-
-		if (dsu % dsectsz) {
-			fprintf(stderr,
-				_("data su must be a multiple of the "
-				"sector size (%d)\n"), dsectsz);
-			usage();
-		}
-
-		*dsunit  = (int)BTOBBT(dsu);
-		*dswidth = *dsunit * dsw;
-	}
-
-	if (*dsunit && (*dswidth % *dsunit != 0)) {
-		fprintf(stderr,
-			_("data stripe width (%d) must be a multiple of the "
-			"data stripe unit (%d)\n"), *dswidth, *dsunit);
-		usage();
-	}
-
-	/* Handle log sunit options */
-
-	if (lsu)
-		*lsunit = (int)BTOBBT(lsu);
-
-	/* verify if lsu/lsunit is a multiple block size */
-	if (lsu % blocksize != 0) {
-		fprintf(stderr,
-_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
-		lsu, blocksize);
-		exit(1);
-	}
-	if ((BBTOB(*lsunit) % blocksize != 0)) {
-		fprintf(stderr,
-_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
-		BBTOB(*lsunit), blocksize);
-		exit(1);
-	}
-}
-
 static void
 check_device_type(
 	const char	*name,
@@ -2377,6 +2309,178 @@ validate_rtextsize(
 	ASSERT(cfg->rtextblocks);
 }
 
+/*
+ * Validate the configured stripe geometry, or is none is specified, pull
+ * the configuration from the underlying device.
+ *
+ * CLI parameters come in as different units, go out as filesystem blocks.
+ */
+static void
+calc_stripe_factors(
+	struct mkfs_params	*cfg,
+	struct cli_params	*cli,
+	struct fs_topology	*ft)
+{
+	int		dsunit = 0;
+	int		dswidth = 0;
+	int		lsunit = 0;
+	int		dsu = 0;
+	int		dsw = 0;
+	int		lsu = 0;
+	bool		use_dev = false;
+
+	if (cli_opt_set(&dopts, D_SUNIT))
+		dsunit = cli->dsunit;
+	if (cli_opt_set(&dopts, D_SWIDTH))
+		dswidth = cli->dswidth;
+
+	if (cli_opt_set(&dopts, D_SU))
+		dsu = getnum(cli->dsu, &dopts, D_SU);
+	if (cli_opt_set(&dopts, D_SW))
+		dsw = cli->dsw;
+
+	/* data sunit/swidth options */
+	if ((dsunit && !dswidth) || (!dsunit && dswidth)) {
+		fprintf(stderr,
+_("both data sunit and data swidth options must be specified\n"));
+		usage();
+	}
+
+	/* convert dsu/dsw to dsunit/dswidth and use them from now on */
+	if (dsu || dsw) {
+		if ((dsu && !dsw) || (!dsu && dsw)) {
+			fprintf(stderr,
+_("both data su and data sw options must be specified\n"));
+			usage();
+		}
+
+		if (dsu % cfg->sectorsize) {
+			fprintf(stderr,
+_("data su must be a multiple of the sector size (%d)\n"), cfg->sectorsize);
+			usage();
+		}
+
+		dsunit  = (int)BTOBBT(dsu);
+		dswidth = dsunit * dsw;
+	}
+
+	if (dsunit && (dswidth % dsunit != 0)) {
+		fprintf(stderr,
+_("data stripe width (%d) must be a multiple of the data stripe unit (%d)\n"),
+			dswidth, dsunit);
+		usage();
+	}
+
+	/* If sunit & swidth were manually specified as 0, same as noalign */
+	if ((cli_opt_set(&dopts, D_SUNIT) || cli_opt_set(&dopts, D_SU)) &&
+	    !dsunit && !dswidth)
+		cfg->sb_feat.nodalign = true;
+
+	/* if we are not using alignment, don't apply device defaults */
+	if (cfg->sb_feat.nodalign) {
+		cfg->dsunit = 0;
+		cfg->dswidth = 0;
+		goto check_lsunit;
+	}
+
+	/* if no stripe config set, use the device default */
+	if (!dsunit) {
+		dsunit = ft->dsunit;
+		dswidth = ft->dswidth;
+		use_dev = true;
+	} else {
+		/* check and warn is alignment is sub-optimal */
+		if (ft->dsunit && ft->dsunit != dsunit) {
+			fprintf(stderr,
+_("%s: Specified data stripe unit %d is not the same as the volume stripe unit %d\n"),
+				progname, dsunit, ft->dsunit);
+		}
+		if (ft->dswidth && ft->dswidth != dswidth) {
+			fprintf(stderr,
+_("%s: Specified data stripe width %d is not the same as the volume stripe width %d\n"),
+				progname, dswidth, ft->dswidth);
+		}
+	}
+
+	/*
+	 * now we have our stripe config, check it's a multiple of block
+	 * size.
+	 */
+	if ((BBTOB(dsunit) % cfg->blocksize) ||
+	    (BBTOB(dswidth) % cfg->blocksize)) {
+		/*
+		 * If we are using device defaults, just clear them and we're
+		 * good to go. Otherwise bail out with an error.
+		 */
+		if (!use_dev) {
+			fprintf(stderr,
+_("%s: Stripe unit(%d) or stripe width(%d) is not a multiple of the block size(%d)\n"),
+				progname, BBTOB(dsunit), BBTOB(dswidth),
+				cfg->blocksize);
+			exit(1);
+		}
+		dsunit = 0;
+		dswidth = 0;
+		cfg->sb_feat.nodalign = true;
+	}
+
+	/* convert from 512 byte blocks to fs blocksize */
+	cfg->dsunit = DTOBT(dsunit, cfg->blocklog);
+	cfg->dswidth = DTOBT(dswidth, cfg->blocklog);
+
+check_lsunit:
+	/* log sunit options */
+	if (cli_opt_set(&lopts, L_SUNIT))
+		lsunit = cli->lsunit;
+	else if (cli_opt_set(&lopts, L_SU))
+		lsu = getnum(cli->lsu, &lopts, L_SU);
+	else /* set default log stripe unit if not set on CLI */
+		lsu = cfg->blocksize;
+
+	if (lsu) {
+		/* verify if lsu is a multiple block size */
+		if (lsu % cfg->blocksize != 0) {
+			fprintf(stderr,
+	_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
+				lsu, cfg->blocksize);
+			usage();
+		}
+		lsunit = (int)BTOBBT(lsu);
+	}
+	if (BBTOB(lsunit) % cfg->blocksize != 0) {
+		fprintf(stderr,
+_("log stripe unit (%d) must be a multiple of the block size (%d)\n"),
+			BBTOB(lsunit), cfg->blocksize);
+		usage();
+	}
+
+	/*
+	 * check that log sunit is modulo fsblksize or default it to dsunit.
+	 */
+	if (lsunit) {
+		/* convert from 512 byte blocks to fs blocks */
+		cfg->lsunit = DTOBT(lsunit, cfg->blocklog);
+	} else if (cfg->sb_feat.log_version == 2 &&
+		   cfg->loginternal && cfg->dsunit) {
+		/* lsunit and dsunit now in fs blocks */
+		cfg->lsunit = cfg->dsunit;
+	}
+
+	if (cfg->sb_feat.log_version == 2 &&
+	    cfg->lsunit * cfg->blocksize > 256 * 1024) {
+		/* Warn only if specified on commandline */
+		if (cli->lsu || cli->lsunit != -1) {
+			fprintf(stderr,
+_("log stripe unit (%d bytes) is too large (maximum is 256KiB)\n"
+  "log stripe unit adjusted to 32KiB\n"),
+				(cfg->lsunit * cfg->blocksize));
+		}
+		/* XXX: 64k block size? */
+		cfg->lsunit = (32 * 1024) / cfg->blocksize;
+	}
+
+}
+
 static void
 print_mkfs_cfg(
 	struct mkfs_params	*cfg,
@@ -3015,11 +3119,8 @@ main(
 	int			dirblocklog;
 	int			dirblocksize;
 	char			*dsize;
-	int			dsu;
-	int			dsw;
 	int			dsunit;
 	int			dswidth;
-	int			dsflag;
 	int			force_overwrite;
 	struct fsxattr		fsx;
 	int			imaxpct;
@@ -3040,11 +3141,8 @@ main(
 	xfs_fsblock_t		logstart;
 	int			lvflag;
 	int			lsflag;
-	int			lsuflag;
-	int			lsunitflag;
 	int			lsectorlog;
 	int			lsectorsize;
-	int			lsu;
 	int			lsunit;
 	int			min_logblocks;
 	xfs_mount_t		*mp;
@@ -3130,14 +3228,14 @@ main(
 
 	agsize = daflag = dasize = dblocks = 0;
 	imflag = 0;
-	liflag = laflag = lsflag = lsuflag = lsunitflag = ldflag = lvflag = 0;
+	liflag = laflag = lsflag = ldflag = lvflag = 0;
 	loginternal = 1;
 	logagno = logblocks = rtblocks = rtextblocks = 0;
 	imaxpct = inodelog = inopblock = isize = 0;
 	dfile = logfile = rtfile = NULL;
 	dsize = logsize = rtsize = protofile = NULL;
-	dsu = dsw = dsunit = dswidth = lalign = lsu = lsunit = 0;
-	dsflag = nodsflag = 0;
+	dsunit = dswidth = lalign = lsunit = 0;
+	nodsflag = 0;
 	force_overwrite = 0;
 	worst_freelist = 0;
 	memset(&fsx, 0, sizeof(fsx));
@@ -3168,18 +3266,6 @@ main(
 			}
 			daflag = cli_opt_set(&dopts, D_AGCOUNT);
 
-			dsunit = cli.dsunit;
-			dswidth = cli.dswidth;
-			dsw = cli.dsw;
-			if (cli_opt_set(&dopts, D_SU)) {
-				dsu = getnum(cli.dsu, &dopts, D_SU);
-				dsflag = 1;
-			}
-			dsflag |= cli_opt_set(&dopts, D_SW) ||
-				  cli_opt_set(&dopts, D_SUNIT) ||
-				  cli_opt_set(&dopts, D_SWIDTH);
-			nodsflag = cli_opt_set(&dopts, D_NOALIGN);
-
 			fsx.fsx_xflags |= cli.fsx.fsx_xflags;
 			fsx.fsx_projid = cli.fsx.fsx_projid;
 			fsx.fsx_extsize = cli.fsx.fsx_extsize;
@@ -3202,13 +3288,6 @@ main(
 			logfile = xi.logname;
 			logsize = cli.logsize;
 
-			lsunit = cli.lsunit;
-			lsunitflag = cli_opt_set(&lopts, L_SUNIT);
-			if (cli_opt_set(&lopts, L_SU)) {
-				lsu = getnum(cli.lsu, &lopts, L_SU);
-				lsuflag = 1;
-			}
-
 			laflag = cli_opt_set(&lopts, L_AGNUM);
 			liflag = cli_opt_set(&lopts, L_INTERNAL);
 			ldflag = cli_opt_set(&lopts, L_NAME) ||
@@ -3297,6 +3376,7 @@ main(
 	cfg.rtblocks = calc_dev_size(cli.rtsize, &cfg, &ropts, R_SIZE, "rt");
 
 	validate_rtextsize(&cfg, &cli, &ft);
+	calc_stripe_factors(&cfg, &cli, &ft);
 
 	/* temp don't break code */
 	sectorsize = cfg.sectorsize;
@@ -3316,16 +3396,13 @@ main(
 	logblocks = cfg.logblocks;
 	rtblocks = cfg.rtblocks;
 	rtextblocks = cfg.rtextblocks;
+	dsunit = cfg.dsunit;
+	dswidth = cfg.dswidth;
+	lsunit = cfg.lsunit;
+	nodsflag = cfg.sb_feat.nodalign;
 	/* end temp don't break code */
 
 
-	calc_stripe_factors(dsu, dsw, sectorsize, lsu, lsectorsize,
-				&dsunit, &dswidth, &lsunit);
-
-	/* If sunit & swidth were manually specified as 0, same as noalign */
-	if (dsflag && !dsunit && !dswidth)
-		nodsflag = 1;
-
 	xi.setblksize = sectorsize;
 
 	/*
@@ -3453,29 +3530,6 @@ reported by the device (%u).\n"),
 		nbmblocks = 0;
 	}
 
-	if (!nodsflag) {
-		if (dsunit) {
-			if (ft.dsunit && ft.dsunit != dsunit) {
-				fprintf(stderr,
-					_("%s: Specified data stripe unit %d "
-					"is not the same as the volume stripe "
-					"unit %d\n"),
-					progname, dsunit, ft.dsunit);
-			}
-			if (ft.dswidth && ft.dswidth != dswidth) {
-				fprintf(stderr,
-					_("%s: Specified data stripe width %d "
-					"is not the same as the volume stripe "
-					"width %d\n"),
-					progname, dswidth, ft.dswidth);
-			}
-		} else {
-			dsunit = ft.dsunit;
-			dswidth = ft.dswidth;
-			nodsflag = 1;
-		}
-	} /* 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.
@@ -3614,30 +3668,6 @@ an AG size that is one stripe unit smaller, for example %llu.\n"),
 	if (!imflag)
 		imaxpct = calc_default_imaxpct(blocklog, dblocks);
 
-	/*
-	 * check that log sunit is modulo fsblksize or default it to dsunit.
-	 */
-
-	if (lsunit) {
-		/* convert from 512 byte blocks to fs blocks */
-		lsunit = DTOBT(lsunit, blocklog);
-	} else if (sb_feat.log_version == 2 && loginternal && dsunit) {
-		/* lsunit and dsunit now in fs blocks */
-		lsunit = dsunit;
-	}
-
-	if (sb_feat.log_version == 2 && (lsunit * blocksize) > 256 * 1024) {
-		/* 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));
-			fprintf(stderr,
-	_("log stripe unit adjusted to 32KiB\n"));
-		}
-		lsunit = (32 * 1024) >> blocklog;
-	}
-
 	min_logblocks = max_trans_res(agsize,
 				   sb_feat.crcs_enabled, sb_feat.dir_version,
 				   sectorlog, blocklog, inodelog, dirblocklog,
-- 
2.13.3


  parent reply	other threads:[~2017-08-29 23:51 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-29 23:50 [PATCH 00/42] mkfs: factor the crap out of the code Dave Chinner
2017-08-29 23:50 ` [PATCH 01/42] mkfs: can't specify sector size of internal log Dave Chinner
2017-08-29 23:50 ` [PATCH 02/42] mkfs: make subopt table const Dave Chinner
2017-08-29 23:50 ` [PATCH 03/42] mkfs: introduce a structure to hold CLI options Dave Chinner
2017-08-29 23:50 ` [PATCH 04/42] mkfs: add generic subopt parsing table Dave Chinner
2017-08-29 23:50 ` [PATCH 05/42] mkfs: factor block subopts parser Dave Chinner
2017-08-29 23:50 ` [PATCH 06/42] mkfs: factor data " Dave Chinner
2017-08-29 23:50 ` [PATCH 07/42] mkfs: factor inode " Dave Chinner
2017-08-29 23:50 ` [PATCH 08/42] mkfs: factor log " Dave Chinner
2017-08-29 23:50 ` [PATCH 09/42] mkfs: factor meta " Dave Chinner
2017-08-29 23:50 ` [PATCH 10/42] mkfs: factor naming " Dave Chinner
2017-08-29 23:50 ` [PATCH 11/42] mkfs: factor rt " Dave Chinner
2017-08-29 23:50 ` [PATCH 12/42] mkfs: factor sector " Dave Chinner
2017-08-29 23:50 ` [PATCH 13/42] mkfs: Introduce mkfs configuration structure Dave Chinner
2017-08-29 23:50 ` [PATCH 14/42] mkfs: factor printing of mkfs config Dave Chinner
2017-08-29 23:50 ` [PATCH 15/42] mkfs: factor in memory superblock setup Dave Chinner
2017-08-29 23:50 ` [PATCH 16/42] mkfs: factor out device preparation Dave Chinner
2017-08-29 23:50 ` [PATCH 17/42] mkfs: factor writing AG headers Dave Chinner
2017-08-29 23:50 ` [PATCH 18/42] mkfs: factor secondary superblock updates Dave Chinner
2017-08-29 23:50 ` [PATCH 19/42] mkfs: introduce default configuration structure Dave Chinner
2017-08-29 23:50 ` [PATCH 20/42] mkfs: rename top level CLI parameters Dave Chinner
2017-08-29 23:50 ` [PATCH 21/42] mkfs: factor sectorsize validation Dave Chinner
2017-08-29 23:50 ` [PATCH 22/42] mkfs: factor blocksize validation Dave Chinner
2017-08-29 23:50 ` [PATCH 23/42] mkfs: factor log sector size validation Dave Chinner
2017-08-29 23:50 ` [PATCH 24/42] mkfs: factor superblock feature validation Dave Chinner
2017-08-29 23:50 ` [PATCH 25/42] mkfs: factor directory blocksize validation Dave Chinner
2017-08-29 23:50 ` [PATCH 26/42] mkfs: factor inode size validation Dave Chinner
2017-08-29 23:50 ` [PATCH 27/42] mkfs: factor out device size calculations Dave Chinner
2017-08-29 23:50 ` [PATCH 28/42] mkfs: fix hidden parameter in DTOBT() Dave Chinner
2017-08-29 23:50 ` [PATCH 29/42] mkfs: factor rtdev extent size validation Dave Chinner
2017-08-29 23:50 ` Dave Chinner [this message]
2017-08-29 23:50 ` [PATCH 31/42] mkfs: factor device opening Dave Chinner
2017-08-29 23:50 ` [PATCH 32/42] mkfs: factor data device validation Dave Chinner
2017-08-29 23:50 ` [PATCH 33/42] mkfs: factor log " Dave Chinner
2017-08-29 23:50 ` [PATCH 34/42] mkfs: factor rt " Dave Chinner
2017-08-29 23:50 ` [PATCH 35/42] mkfs: factor AG geometry calculations Dave Chinner
2017-08-29 23:50 ` [PATCH 36/42] mkfs: factor AG alignment Dave Chinner
2017-08-30 23:44   ` Dave Chinner
2017-08-29 23:50 ` [PATCH 37/42] mkfs: rework imaxpct calculation Dave Chinner
2017-08-29 23:50 ` [PATCH 38/42] mkfs: factor initial mount setup Dave Chinner
2017-08-29 23:50 ` [PATCH 39/42] mkfs: factor log size calculations Dave Chinner
2017-09-05  5:23   ` Dave Chinner
2017-08-29 23:50 ` [PATCH 40/42] mkfs: cleanup redundant temporary code Dave Chinner
2017-08-29 23:50 ` [PATCH 41/42] mkfs: move error functions Dave Chinner
2017-08-29 23:50 ` [PATCH 42/42] mkfs: tidy up definitions Dave Chinner
2017-08-30  1:23 ` [PATCH 00/42] mkfs: factor the crap out of the code Darrick J. Wong
2017-08-30  1:57   ` Dave Chinner
2017-08-30  4:16 ` Luis R. Rodriguez
2017-08-30  5:44   ` Dave Chinner
2017-08-30 22:10     ` Luis R. Rodriguez
2017-08-30 23:22       ` Dave Chinner
2017-08-31  0:05         ` Luis R. Rodriguez
2017-08-31 16:23     ` Jan Tulak
2017-08-30  7:44 ` Martin Steigerwald
2017-09-04 12:31 ` Chandan Rajendra
2017-09-04 15:34   ` Eric Sandeen
2017-09-04 22:40   ` Dave Chinner
2017-09-07 10:31 ` Chandan Rajendra
2017-09-07 23:38   ` Dave Chinner
2017-09-09 10:24 ` Chandan Rajendra
2017-09-15  9:42 ` Jan Tulak
2017-09-16 11:29   ` Dave Chinner
2017-10-24  3:00 ` Eric Sandeen
2017-10-25  0:59   ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170829235052.21050-31-david@fromorbit.com \
    --to=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).