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

lijiang lijiang at redhat.com
Tue Jun 19 20:07:46 PDT 2018


在 2018年04月10日 15:47, Petr Tesarik 写道:
> On Tue, 10 Apr 2018 15:09:37 +0800
> lijiang <lijiang at 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 at 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.
Hi, Petr
Previously mentioned that you would solve this issue, would you have any plans
about this issue?

Thanks.
Lianbo
>> 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
> 



More information about the kexec mailing list