Re: [Mesa-dev] [PATCH] gallium/os: use CLOCK_MONOTONIC for sleeps (v2)

2016-07-20 Thread Eric Engestrom 
On Tue, Jul 19, 2016 at 06:29:20PM +0200, Marek Olšák wrote:
> From: Marek Olšák 
> 
> v2: handle EINTR, remove backslashes

I was a bit surprised by the way you use clock_nanosleep(), but
I checked the man and you're right :)
Reviewed-by: Eric Engestrom 

> ---
>  src/gallium/auxiliary/os/os_time.c | 16 ++--
>  src/gallium/auxiliary/os/os_time.h |  4 
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/os/os_time.c 
> b/src/gallium/auxiliary/os/os_time.c
> index 3d2e416..e169139 100644
> --- a/src/gallium/auxiliary/os/os_time.c
> +++ b/src/gallium/auxiliary/os/os_time.c
> @@ -40,6 +40,7 @@
>  #  include  /* timeval */
>  #  include  /* timeval */
>  #  include  /* sched_yield */
> +#  include 
>  #elif defined(PIPE_SUBSYSTEM_WINDOWS_USER)
>  #  include 
>  #else
> @@ -81,19 +82,30 @@ os_time_get_nano(void)
>  }
>  
>  
> -#if defined(PIPE_SUBSYSTEM_WINDOWS_USER)
>  
>  void
>  os_time_sleep(int64_t usecs)
>  {
> +#if defined(PIPE_OS_LINUX)
> +   struct timespec time;
> +   time.tv_sec = usecs / 100;
> +   time.tv_nsec = (usecs % 100) * 1000;
> +   while (clock_nanosleep(CLOCK_MONOTONIC, 0, &time, &time) == EINTR);
> +
> +#elif defined(PIPE_OS_UNIX)
> +   usleep(usecs);
> +
> +#elif defined(PIPE_SUBSYSTEM_WINDOWS_USER)
> DWORD dwMilliseconds = (DWORD) ((usecs + 999) / 1000);
> /* Avoid Sleep(O) as that would cause to sleep for an undetermined 
> duration */
> if (dwMilliseconds) {
>Sleep(dwMilliseconds);
> }
> +#else
> +#  error Unsupported OS
> +#endif
>  }
>  
> -#endif
>  
>  
>  int64_t
> diff --git a/src/gallium/auxiliary/os/os_time.h 
> b/src/gallium/auxiliary/os/os_time.h
> index 9312e02..ca0bdd5 100644
> --- a/src/gallium/auxiliary/os/os_time.h
> +++ b/src/gallium/auxiliary/os/os_time.h
> @@ -70,12 +70,8 @@ os_time_get(void)
>  /*
>   * Sleep.
>   */
> -#if defined(PIPE_OS_UNIX)
> -#define os_time_sleep(_usecs) usleep(_usecs)
> -#else
>  void
>  os_time_sleep(int64_t usecs);
> -#endif
>  
>  
>  /*
> -- 
> 2.7.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] gallium/os: use CLOCK_MONOTONIC for sleeps (v2)

2016-07-19 Thread Marek Olšák 
From: Marek Olšák 

v2: handle EINTR, remove backslashes
---
 src/gallium/auxiliary/os/os_time.c | 16 ++--
 src/gallium/auxiliary/os/os_time.h |  4 
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/gallium/auxiliary/os/os_time.c 
b/src/gallium/auxiliary/os/os_time.c
index 3d2e416..e169139 100644
--- a/src/gallium/auxiliary/os/os_time.c
+++ b/src/gallium/auxiliary/os/os_time.c
@@ -40,6 +40,7 @@
 #  include  /* timeval */
 #  include  /* timeval */
 #  include  /* sched_yield */
+#  include 
 #elif defined(PIPE_SUBSYSTEM_WINDOWS_USER)
 #  include 
 #else
@@ -81,19 +82,30 @@ os_time_get_nano(void)
 }
 
 
