mkfs: default to CRC enabled filesystems

From: Dave Chinner <[email protected]>

It's time to change the mkfs defaults to enable CRCs for all new filesystems. While there, also enable the free inode btree by default, too, as that functionality has also had long enough to make it into distro kernels, too. Also update the man page to reflect the change in defaults.

Signed-off-by Dave Chinner <[email protected]>

---

 man/man8/mkfs.xfs.8 |  9 +++++----
 mkfs/xfs_mkfs.c     | 27 +++++++++++++++++----------
 2 files changed, 22 insertions(+), 14 deletions(-)

man/man8/mkfs.xfs.8 +5 -4

index ad9ff3d..e18f6d1 100644
@@ -150,7 +150,7 @@ of calculating and checking the CRCs is not noticable in normal operation.
 .IP
 By default,
 .B mkfs.xfs
-will not enable metadata CRCs.
+will enable metadata CRCs.
 .TP
 .BI finobt= value
 This option enables the use of a separate free inode btree index in each

@@ -164,10 +164,11 @@ filesystems age.
 .IP
 By default,
 .B mkfs.xfs
-will not create free inode btrees. This feature is also currently only available
-for filesystems created with the
+will create free inode btrees for filesystems created with the (default)
 .B \-m crc=1
-option set.
+option set. When the option
+.B \-m crc=0
+is used, the free inode btree feature is not supported and is disabled.
 .RE
 .TP
 .BI \-d " data_section_options"

mkfs/xfs_mkfs.c +17 -10

index 5084d75..0218491 100644
@@ -1004,6 +1004,7 @@ main(
 	int			lazy_sb_counters;
 	int			crcs_enabled;
 	int			finobt;
+	bool			finobtflag;
 
 	progname = basename(argv[0]);
 	setlocale(LC_ALL, "");

@@ -1036,8 +1037,9 @@ main(
 	force_overwrite = 0;
 	worst_freelist = 0;
 	lazy_sb_counters = 1;
-	crcs_enabled = 0;
-	finobt = 0;
+	crcs_enabled = 1;
+	finobt = 1;
+	finobtflag = false;
 	memset(&fsx, 0, sizeof(fsx));
 
 	memset(&xi, 0, sizeof(xi));

@@ -1538,6 +1540,7 @@ _("cannot specify both crc and ftype\n"));
 					if (c < 0 || c > 1)
 						illegal(value, "m finobt");
 					finobt = c;
+					finobtflag = true;
 					break;
 				default:
 					unknown('m', value);

@@ -1878,15 +1881,19 @@ _("V2 attribute format always enabled on CRC enabled filesytems\n"));
 _("32 bit Project IDs always enabled on CRC enabled filesytems\n"));
 			usage();
 		}
-	}
-
-	/*
-	 * The kernel doesn't currently support crc=0,finobt=1 filesystems.
-	 * Catch it here, disable finobt and warn the user.
-	 */
-	if (finobt && !crcs_enabled) {
-		fprintf(stderr,
+	} else {
+		/*
+		 * The kernel doesn't currently support crc=0,finobt=1
+		 * filesystems. If crcs are not enabled and the user has
+		 * explicitly turned them off then silently turn them off
+		 * to avoid an unnecessary warning. If the user explicitly
+		 * tried to use crc=0,finobt=1, then issue a warning before
+		 * turning them off.
+		 */
+		if (finobt && finobtflag) {
+			fprintf(stderr,
 _("warning: finobt not supported without CRC support, disabled.\n"));
+		}
 		finobt = 0;
 	}
 

Signature · 6 lines

2.0.0

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

From: Dave Chinner <[email protected]>

As reported by Brain:

# ./mkfs/mkfs.xfs -f /dev/test/scratch -b size=512
illegal inode size 512
allowable inode size with 512 byte blocks is 256
# ./mkfs/mkfs.xfs -f /dev/test/scratch -b size=512 -i size=256
Minimum inode size for CRCs is 512 bytes
Usage: mkfs.xfs
...

Fix mkfs to catch the block size as being too small, rather than leaving it for inode size detection to enforce.

Signed-off-by Dave Chinner <[email protected]>

---
 include/xfs_types.h | 3 +++
 mkfs/xfs_mkfs.c     | 6 ++++++
 2 files changed, 9 insertions(+)

include/xfs_types.h +3 -0

index 65c6e66..accb95d 100644
@@ -100,11 +100,14 @@ typedef __uint64_t	xfs_filblks_t;	/* number of blocks in a file */
  * Minimum and maximum blocksize and sectorsize.
  * The blocksize upper limit is pretty much arbitrary.
  * The sectorsize upper limit is due to sizeof(sb_sectsize).
+ * CRC enable filesystems use 512 byte inodes, meaning 512 byte block sizes
+ * cannot be used.
  */
 #define XFS_MIN_BLOCKSIZE_LOG	9	/* i.e. 512 bytes */
 #define XFS_MAX_BLOCKSIZE_LOG	16	/* i.e. 65536 bytes */
 #define XFS_MIN_BLOCKSIZE	(1 << XFS_MIN_BLOCKSIZE_LOG)
 #define XFS_MAX_BLOCKSIZE	(1 << XFS_MAX_BLOCKSIZE_LOG)
+#define XFS_MIN_CRC_BLOCKSIZE	(1 << (XFS_MIN_BLOCKSIZE_LOG + 1))
 #define XFS_MIN_SECTORSIZE_LOG	9	/* i.e. 512 bytes */
 #define XFS_MAX_SECTORSIZE_LOG	15	/* i.e. 32768 bytes */
 #define XFS_MIN_SECTORSIZE	(1 << XFS_MIN_SECTORSIZE_LOG)

mkfs/xfs_mkfs.c +0 -0

index 0218491..e2a052d 100644

mkfs/xfs_mkfs.c +0 -0

 	memset(&ft, 0, sizeof(ft));
 	get_topology(&xi, &ft, force_overwrite);
 

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

Quoted 17 lines · Dave Chinner

From: Dave Chinner <[email protected]>

As reported by Brain:

# ./mkfs/mkfs.xfs -f /dev/test/scratch -b size=512
illegal inode size 512
allowable inode size with 512 byte blocks is 256
# ./mkfs/mkfs.xfs -f /dev/test/scratch -b size=512 -i size=256
Minimum inode size for CRCs is 512 bytes
Usage: mkfs.xfs
...

Fix mkfs to catch the block size as being too small, rather than leaving it for inode size detection to enforce.

Signed-off-by Dave Chinner <[email protected]>

mkfs/xfs_mkfs.c _("cannot specify both crc and ftype\n")); · line 1766

Quoted 6 lines

mkfs/xfs_mkfs.c +6 -0

+	if (crcs_enabled && blocksize < XFS_MIN_CRC_BLOCKSIZE) {
+		fprintf(stderr,
+_("Minimum block size for CRC enabled filesystems is %d bytes.\n"),
+			XFS_MIN_CRC_BLOCKSIZE);
+		usage();
+	}

I'd move this down by the crc inode size check a page or so down since we already have a crcs_enabled block there (the error messages could also be made more uniform I suppose). Otherwise looks good, thanks!

Reviewed-by Brian Foster <[email protected]>