Skip to contentSkip to conversation

mke2fs: add discard to default mke2fs.conf and disable by default

PATCH Superseded Eric Sandeen 2 participants 4 messages Apr 17, 2013 – Apr 18, 2013

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mke2fs: add discard to default mke2fs.conf and disable by default

Discard is a very big hammer. If it succeeds, there's no undo or going back; on the other hand, on some devices the giant discard issued at mkfs time makes them go offline. :(

Although I'm reluctant to waffle back and forth on default behavior, I think this disabling it by default the prudent thing to do.

This patch also adds the discard option into the default mke2fs.conf file; although it could be set, it was not in the template.

---

If disabling by default isn't desired, we should at least include the mke2fs.conf.in change (set to 1).

Thanks,
-Eric

misc/mke2fs.8.in +3 -3
index 023ba49..a005eca 100644
@@ -277,11 +277,11 @@ Attempt to discard blocks at mkfs time (discarding blocks initially is useful
 on solid state devices and sparse / thin-provisioned storage). When the device
 advertises that discard also zeroes data (any subsequent read after the discard
 and before write returns zero), then mark all not-yet-zeroed inode tables as
-zeroed. This significantly speeds up filesystem initialization. This is set
-as default.
+zeroed. This significantly speeds up filesystem initialization. This is disabled
+by default.
 .TP
 .BI nodiscard
-Do not attempt to discard blocks at mkfs time.
+Do not attempt to discard blocks at mkfs time.  This is the default.
 .TP
 .BI quotatype
 Specify which quota type ('usr' or 'grp') is to be initialized. This
misc/mke2fs.c +1 -1
index bbf477a..d723ee7 100644
@@ -84,7 +84,7 @@ int	cflag;
 int	verbose;
 int	quiet;
 int	super_only;
-int	discard = 1;	/* attempt to discard device before fs creation */
+int	discard;	/* attempt to discard device before fs creation? */
 int	direct_io;
 int	force;
 int	noaction;
misc/mke2fs.conf.in +1 -0
index 178733f..db6decb 100644
@@ -2,6 +2,7 @@
 	base_features = sparse_super,filetype,resize_inode,dir_index,ext_attr
 	default_mntopts = acl,user_xattr
 	enable_periodic_fsck = 0
+	discard = 0
 	blocksize = 4096
 	inode_size = 256
 	inode_ratio = 16384
#1 lore raw
Quoted 11 lines · Eric Sandeen

Discard is a very big hammer. If it succeeds, there's no undo or going back; on the other hand, on some devices the giant discard issued at mkfs time makes them go offline. :(

Although I'm reluctant to waffle back and forth on default behavior, I think this disabling it by default the prudent thing to do.

This patch also adds the discard option into the default mke2fs.conf file; although it could be set, it was not in the template.

Sorry, scratch this; it won't work if, say, discard=1 in the conf file and the user specifies -E nodiscard or -K; it'll turn it back on after those options are processed, I think. I'll send another patch later when I'm not firing it off too quickly from a conference. ;)

-Eric

Quoted 58 lines

---

If disabling by default isn't desired, we should at least include the mke2fs.conf.in change (set to 1).

Thanks,

(quoted diff) +0 -1
-Eric
misc/mke2fs.8.in +3 -3
index 023ba49..a005eca 100644
@@ -277,11 +277,11 @@ Attempt to discard blocks at mkfs time (discarding blocks initially is useful
 on solid state devices and sparse / thin-provisioned storage). When the device
 advertises that discard also zeroes data (any subsequent read after the discard
 and before write returns zero), then mark all not-yet-zeroed inode tables as
-zeroed. This significantly speeds up filesystem initialization. This is set
-as default.
+zeroed. This significantly speeds up filesystem initialization. This is disabled
+by default.
 .TP
 .BI nodiscard
-Do not attempt to discard blocks at mkfs time.
+Do not attempt to discard blocks at mkfs time.  This is the default.
 .TP
 .BI quotatype
 Specify which quota type ('usr' or 'grp') is to be initialized. This