-#if defined(PIPE_SUBSYSTEM_WINDOWS_USER)
 
 void
 os_time_sleep(int64_t usecs)
 {
+#if defined(PIPE_OS_LINUX)
+   struct timespec time;
+   time.tv_sec = usecs / 100;
+   time.tv_nsec = (usecs % 100) * 1000;
+   while (clock_nanosleep(CLOCK_MONOTONIC, 0, &time, &time) == EINTR);
+
+#elif defined(PIPE_OS_UNIX)
+   usleep(usecs);
+
+#elif defined(PIPE_SUBSYSTEM_WINDOWS_USER)
DWORD dwMilliseconds = (DWORD) ((usecs + 999) / 1000);
/* Avoid Sleep(O) as that would cause to sleep for an undetermined duration 
*/
if (dwMilliseconds) {
   Sleep(dwMilliseconds);
}
+#else
+#  error Unsupported OS
+#endif
 }
 
-#endif
 
 
 int64_t
diff --git a/src/gallium/auxiliary/os/os_time.h 
b/src/gallium/auxiliary/os/os_time.h
index 9312e02..ca0bdd5 100644
--- a/src/gallium/auxiliary/os/os_time.h
+++ b/src/gallium/auxiliary/os/os_time.h
@@ -70,12 +70,8 @@ os_time_get(void)
 /*
  * Sleep.
  */
-#if defined(PIPE_OS_UNIX)
-#define os_time_sleep(_usecs) usleep(_usecs)
-#else
 void
 os_time_sleep(int64_t usecs);
-#endif
 
 
 /*
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium/os: use CLOCK_MONOTONIC for sleeps

2016-07-19 Thread Nicolai Hähnle 

With Eric's backslash comment addressed:

Reviewed-by: Nicolai Hähnle 

On 18.07.2016 14:14, Marek Olšák wrote:

From: Marek Olšák 

---
  src/gallium/auxiliary/os/os_time.c | 15 +--
  src/gallium/auxiliary/os/os_time.h |  4 
  2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/gallium/auxiliary/os/os_time.c 
b/src/gallium/auxiliary/os/os_time.c
index 3d2e416..633ee3d 100644
--- a/src/gallium/auxiliary/os/os_time.c
+++ b/src/gallium/auxiliary/os/os_time.c
@@ -81,19 +81,30 @@ os_time_get_nano(void)
  }


-#if defined(PIPE_SUBSYSTEM_WINDOWS_USER)

  void
  os_time_sleep(int64_t usecs)
  {
+#if defined(PIPE_OS_LINUX)
+   struct timespec time; \
+   time.tv_sec = usecs / 100; \
+   time.tv_nsec = (usecs % 100) * 1000; \
+   clock_nanosleep(CLOCK_MONOTONIC, 0, &time, NULL); \
+
+#elif defined(PIPE_OS_UNIX)
+   usleep(usecs);
+
+#elif defined(PIPE_SUBSYSTEM_WINDOWS_USER)
 DWORD dwMilliseconds = (DWORD) ((usecs + 999) / 1000);
 /* Avoid Sleep(O) as that would cause to sleep for an undetermined 
duration */
 if (dwMilliseconds) {
Sleep(dwMilliseconds);
 }
+#else
+#  error Unsupported OS
+#endif
  }

-#endif


  int64_t
diff --git a/src/gallium/auxiliary/os/os_time.h 
b/src/gallium/auxiliary/os/os_time.h
index 9312e02..ca0bdd5 100644
--- a/src/gallium/auxiliary/os/os_time.h
+++ b/src/gallium/auxiliary/os/os_time.h
@@ -70,12 +70,8 @@ os_time_get(void)
  /*
   * Sleep.
   */
-#if defined(PIPE_OS_UNIX)
-#define os_time_sleep(_usecs) usleep(_usecs)
-#else
  void
  os_time_sleep(int64_t usecs);
