On Wednesday 30 September 2015 18:10:49 Ksenija Stanojevic wrote:
> struct timespec will overflow in year 2038, so replace it with
> ktime_t. And replace functions that use struct timespec,
> timespec_sub with ktime_sub. Also use monotonic time instead of real
> time, by replacing getnstimeofday with ktime_get, to be more robust
> against leap seconds and settimeofday() calls.
> diff --git a/drivers/staging/fbtft/fbtft-core.c
> b/drivers/staging/fbtft/fbtft-core.c
> index 7f5fa3d..a1645e1 100644
> --- a/drivers/staging/fbtft/fbtft-core.c
> +++ b/drivers/staging/fbtft/fbtft-core.c
> @@ -365,16 +365,15 @@ static void fbtft_update_display(struct fbtft_par *par,
> unsigned start_line,
> unsigned end_line)
> {
> size_t offset, len;
> - struct timespec ts_start, ts_end, ts_fps, ts_duration;
> - long fps_ms, fps_us, duration_ms, duration_us;
> - long fps, throughput;
> + ktime_t ts_start, ts_end, ts_fps;
> + long long fps, throughput;
Here you declare fps and throughput as 'long long', which causes problems later:
> @@ -411,30 +410,22 @@ static void fbtft_update_display(struct fbtft_par *par,
> unsigned start_line,
> __func__);
>
> if (unlikely(timeit)) {
> - getnstimeofday(&ts_end);
> - if (par->update_time.tv_nsec == 0 && par->update_time.tv_sec ==
> 0) {
> - par->update_time.tv_sec = ts_start.tv_sec;
> - par->update_time.tv_nsec = ts_start.tv_nsec;
> - }
> - ts_fps = timespec_sub(ts_start, par->update_time);
> - par->update_time.tv_sec = ts_start.tv_sec;
> - par->update_time.tv_nsec = ts_start.tv_nsec;
> - fps_ms = (ts_fps.tv_sec * 1000) + ((ts_fps.tv_nsec / 1000000) %
> 1000);
> - fps_us = (ts_fps.tv_nsec / 1000) % 1000;
> - fps = fps_ms * 1000 + fps_us;
> + ts_end = ktime_get();
> + if (par->update_time.tv64 == 0)
> + par->update_time = ts_start;
It's better not to access the 'tv64' field of the ktime_t directly, this
is supposed to be hidden. Just use ktime_to_ns() to do the same thing.
> + ts_fps = ktime_sub(ts_start, par->update_time);
> + par->update_time = ts_start;
> + fps = ktime_to_us(ts_fps);
This can be written slightly simpler using the ktime_us_delta() function,
like you do below.
> fps = fps ? 1000000 / fps : 0;
>
> - ts_duration = timespec_sub(ts_end, ts_start);
> - duration_ms = (ts_duration.tv_sec * 1000) +
> ((ts_duration.tv_nsec / 1000000) % 1000);
> - duration_us = (ts_duration.tv_nsec / 1000) % 1000;
> - throughput = duration_ms * 1000 + duration_us;
> + throughput = ktime_us_delta(ts_end, ts_start);
> throughput = throughput ? (len * 1000) / throughput : 0;
> throughput = throughput * 1000 / 1024;
As mentioned above, throughput is a 64-bit 'long long', so the last line
of the context will result in a 64-bit division, which is not allowed in
32-bit kernels.
This is a bit hard to detect, and the most reliable way to find issues
like this is to compile the kernel for both a 32-bit target and a 64-bit
target (on x86, just turn CONFIG_64BIT on/off) to see all the compile-time
errors and warnings.
Arnd
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038
