Skip to contentSkip to conversation

- Make mke2fs.c defaults match mke2fs.conf defaults

PATCH Applied Eric Sandeen 3 participants 5 messages May 7, 2007 – May 8, 2007

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 - Make mke2fs.c defaults match mke2fs.conf defaults

One of our testers filed a bug that said "mkfs.ext3 is much slower when mke2fs.conf is missing..."

This is because the shipped defaults in mke2fs.conf do not match the shipped defaults in the mkfs code itself; he wound up making a 1k block filesystem on a very large block device, for example.

So - How about this patch, to bring them back into line? Which makes me wonder; having "defaults" in 2 different places is bound to get out of sync; should we instead generate both code & config file defaults (and maybe man page defaults) from a common source?

Anyway, here's a patch to bring mke2fs.conf and mke2fs.c into line for current defaults...

Thanks,
-Eric

misc/mke2fs.c +3 -2
Index: e2fsprogs-hg/misc/mke2fs.c
===================================================================
@@ -1288,7 +1288,8 @@ static void PRS(int argc, char *argv[])
 	tmp = tmp2 = NULL;
 	if (fs_param.s_rev_level != EXT2_GOOD_OLD_REV) {
 		profile_get_string(profile, "defaults", "base_features", 0,
-				   "filetype,sparse_super", &tmp);
+				   "sparse_super,filetype,resize_inode,dir_index",
+				   &tmp);
 		profile_get_string(profile, "fs_types", fs_type, 
 				   "base_features", tmp, &tmp2);
 		edit_feature(tmp2, &fs_param.s_feature_compat);
@@ -1365,7 +1366,7 @@ static void PRS(int argc, char *argv[])
 	
 	if (blocksize <= 0) {
 		profile_get_integer(profile, "defaults", "blocksize", 0,
-				    1024, &use_bsize);
+				    4096, &use_bsize);
 		profile_get_integer(profile, "fs_types", fs_type, 
 				    "blocksize", use_bsize, &use_bsize);
 
#1 lore raw

On Mon, May 07, 2007 at 11:31:22AM -0500, Eric Sandeen wrote: One of

Quoted 8 lines

our testers filed a bug that said "mkfs.ext3 is much slower when mke2fs.conf is missing..."

This is because the shipped defaults in mke2fs.conf do not match the shipped defaults in the mkfs code itself; he wound up making a 1k block filesystem on a very large block device, for example.

So - How about this patch, to bring them back into line?

It doesn't actually bring them completely back into line, since mke2fs will use different block sizes depending on the size of the filesystem. So your patch makes the default probably a bit more reasonable, and so I'll probably end up applying it, but it definitely isn't a complete replacement for /etc/mke2fs.conf.

How likely do you think the case will be that mke2fs.conf would be missing? I'm trying to figure out how high priority of an item this really is.

Quoted 4 lines

Which makes me wonder; having "defaults" in 2 different places is bound to get out of sync; should we instead generate both code & config file defaults (and maybe man page defaults) from a common source?

I had been working on the assumption that the defaults if mke2fs.conf were not present were more in the nature of emergency defaults as opposed something that could be used a fully functional set of configuration parameters. So the assumption was that when you installed the RPM, mke2fs.conf would also be there.

We could enhance the profile code so that it could read in the profile from a memory buffer, and simply compile /etc/mke2fs.conf into mke2fs, but that adds bloat --- the question is how necessary do we think that really is?

Regards,

- Ted

#2 lore raw
Quoted 8 lines · Theodore Tso

On Mon, May 07, 2007 at 11:31:22AM -0500, Eric Sandeen wrote: One of

Quoted 7 lines

our testers filed a bug that said "mkfs.ext3 is much slower when mke2fs.conf is missing..."

This is because the shipped defaults in mke2fs.conf do not match the shipped defaults in the mkfs code itself; he wound up making a 1k block filesystem on a very large block device, for example.

Quoted 7 lines
Quoted 1 line

So - How about this patch, to bring them back into line?

It doesn't actually bring them completely back into line, since mke2fs will use different block sizes depending on the size of the filesystem. So your patch makes the default probably a bit more reasonable, and so I'll probably end up applying it, but it definitely isn't a complete replacement for /etc/mke2fs.conf.

Well, sure, I don't mean for it to *replace* mke2fs.conf...

Hm, does having

[defaults]

        blocksize = 4096

in mke2fs.conf turn off the blocksize heuristics and force 4k? is what's in mke2fs.conf a starting point or an absolute? I guess I need to read up on the code...

Quoted 3 lines

How likely do you think the case will be that mke2fs.conf would be missing? I'm trying to figure out how high priority of an item this really is.

Well, not too likely, although for some reason I guess it happened in the installer root in FC6 or so. That's what raised the issue.

Quoted 4 lines

We could enhance the profile code so that it could read in the profile from a memory buffer, and simply compile /etc/mke2fs.conf into mke2fs, but that adds bloat --- the question is how necessary do we think that really is?

I guess it doesn't really sound *necessary* - it's just that if we have 2 different "defaults" and they drift, it can be confusing...

-Eric

#3 lore raw
Quoted 7 lines · Eric Sandeen
Quoted 3 lines · Theodore Tso

How likely do you think the case will be that mke2fs.conf would be missing? I'm trying to figure out how high priority of an item this really is.

Well, not too likely, although for some reason I guess it happened in the installer root in FC6 or so. That's what raised the issue.

Ah, good point - there are probably lots of installers and rescue disks that grab mke2fs but don't know anything about /etc/mke2fs.conf.

Quoted 7 lines
Quoted 4 lines

We could enhance the profile code so that it could read in the profile from a memory buffer, and simply compile /etc/mke2fs.conf into mke2fs, but that adds bloat --- the question is how necessary do we think that really is?

I guess it doesn't really sound *necessary* - it's just that if we have 2 different "defaults" and they drift, it can be confusing...

Since the shipped mke2fs.conf is only on the order of a few hundred bytes I don't think it is a huge issue to include it. I like the idea of it being compiled into the code and yet consistent with the default install.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.