-#endif


  /*


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium/os: use CLOCK_MONOTONIC for sleeps

2016-07-18 Thread Grazvydas Ignotas 
On Mon, Jul 18, 2016 at 3:14 PM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> ---
>  src/gallium/auxiliary/os/os_time.c | 15 +--
>  src/gallium/auxiliary/os/os_time.h |  4 
>  2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/src/gallium/auxiliary/os/os_time.c 
> b/src/gallium/auxiliary/os/os_time.c
> index 3d2e416..633ee3d 100644
> --- a/src/gallium/auxiliary/os/os_time.c
> +++ b/src/gallium/auxiliary/os/os_time.c
> @@ -81,19 +81,30 @@ os_time_get_nano(void)
>  }
>
>
> -#if defined(PIPE_SUBSYSTEM_WINDOWS_USER)
>
>  void
>  os_time_sleep(int64_t usecs)
>  {
> +#if defined(PIPE_OS_LINUX)
> +   struct timespec time; \
> +   time.tv_sec = usecs / 100; \
> +   time.tv_nsec = (usecs % 100) * 1000; \
> +   clock_nanosleep(CLOCK_MONOTONIC, 0, &time, NULL); \

Maybe handle EINTR while you're at it?

Gražvydas
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium/os: use CLOCK_MONOTONIC for sleeps

2016-07-18 Thread Marek Olšák 
On Mon, Jul 18, 2016 at 2:38 PM, Eric Engestrom
 wrote:
> On Mon, Jul 18, 2016 at 02:14:49PM +0200, Marek Olšák wrote:
>> From: Marek Olšák 
>>
>> ---
>>  src/gallium/auxiliary/os/os_time.c | 15 +--
>>  src/gallium/auxiliary/os/os_time.h |  4 
>>  2 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/os/os_time.c 
>> b/src/gallium/auxiliary/os/os_time.c
>> index 3d2e416..633ee3d 100644
>> --- a/src/gallium/auxiliary/os/os_time.c
>> +++ b/src/gallium/auxiliary/os/os_time.c
>> @@ -81,19 +81,30 @@ os_time_get_nano(void)
>>  }
>>
>>
>> -#if defined(PIPE_SUBSYSTEM_WINDOWS_USER)
>>
>>  void
>>  os_time_sleep(int64_t usecs)
>>  {
>> +#if defined(PIPE_OS_LINUX)
>> +   struct timespec time; \
>> +   time.tv_sec = usecs / 100; \
>> +   time.tv_nsec = (usecs % 100) * 1000; \
>> +   clock_nanosleep(CLOCK_MONOTONIC, 0, &time, NULL); \
>
> This is an actual function, not a macro; the backslashes are unnecessary :)
> This is a good change though, so with those removed:
> Reviewed-by: Eric Engestrom 

Thanks.

>
> One (kind of unrelated) thing, though:
> Does a negative sleep really make sense? Shouldn't the param be `uint64_t`?
> I know on Linux and Unix it doesn't.
> If there a reason to keep it signed, maybe a check like this could be
> added:
> if (usecs < 0) return;

The time in Gallium is always int64_t. clock_nanosleep will fail if
tv_nsec is negative. Thus, a negative time has no effect.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] gallium/os: use CLOCK_MONOTONIC for sleeps

2016-07-18 Thread Eric Engestrom 
On Mon, Jul 18, 2016 at 02:14:49PM +0200, Marek Olšák wrote:
> From: Marek Olšák 
> 
> ---
>  src/gallium/auxiliary/os/os_time.c | 15 +--
>  src/gallium/auxiliary/os/os_time.h |  4 
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/os/os_time.c 
> b/src/gallium/auxiliary/os/os_time.c
> index 3d2e416..633ee3d 100644
> --- a/src/gallium/auxiliary/os/os_time.c
> +++ b/src/gallium/auxiliary/os/os_time.c
> @@ -81,19 +81,30 @@ os_time_get_nano(void)
>  }
>  
>  
> -#if defined(PIPE_SUBSYSTEM_WINDOWS_USER)
>  
>  void
>  os_time_sleep(int64_t usecs)
>  {
> +#if defined(PIPE_OS_LINUX)
> +   struct timespec time; \
> +   time.tv_sec = usecs / 100; \
> +   time.tv_nsec = (usecs % 100) * 1000; \
> +   clock_nanosleep(CLOCK_MONOTONIC, 0, &time, NULL); \

This is an actual function, not a macro; the backslashes are unnecessary :)
This is a good change though, so with those removed:
Reviewed-by: Eric Engestrom 

One (kind of unrelated) thing, though:
Does a negative sleep really make sense? Shouldn't the param be `uint64_t`?
I know on Linux and Unix it doesn't.
If there a reason to keep it signed, maybe a check like this could be
added:
if (usecs < 0) return;

(That'd be a separate patch anyway)

> +
> +#elif defined(PIPE_OS_UNIX)
> +   usleep(usecs);
> +
> +#elif defined(PIPE_SUBSYSTEM_WINDOWS_USER)
> DWORD dwMilliseconds = (DWORD) ((usecs + 999) / 1000);
> /* Avoid Sleep(O) as that would cause to sleep for an undetermined 
> duration */
> if (dwMilliseconds) {
>Sleep(dwMilliseconds);
> }
> +#else
> +#  error Unsupported OS
> +#endif
>  }
>  
> -#endif
>  
>  
>  int64_t
> diff --git a/src/gallium/auxiliary/os/os_time.h 
> b/src/gallium/auxiliary/os/os_time.h
> index 9312e02..ca0bdd5 100644
> --- a/src/gallium/auxiliary/os/os_time.h
> +++ b/src/gallium/auxiliary/os/os_time.h
> @@ -70,12 +70,8 @@ os_time_get(void)
>  /*
>   * Sleep.
>   */
> -#if defined(PIPE_OS_UNIX)
> -#define os_time_sleep(_usecs) usleep(_usecs)
> -#else
>  void
>  os_time_sleep(int64_t usecs);
> -#endif
>  
>  
>  /*
> -- 
> 2.7.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] gallium/os: use CLOCK_MONOTONIC for sleeps

2016-07-18 Thread Marek Olšák 
From: Marek Olšák 

---
 src/gallium/auxiliary/os/os_time.c | 15 +--
 src/gallium/auxiliary/os/os_time.h |  4 
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/gallium/auxiliary/os/os_time.c 
b/src/gallium/auxiliary/os/os_time.c
index 3d2e416..633ee3d 100644
--- a/src/gallium/auxiliary/os/os_time.c
+++ b/src/gallium/auxiliary/os/os_time.c
@@ -81,19 +81,30 @@ os_time_get_nano(void)
 }
 
 
-#if defined(PIPE_SUBSYSTEM_WINDOWS_USER)
 
 void
 os_time_sleep(int64_t usecs)
 {
+#if defined(PIPE_OS_LINUX)
+   struct timespec time; \
+   time.tv_sec = usecs / 100; \
+   time.tv_nsec = (usecs % 100) * 1000; \
+   clock_nanosleep(CLOCK_MONOTONIC, 0, &time, NULL); \
+
+#elif defined(PIPE_OS_UNIX)
+   usleep(usecs);
+
+#elif defined(PIPE_SUBSYSTEM_WINDOWS_USER)
DWORD dwMilliseconds = (DWORD) ((usecs + 999) / 1000);
/* Avoid Sleep(O) as that would cause to sleep for an undetermined duration 
*/
if (dwMilliseconds) {
   Sleep(dwMilliseconds);
}
+#else
+#  error Unsupported OS
+#endif
 }
 
-#endif
 
 
 int64_t
diff --git a/src/gallium/auxiliary/os/os_time.h 
b/src/gallium/auxiliary/os/os_time.h
index 9312e02..ca0bdd5 100644
--- a/src/gallium/auxiliary/os/os_time.h
+++ b/src/gallium/auxiliary/os/os_time.h
@@ -70,12 +70,8 @@ os_time_get(void)
 /*
  * Sleep.
  */
-#if defined(PIPE_OS_UNIX)
-#define os_time_sleep(_usecs) usleep(_usecs)
-#else
 void
 os_time_sleep(int64_t usecs);
-#endif
 
 
 /*
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev