Hi,

On Wed, Sep 30, 2015 at 11:52 PM, Arnd Bergmann <[email protected]> wrote:
> 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.

In my local repository I modified my .config file so that CONFIG_64BIT
is not set,
and after that I recompiled all directory, but I don't get any errors/warning at
compile-time.
Also I separetly compiled this specific file but still no warnings
My .config  looks something like this:

#
# Automatically generated file; DO NOT EDIT.
# Linux/x86 4.3.0-rc3 Kernel Configuration
#
# CONFIG_64BIT is not set
CONFIG_X86_32=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_PERF_EVENTS_INTEL_UNCORE=y
CONFIG_OUTPUT_FORMAT="elf32-i386"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/i386_defconfig"
. . .

Should I change my working kernel or .config file is just enough?

Thanks,
Ksenija

>         Arnd
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038

Reply via email to