misc/mke2fs.c +1 -1
index bbf477a..d723ee7 100644
@@ -84,7 +84,7 @@ int	cflag;
 int	verbose;
 int	quiet;
 int	super_only;
-int	discard = 1;	/* attempt to discard device before fs creation */
+int	discard;	/* attempt to discard device before fs creation? */
 int	direct_io;
 int	force;
 int	noaction;
misc/mke2fs.conf.in +1 -1
index 178733f..db6decb 100644
@@ -2,6 +2,7 @@
 	base_features = sparse_super,filetype,resize_inode,dir_index,ext_attr
 	default_mntopts = acl,user_xattr
 	enable_periodic_fsck = 0
+	discard = 0
 	blocksize = 4096
 	inode_size = 256
 	inode_ratio = 16384
 
--

To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw

PATCH v2 1/1 mke2fs: add discard to default mke2fs.conf and disable by default

In reply to Eric Sandeen

Discard is a very big hammer. If it succeeds, there's no undo or going back; on the other hand, on some devices the giant discard issued at mkfs time makes them go offline.

Although I'm reluctant to waffle back and forth on default behavior, I think this disabling it by default the prudent thing to do.

This patch also adds the discard option into the default mke2fs.conf file; although it could be set, it was not in the template.

---

V2: handle discard disable via "-K" since it's parsed *before* we get the default. The extended options are parsed later, and will override the mke2fs.conf default without problems.

misc/mke2fs.8.in +3 -3
index 023ba49..a005eca 100644
@@ -277,11 +277,11 @@ Attempt to discard blocks at mkfs time (discarding blocks initially is useful
 on solid state devices and sparse / thin-provisioned storage). When the device
 advertises that discard also zeroes data (any subsequent read after the discard
 and before write returns zero), then mark all not-yet-zeroed inode tables as
-zeroed. This significantly speeds up filesystem initialization. This is set
-as default.
+zeroed. This significantly speeds up filesystem initialization. This is disabled
+by default.
 .TP
 .BI nodiscard
-Do not attempt to discard blocks at mkfs time.
+Do not attempt to discard blocks at mkfs time.  This is the default.
 .TP
 .BI quotatype
 Specify which quota type ('usr' or 'grp') is to be initialized. This
misc/mke2fs.c +5 -3
index bbf477a..fa11da6 100644
@@ -84,7 +84,7 @@ int	cflag;
 int	verbose;
 int	quiet;
 int	super_only;
-int	discard = 1;	/* attempt to discard device before fs creation */
+int	discard;	/* attempt to discard device before fs creation? */
 int	direct_io;
 int	force;
 int	noaction;
@@ -1266,6 +1266,7 @@ static void PRS(int argc, char *argv[])
 	double		reserved_ratio = -1.0;
 	int		lsector_size = 0, psector_size = 0;
 	int		show_version_only = 0;
+	int		nodiscard = 0;
 	unsigned long long num_inodes = 0; /* unsigned long long to catch too-large input */
 	errcode_t	retval;
 	char *		oldpath = getenv("PATH");
@@ -1444,7 +1445,7 @@ profile_error:
 					  "should not be used anymore. Use "
 					  "\'-E nodiscard\' extended option "
 					  "instead!\n"));
-			discard = 0;
+			nodiscard = 1;
 			break;
 		case 'j':
 			if (!journal_size)
@@ -1985,7 +1986,8 @@ profile_error:
 	lazy_itable_init = get_bool_from_profile(fs_types,
 						 "lazy_itable_init",
 						 lazy_itable_init);
-	discard = get_bool_from_profile(fs_types, "discard" , discard);
+	/* nodiscard is set if -K was used, extended discard opts parsed later */
+	discard = nodiscard ? 0 : get_bool_from_profile(fs_types, "discard" , 0);
 	journal_flags |= get_bool_from_profile(fs_types,
 					       "lazy_journal_init", 0) ?
 					       EXT2_MKJOURNAL_LAZYINIT : 0;
