Re: [PATCH] e2fsck: skip extent optimization by default

[Andreas Dilger <adilger@dilger.ca>]

[-- Attachment #1: Type: text/plain, Size: 4408 bytes --]

On Sep 22, 2020, at 4:26 AM, Lukas Czerner <lczerner@redhat.com> wrote:
> 
> On Mon, Sep 21, 2020 at 04:16:02PM -0600, adilger@whamcloud.com wrote:
>> From: Andreas Dilger <adilger@whamcloud.com>
>> 
>> 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.
> 
> it seem counterproductive to me that we would disable usefull (even if
> just a little) optimization just because the way it is presented to the
> user is inconvenient. I agree that messages during e2fsck often raise
> alarms, as they should, but perfeps instead of disabling the feature we
> can figure out a way to make the messaging better ?
> 
> Can we just not print the every message if the answer is going to be yes
> anyway, either because of -y, -p, <a> or whatever when the user is not
> involved in the decision anymore ? Maybe a log file can be created
> for the purpose of storing the full log of changes. Or perhaps we can
> print out a summary for each type of the problem and how many of the
> instaces of a particular problem have been optimized/fixed after the
> e2fsck is done pointing to that full log for details ?

I think the standard way to handle this in e2fsck is with a "latch question",
so that after the first or second 'y' with "answer 'y' to all questions".
This will quiet most of the messages without disabling the optimization.

The other question is whether the "optimization" is worthwhile or not?
Since e2fsck is rarely run, a number of unoptimized files will exist in
the filesystem all the time.  In our case at least, files have a turnover
rate, so optimizing the current set of inodes doesn't help much, since
they would likely be deleted in a few weeks and new files will replace them.

Cheers, Andreas

>> 
>> Signed-off-by: Andreas Dilger <adilger@dilger.ca>
>> ---
>> e2fsck/e2fsck.8.in | 4 ++--
>> e2fsck/unix.c      | 7 +++++++
>> 2 files changed, 9 insertions(+), 2 deletions(-)
>> 
>> diff --git a/e2fsck/e2fsck.8.in b/e2fsck/e2fsck.8.in
>> index 4e3890b..4f5086a 100644
>> --- a/e2fsck/e2fsck.8.in
>> +++ b/e2fsck/e2fsck.8.in
>> @@ -228,12 +228,12 @@ exactly the opposite of discard option. This is set as default.
>> .TP
>> .BI no_optimize_extents
>> Do not offer to optimize the extent tree by eliminating unnecessary
>> -width or depth.  This can also be enabled in the options section of
>> +width or depth.  This is the default unless otherwise specified in
>> .BR /etc/e2fsck.conf .
>> .TP
>> .BI optimize_extents
>> Offer to optimize the extent tree by eliminating unnecessary
>> -width or depth.  This is the default unless otherwise specified in
>> +width or depth.  This can also be enabled in the options section of
>> .BR /etc/e2fsck.conf .
>> .TP
>> .BI inode_count_fullmap
>> diff --git a/e2fsck/unix.c b/e2fsck/unix.c
>> index 1b7ccea..445f806 100644
>> --- a/e2fsck/unix.c
>> +++ b/e2fsck/unix.c
>> @@ -840,6 +840,8 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx)
>> 	else
>> 		ctx->program_name = "e2fsck";
>> 
>> +	ctx->options |= E2F_OPT_NOOPT_EXTENTS;
>> +
>> 	phys_mem_kb = get_memory_size() / 1024;
>> 	ctx->readahead_kb = ~0ULL;
>> 	while ((c = getopt(argc, argv, "panyrcC:B:dE:fvtFVM:b:I:j:P:l:L:N:SsDkz:")) != EOF)
>> @@ -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;
>> +
>> 	profile_get_boolean(ctx->profile, "options", "inode_count_fullmap",
>> 			    0, 0, &c);
>> 	if (c)
>> --
>> 1.7.12.4
>> 
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]