Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1380656 > unrolled thread

Re: [Y2038] [PATCH] drm/sti: Use 64-bit timestamps

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [Y2038] [PATCH] drm/sti: Use 64-bit timestamps Arnd Bergmann <arnd@arndb.de> - 2016-04-17 01:40 +0200
    Re: [Y2038] [PATCH] drm/sti: Use 64-bit timestamps Vincent ABRIOU <vincent.abriou@st.com> - 2016-04-18 12:10 +0200
      Re: [Y2038] [PATCH] drm/sti: Use 64-bit timestamps Arnd Bergmann <arnd@arndb.de> - 2016-04-18 12:40 +0200

#1380656 — Re: [Y2038] [PATCH] drm/sti: Use 64-bit timestamps

On Wednesday 13 April 2016 02:28:02 Tina Ruchandani wrote:
> 'struct timespec' uses a 32-bit field for seconds, which
> will overflow in year 2038 and beyond. This patch is part
> of a larger attempt to remove instances of timeval, timespec
> and time_t, all of which suffer from the y2038 issue, from the
> kernel.
> 
> Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>

Looks good in principle. Two small points:

>  void sti_plane_update_fps(struct sti_plane *plane,
>  			  bool new_frame,
>  			  bool new_field)
>  {
> -	struct timespec now;
> +	ktime_t now;
>  	struct sti_fps_info *fps;
>  	int fpks, fipks, ms_since_last, num_frames, num_fields;
> 
> -	getrawmonotonic(&now);
> +	now = ktime_get();

It's unclear why the driver was using getrawmonotonic() here rather
than ktime_get_ts(). The code is fairly new, so Vincent can
probably explain this.

If it was intentional, we should use ktime_get_raw() instead of
ktime_get().

> @@ -76,7 +66,7 @@ void sti_plane_update_fps(struct sti_plane *plane,
>  		return;
> 
>  	fps->curr_frame_counter++;
> -	ms_since_last = sti_plane_timespec_ms_diff(now, fps->last_timestamp);
> +	ms_since_last = ktime_to_ms(ktime_sub(now, fps->last_timestamp));
>  	num_frames = fps->curr_frame_counter - fps->last_frame_counter;

This could be expressed in a more compact way using ktime_ms_delta().

	Arnd

[toc] | [next] | [standalone]

#1381453

On 04/17/2016 01:39 AM, Arnd Bergmann wrote:
> On Wednesday 13 April 2016 02:28:02 Tina Ruchandani wrote:
>> 'struct timespec' uses a 32-bit field for seconds, which
>> will overflow in year 2038 and beyond. This patch is part
>> of a larger attempt to remove instances of timeval, timespec
>> and time_t, all of which suffer from the y2038 issue, from the
>> kernel.
>>
>> Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
>
> Looks good in principle. Two small points:
>
>>   void sti_plane_update_fps(struct sti_plane *plane,
>>   			  bool new_frame,
>>   			  bool new_field)
>>   {
>> -	struct timespec now;
>> +	ktime_t now;
>>   	struct sti_fps_info *fps;
>>   	int fpks, fipks, ms_since_last, num_frames, num_fields;
>>
>> -	getrawmonotonic(&now);
>> +	now = ktime_get();
>
> It's unclear why the driver was using getrawmonotonic() here rather
> than ktime_get_ts(). The code is fairly new, so Vincent can
> probably explain this.
>
> If it was intentional, we should use ktime_get_raw() instead of
> ktime_get().
>

getrawmonotonic comes from a legacy code so the use is not intentional.
Honestly, it is not clear to me the difference between monotonic and 
rawmonotonic. But in the debug context in which it is used, ktime_get 
and ktime_get_raw will deliver the same level of information we need. So 
implementation done by Tina is fine for me.

Vincent

>> @@ -76,7 +66,7 @@ void sti_plane_update_fps(struct sti_plane *plane,
>>   		return;
>>
>>   	fps->curr_frame_counter++;
>> -	ms_since_last = sti_plane_timespec_ms_diff(now, fps->last_timestamp);
>> +	ms_since_last = ktime_to_ms(ktime_sub(now, fps->last_timestamp));
>>   	num_frames = fps->curr_frame_counter - fps->last_frame_counter;
>
> This could be expressed in a more compact way using ktime_ms_delta().
>
> 	Arnd
>

[toc] | [prev] | [next] | [standalone]

#1381472

On Monday 18 April 2016 12:02:35 Vincent ABRIOU wrote:
> 
> getrawmonotonic comes from a legacy code so the use is not intentional.
> Honestly, it is not clear to me the difference between monotonic and 
> rawmonotonic. But in the debug context in which it is used, ktime_get 
> and ktime_get_raw will deliver the same level of information we need. So 
> implementation done by Tina is fine for me.
> 

Ok, cool, thanks for confirming!

FWIW, the best way I can see for illustrating the difference is that
rawmonotonic time is for things that should be synchronized with the
machines clock generators (e.g. A/V sync), while monotonic time is
for the case where you want to synchronize with another machine (or
the internet) that may have a slightly different clock generator but
uses NTP to correct for that.

In most cases the difference is irrelevant and we tend to use monotonic
time by default.

	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web