misc/mke2fs.conf.in +1 -0
index 178733f..db6decb 100644
@@ -2,6 +2,7 @@
 	base_features = sparse_super,filetype,resize_inode,dir_index,ext_attr
 	default_mntopts = acl,user_xattr
 	enable_periodic_fsck = 0
+	discard = 0
 	blocksize = 4096
 	inode_size = 256
 	inode_ratio = 16384
#3 lore raw
Quoted 7 lines · Eric Sandeen

Discard is a very big hammer. If it succeeds, there's no undo or going back; on the other hand, on some devices the giant discard issued at mkfs time makes them go offline.

Although I'm reluctant to waffle back and forth on default behavior, I think this disabling it by default the prudent thing to do.

Ok, self-nak on this one, the folks in the hallway track convinced me that it was wrong. I'll send another to handle it properly in mke2fs.conf, though.

Thanks,
Eric

Quoted 86 lines

This patch also adds the discard option into the default mke2fs.conf file; although it could be set, it was not in the template.

---

V2: handle discard disable via "-K" since it's parsed *before* we get the default. The extended options are parsed later, and will override the mke2fs.conf default without problems.

misc/mke2fs.8.in +0 -0
index 023ba49..a005eca 100644
@@ -277,11 +277,11 @@ Attempt to discard blocks at mkfs time (discarding blocks initially is useful

on solid state devices and sparse / thin-provisioned storage). When the device advertises that discard also zeroes data (any subsequent read after the discard and before write returns zero), then mark all not-yet-zeroed inode tables as

misc/mke2fs.8.in +2 -2
-zeroed. This significantly speeds up filesystem initialization. This is set
-as default.
+zeroed. This significantly speeds up filesystem initialization. This is disabled
+by default.

.TP
.BI nodiscard

misc/mke2fs.8.in +1 -1
-Do not attempt to discard blocks at mkfs time.
+Do not attempt to discard blocks at mkfs time.  This is the default.

.TP
.BI quotatype
Specify which quota type ('usr' or 'grp') is to be initialized. This

misc/mke2fs.c +5 -3
index bbf477a..fa11da6 100644
@@ -84,7 +84,7 @@ int    cflag;
 int    verbose;
 int    quiet;
 int    super_only;
-int    discard = 1;    /* attempt to discard device before fs creation */
+int    discard;    /* attempt to discard device before fs creation? */
 int    direct_io;
 int    force;
 int    noaction;
@@ -1266,6 +1266,7 @@ static void PRS(int argc, char *argv[])
   double        reserved_ratio = -1.0;
   int        lsector_size = 0, psector_size = 0;
   int        show_version_only = 0;
+    int        nodiscard = 0;
   unsigned long long num_inodes = 0; /* unsigned long long to catch too-large input */
   errcode_t    retval;
   char *        oldpath = getenv("PATH");
@@ -1444,7 +1445,7 @@ profile_error:
                     "should not be used anymore. Use "
                     "\'-E nodiscard\' extended option "
                     "instead!\n"));
-            discard = 0;
+            nodiscard = 1;
           break;
       case 'j':
           if (!journal_size)
@@ -1985,7 +1986,8 @@ profile_error:
   lazy_itable_init = get_bool_from_profile(fs_types,
                        "lazy_itable_init",
                        lazy_itable_init);
-    discard = get_bool_from_profile(fs_types, "discard" , discard);
+    /* nodiscard is set if -K was used, extended discard opts parsed later */
+    discard = nodiscard ? 0 : get_bool_from_profile(fs_types, "discard" , 0);
   journal_flags |= get_bool_from_profile(fs_types,
                          "lazy_journal_init", 0) ?
                          EXT2_MKJOURNAL_LAZYINIT : 0;
misc/mke2fs.conf.in +1 -1
index 178733f..db6decb 100644
@@ -2,6 +2,7 @@
   base_features = sparse_super,filetype,resize_inode,dir_index,ext_attr
   default_mntopts = acl,user_xattr
   enable_periodic_fsck = 0
+    discard = 0
   blocksize = 4096
   inode_size = 256
   inode_ratio = 16384
 
--

To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html