diff mbox series

[ovs-dev] timeval: Fix misleading function descriptions.

Message ID 1503904105-10475-1-git-send-email-i.maximets@samsung.com
State Accepted
Headers show
Series [ovs-dev] timeval: Fix misleading function descriptions. | expand

Commit Message

Ilya Maximets Aug. 28, 2017, 7:08 a.m. UTC
TIME_UPDATE_INTERVAL was removed long time ago.
Now each call leads to time update via syscall and it's
granularity is system dependent.

Fixes: 31ef9f5178de ("timeval: Remove CACHE_TIME scheme.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/timeval.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Gregory Rose Aug. 29, 2017, 4:27 p.m. UTC | #1
On 08/28/2017 12:08 AM, Ilya Maximets wrote:
> TIME_UPDATE_INTERVAL was removed long time ago.
> Now each call leads to time update via syscall and it's
> granularity is system dependent.
>
> Fixes: 31ef9f5178de ("timeval: Remove CACHE_TIME scheme.")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>   lib/timeval.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/lib/timeval.c b/lib/timeval.c
> index dd63f03..2c7f43a 100644
> --- a/lib/timeval.c
> +++ b/lib/timeval.c
> @@ -171,16 +171,14 @@ time_timespec__(struct clock *c, struct timespec *ts)
>       }
>   }
>
> -/* Stores a monotonic timer, accurate within TIME_UPDATE_INTERVAL ms, into
> - * '*ts'. */
> +/* Stores a monotonic timer into '*ts'. */
>   void
>   time_timespec(struct timespec *ts)
>   {
>       time_timespec__(&monotonic_clock, ts);
>   }
>
> -/* Stores the current time, accurate within TIME_UPDATE_INTERVAL ms, into
> - * '*ts'. */
> +/* Stores the current time into '*ts'. */
>   void
>   time_wall_timespec(struct timespec *ts)
>   {
> @@ -219,14 +217,14 @@ time_msec__(struct clock *c)
>       return timespec_to_msec(&ts);
>   }
>
> -/* Returns a monotonic timer, in ms (within TIME_UPDATE_INTERVAL ms). */
> +/* Returns a monotonic timer, in ms. */
>   long long int
>   time_msec(void)
>   {
>       return time_msec__(&monotonic_clock);
>   }
>
> -/* Returns the current time, in ms (within TIME_UPDATE_INTERVAL ms). */
> +/* Returns the current time, in ms. */
>   long long int
>   time_wall_msec(void)
>   {
>
How about replaciing "accurate within TIME_UPDATE_INTERVAL ms" with
"accurate within the system dependent timer interval"?

I'm concerned about leaving the wrong impression with the change in comments.

Thanks,

- Greg
Ilya Maximets Aug. 30, 2017, 1:56 p.m. UTC | #2
On 29.08.2017 19:27, Greg Rose wrote:
> On 08/28/2017 12:08 AM, Ilya Maximets wrote:
>> TIME_UPDATE_INTERVAL was removed long time ago.
>> Now each call leads to time update via syscall and it's
>> granularity is system dependent.
>>
>> Fixes: 31ef9f5178de ("timeval: Remove CACHE_TIME scheme.")
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>>   lib/timeval.c | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/timeval.c b/lib/timeval.c
>> index dd63f03..2c7f43a 100644
>> --- a/lib/timeval.c
>> +++ b/lib/timeval.c
>> @@ -171,16 +171,14 @@ time_timespec__(struct clock *c, struct timespec *ts)
>>       }
>>   }
>>
>> -/* Stores a monotonic timer, accurate within TIME_UPDATE_INTERVAL ms, into
>> - * '*ts'. */
>> +/* Stores a monotonic timer into '*ts'. */
>>   void
>>   time_timespec(struct timespec *ts)
>>   {
>>       time_timespec__(&monotonic_clock, ts);
>>   }
>>
>> -/* Stores the current time, accurate within TIME_UPDATE_INTERVAL ms, into
>> - * '*ts'. */
>> +/* Stores the current time into '*ts'. */
>>   void
>>   time_wall_timespec(struct timespec *ts)
>>   {
>> @@ -219,14 +217,14 @@ time_msec__(struct clock *c)
>>       return timespec_to_msec(&ts);
>>   }
>>
>> -/* Returns a monotonic timer, in ms (within TIME_UPDATE_INTERVAL ms). */
>> +/* Returns a monotonic timer, in ms. */
>>   long long int
>>   time_msec(void)
>>   {
>>       return time_msec__(&monotonic_clock);
>>   }
>>
>> -/* Returns the current time, in ms (within TIME_UPDATE_INTERVAL ms). */
>> +/* Returns the current time, in ms. */
>>   long long int
>>   time_wall_msec(void)
>>   {
>>
> How about replaciing "accurate within TIME_UPDATE_INTERVAL ms" with
> "accurate within the system dependent timer interval"?
> 
> I'm concerned about leaving the wrong impression with the change in comments.
> 
> Thanks,
> 
> - Greg

Hi Greg,

Thanks for looking at this, but I don't think that such a comment is useful
because I believe that any system nowadays is able to provide millisecond
granularity.

Best regards, Ilya Maximets.
Ben Pfaff Oct. 25, 2017, 3:56 a.m. UTC | #3
On Mon, Aug 28, 2017 at 10:08:25AM +0300, Ilya Maximets wrote:
> TIME_UPDATE_INTERVAL was removed long time ago.
> Now each call leads to time update via syscall and it's
> granularity is system dependent.
> 
> Fixes: 31ef9f5178de ("timeval: Remove CACHE_TIME scheme.")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>

Thanks, Ilya, I applied this to master.
diff mbox series

Patch

diff --git a/lib/timeval.c b/lib/timeval.c
index dd63f03..2c7f43a 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -171,16 +171,14 @@  time_timespec__(struct clock *c, struct timespec *ts)
     }
 }
 
-/* Stores a monotonic timer, accurate within TIME_UPDATE_INTERVAL ms, into
- * '*ts'. */
+/* Stores a monotonic timer into '*ts'. */
 void
 time_timespec(struct timespec *ts)
 {
     time_timespec__(&monotonic_clock, ts);
 }
 
-/* Stores the current time, accurate within TIME_UPDATE_INTERVAL ms, into
- * '*ts'. */
+/* Stores the current time into '*ts'. */
 void
 time_wall_timespec(struct timespec *ts)
 {
@@ -219,14 +217,14 @@  time_msec__(struct clock *c)
     return timespec_to_msec(&ts);
 }
 
-/* Returns a monotonic timer, in ms (within TIME_UPDATE_INTERVAL ms). */
+/* Returns a monotonic timer, in ms. */
 long long int
 time_msec(void)
 {
     return time_msec__(&monotonic_clock);
 }
 
-/* Returns the current time, in ms (within TIME_UPDATE_INTERVAL ms). */
+/* Returns the current time, in ms. */
 long long int
 time_wall_msec(void)
 {