Re: [PATCH] trace: Make trace_hwlat timestamp y2038 safe

[Arnd Bergmann]

On Mon, Mar 27, 2017 at 11:02 PM, Deepa Dinamani <deepa.kernel@xxxxxxxxx> wrote:
>>>> >
>>>> >    kernel/trace/trace_output.c: In function 'trace_hwlat_print':
>>>> >>> kernel/trace/trace_output.c:1168:5: warning: format '%lld' expects argument of type 'long long int', but argument 6 has type '__kernel_time_t' [-Wformat=]
>>>> >         field->timestamp.tv_nsec);
>>>>
>>>> This happens on 64-bit architectures because 'struct timespec64' is defined to
>>>> be equal to 'struct timespec', which has a 'long' tv_sec rather than
>>>> 'long long'.
>>>>
>>>> A cast to 's64' is probably the best workaround.
>
> We have been using long long cast to print timespec64 tv_sec.
>
> I'm not sure what format specifier we would use if we cast to s64.
> printk-formats suggests %lld or %llx.
> Meaning a cast to long long.

Right, s64 is alway 'long long'. Out of the two, the former feels
more natural as we normally count seconds in decimal.

>>>>
>>>> >    kernel/trace/trace_output.c: In function 'trace_hwlat_raw':
>>>> >    kernel/trace/trace_output.c:1202:5: warning: format '%lld' expects argument of type 'long long int', but argument 5 has type '__kernel_time_t' [-Wformat=]
>>>> >         field->seqnum);
>>>>
>>>> Same here.
>>>>
>>>
>>> Actually, I believe that "%zd" will work. It's made to work with size_t
>>> which is long long on 32 and long on 64.
>>
>> size_t is always 'long', not 'long long'. We have %pad for dma_addr_t
>> which may be 'long' or 'long long', but it is configuration dependent
>> which one it is on 32-bit.
>>
>> We could probably introduce a %pts format string for timespec64
>> and have that pretty-printed.
>
> dma_addr_t can be u64 or u32 based on architecture be it 32 bit or 64 bit.
> But, timespec64 tv_sec is always s64. This is the opposite problem
> from dma_addr_t that it is 64 bits regardless of architecture.
> So, wouldn't it be easier to use long long cast for tv_sec and then
> print tv_nsec as long?

The cast is clearly easier for this one instance, and that's what we should
do here.

Generally speaking I think it would be nice to have have a way to print
a timespec64 and I thought it would also address this instance but as
Steve pointed out, we can't print objects by reference in trace output
without duplicating the logic there, so let's not go there now.

      Arnd