2015-03-13 22:31:18

by Eric Sandeen

[permalink] [raw]
Subject: How is e2fsck's time_fudge supposed to behave?

I'm a little confused by e2fsck's time fudge current behavior, vs its
apparent intent.

We do:

if ( ... &&
fs->super->s_mtime > (__u32) ctx->now) {
pctx.num = fs->super->s_mtime;
problem = PR_0_FUTURE_SB_LAST_MOUNT;
if (fs->super->s_mtime <= (__u32) ctx->now + ctx->time_fudge)
problem = PR_0_FUTURE_SB_LAST_MOUNT_FUDGED;
if (fix_problem(ctx, problem, &pctx)) {
fs->super->s_mtime = ctx->now;
fs->flags |= EXT2_FLAG_DIRTY;
}

So if we are inside the time_fudge value we simply change the problem,
but PR_0_FUTURE_SB_LAST_MOUNT_FUDGED behaves exactly like
PR_0_FUTURE_SB_LAST_MOUNT, other than the message:

/* Last mount time is in the future (fudged) */
{ PR_0_FUTURE_SB_LAST_MOUNT_FUDGED,
N_("@S last mount time is in the future.\n\t(by less than a day, "
"probably due to the hardware clock being incorrectly set) "),
PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },

vs:

/* Last mount time is in the future */
{ PR_0_FUTURE_SB_LAST_MOUNT,
N_("@S last mount time (%t,\n\tnow = %T) is in the future.\n"),
PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },

So unless I'm missing something, the whole fudge_time dance does nothing
except change the message, and after reading lots of words in the e2fsck.conf
manpage ;) this bit seems relevant as to the intent:

> So by default, we allow the superblock times to
> be fudged by up to 24 hours.

I had the impression that "allow" meant "ignore" but this still triggers
exactly the same action and correction. Is that as intended?

I'll send a patch do a printf and take no other action if inside the
fudge_time window, if that seems like the right thing to do.

Thanks,
-Eric


2015-03-17 17:32:32

by Jan Kara

[permalink] [raw]
Subject: Re: How is e2fsck's time_fudge supposed to behave?

On Fri 13-03-15 17:31:18, Eric Sandeen wrote:
> I'm a little confused by e2fsck's time fudge current behavior, vs its
> apparent intent.
>
> We do:
>
> if ( ... &&
> fs->super->s_mtime > (__u32) ctx->now) {
> pctx.num = fs->super->s_mtime;
> problem = PR_0_FUTURE_SB_LAST_MOUNT;
> if (fs->super->s_mtime <= (__u32) ctx->now + ctx->time_fudge)
> problem = PR_0_FUTURE_SB_LAST_MOUNT_FUDGED;
> if (fix_problem(ctx, problem, &pctx)) {
> fs->super->s_mtime = ctx->now;
> fs->flags |= EXT2_FLAG_DIRTY;
> }
>
> So if we are inside the time_fudge value we simply change the problem,
> but PR_0_FUTURE_SB_LAST_MOUNT_FUDGED behaves exactly like
> PR_0_FUTURE_SB_LAST_MOUNT, other than the message:
>
> /* Last mount time is in the future (fudged) */
> { PR_0_FUTURE_SB_LAST_MOUNT_FUDGED,
> N_("@S last mount time is in the future.\n\t(by less than a day, "
> "probably due to the hardware clock being incorrectly set) "),
> PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },
>
> vs:
>
> /* Last mount time is in the future */
> { PR_0_FUTURE_SB_LAST_MOUNT,
> N_("@S last mount time (%t,\n\tnow = %T) is in the future.\n"),
> PROMPT_FIX, PR_PREEN_OK | PR_NO_OK },
>
> So unless I'm missing something, the whole fudge_time dance does nothing
> except change the message, and after reading lots of words in the e2fsck.conf
> manpage ;) this bit seems relevant as to the intent:
>
> > So by default, we allow the superblock times to
> > be fudged by up to 24 hours.
>
> I had the impression that "allow" meant "ignore" but this still triggers
> exactly the same action and correction. Is that as intended?
>
> I'll send a patch do a printf and take no other action if inside the
> fudge_time window, if that seems like the right thing to do.
The actions became the same after commit
87aca2ad028b9 (e2fsck: fix last mount time and last write time in preen
mode). Previously only fudged values were allowed to be fixed in the preen
mode. The question is whether we now want to change e2fsck to just ignore
difference within fudge or whether we just stop doing that fudge thing.
Either makes sense to me...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2015-03-18 18:59:00

by Eric Sandeen

[permalink] [raw]
Subject: Re: How is e2fsck's time_fudge supposed to behave?

On 3/17/15 12:32 PM, Jan Kara wrote:
> On Fri 13-03-15 17:31:18, Eric Sandeen wrote:

...

>> So unless I'm missing something, the whole fudge_time dance does nothing
>> except change the message, and after reading lots of words in the e2fsck.conf
>> manpage ;) this bit seems relevant as to the intent:
>>
>>> So by default, we allow the superblock times to
>>> be fudged by up to 24 hours.
>>
>> I had the impression that "allow" meant "ignore" but this still triggers
>> exactly the same action and correction. Is that as intended?
>>
>> I'll send a patch do a printf and take no other action if inside the
>> fudge_time window, if that seems like the right thing to do.
> The actions became the same after commit
> 87aca2ad028b9 (e2fsck: fix last mount time and last write time in preen
> mode). Previously only fudged values were allowed to be fixed in the preen
> mode. The question is whether we now want to change e2fsck to just ignore
> difference within fudge or whether we just stop doing that fudge thing.
> Either makes sense to me...

Oh, thanks, I had missed that. Funny that OpenSUSE also set broken_system_clock;
Fedora had been doing that too. For the same reasons. o_O

The problem I'm still seeing is that if the clock is off by under 24h, we still
do fix_problem(), and set E2F_FLAG_PROBLEMS_FIXED, so check_if_skip() doesn't allow
a skip. This happens on every single boot.

It sure seems like the intent was to completely ignore superblock time deltas under
24h... I guess I'll send a patch to do that, and see what Ted thinks.

Thanks,
-Eric