On Oct 1, 2020, at 12:03 PM, Theodore Y. Ts'o <tytso@xxxxxxx> wrote: > On Mon, Sep 21, 2020 at 04:16:02PM -0600, adilger@xxxxxxxxxxxxx wrote: >> From: Andreas Dilger <adilger@xxxxxxxxxxxxx> >> >> The e2fsck error message: >> >> inode nnn extent tree (at level 1) could be narrower. Optimize<y>? >> >> can be fairly verbose at times, and leads users to think that there >> may be something wrong with the filesystem. Basically, almost any >> message printed by e2fsck makes users nervous when they are facing >> other corruption, and a few thousand of these printed may hide other >> errors. It also isn't clear that saving a few blocks optimizing the >> extent tree noticeably improves performance. >> >> This message has previously been annoying enough for Ted to add the >> "-E no_optimize_extents" option to disable it. Just enable this >> option by default, similar to the "-D" directory optimization option. >> >> Signed-off-by: Andreas Dilger <adilger@xxxxxxxxx> > > Applying this patch causes a whole bunch of tests fail: > > 348 tests succeeded 9 tests failed > Tests failed: d_punch_bigalloc d_punch f_collapse_extent_tree > f_compress_extent_tree_level f_extent_bad_node f_extent_int_bad_magic > f_extent_leaf_bad_magic f_extent_oobounds f_quota_extent_opt Sorry about that, I usually *do* run the tests after every patch, I'm not sure why I didn't for this patch. >> @@ -1051,6 +1053,11 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx) >> if (c) >> ctx->options |= E2F_OPT_NOOPT_EXTENTS; >> >> + profile_get_boolean(ctx->profile, "options", "optimize_extents", >> + 0, 0, &c); >> + if (c) >> + ctx->options &= ~E2F_OPT_NOOPT_EXTENTS; >> + > > We already have a no_optimize_extents option supported in e2fsck.conf. > So if we want to change the default, a simpler way to do this might be > to edit e2fsck.conf.5.in to simply add "no_optimize_extents=true" to > the default version of e2fsck.conf defined by default. Does that mean you *don't* want a refresh of this patch that fixes the test cases? Lukas had also been discussing how to change e2fsck so it still fixed the inodes, but didn't print a message for each one, though it wasn't clear to me that there is much benefit to this at all. > As a reminder, for future changes, when we add a new tunable to > e2fsck.conf or mke2fs.conf, the man page should be edited. Yes, I did edit the e2fsck.8.in man page to describe the change in default behavior. Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
