Re: [PATCH] makedumpfile: Do not print ETA value if current progress is 0

[Petr Tesarik <ptesarik@suse.cz>]

On Tue, 10 Apr 2018 15:09:37 +0800
lijiang <lijiang@redhat.com> wrote:

> 在 2018年04月09日 17:40, Petr Tesarik 写道:
>[...]
> > Last but not least, part of the issue was probably caused by the
> > wrong assumption that integers < 100 can be interpreted with max 3
> > ASCII characters, but that's not true for signed integers. To make
> > eta_to_human_short() a bit safer, use an unsigned integer type.  
> > > Signed-off-by: Petr Tesarik <ptesarik@suse.com>  
> Hi, Petr
> It is good that checks the value of "current".
> The previous patch resolved system crash in exceptional case. The calculation
> result of "calc_delta" may be very large number

Hmmm. The resulting ETA is in seconds. To overflow a 32-bit integer,
the elapsed time would have to be more than 2^32 seconds, or approx.
136 years. I think it is reasonable to assume that makedumpfile will
never run for more than a century...

> or even a negative number when making the time jump very great.

Yes, the current code will not produce sensible results if the system
clock moves backwards. With my patch it will most likely say: ">2day".
But it will not work any better if you use a 64-bit integer.

Instead, we should be using clock_gettime(CLOCK_MONOTONIC, ...).
That's a separate issue that I can solve in another patch.

> In this case, it is not enough for "unsigned
> integer type" and double. The struct timeval has two 64bit variables in x86 64.

First, of the second field (tv_usec), only 20 bits may actually be
used, because it can never be greater than one million.

Second, to make 100% sure that the target value fits into the eta
variable, you would have to consider the worst case:

  end = ULONG_MAX
  current = 1
  progress = 100.0 / ULONG_MAX
  delta = { ULONG_MAX, 999999 }
  eta = ULONG_MAX + 999999/1e6
  eta = (100 - progress) * eta / progress

Theoretically, this gives:

  eta = 340345831618257409870852130465035835837

Even a 128-bit integer is too small to hold this maximum theoretical
value (129 bits would be needed).

Let's stay reasonable. Any value which represents more than a few
(dozen) hours is not usable in practice. But hey, to make sure we
cannot hit undefined behaviour, why not pass a double to
eta_to_human_short()...

Going to send an updated patch.

Petr T

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec