[v13,04/10] eal/Windows: add clock_gettime on Windows

Message ID 1620241931-28435-5-git-send-email-jizh@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: Andrew Rybchenko
Headers
Series app/testpmd: enable testpmd on Windows |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Jie Zhou May 5, 2021, 7:12 p.m. UTC
  Add clock_gettime on Windows in rte_os_shim.h

Signed-off-by: Jie Zhou <jizh@microsoft.com>
Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>
---
 lib/eal/windows/include/rte_os_shim.h | 38 +++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
  

Comments

Dmitry Kozlyuk June 20, 2021, 11:30 p.m. UTC | #1
2021-05-05 12:12 (UTC-0700), Jie Zhou:
> Add clock_gettime on Windows in rte_os_shim.h
> 
> Signed-off-by: Jie Zhou <jizh@microsoft.com>
> Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>
> ---
>  lib/eal/windows/include/rte_os_shim.h | 38 +++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/lib/eal/windows/include/rte_os_shim.h b/lib/eal/windows/include/rte_os_shim.h
> index 3763cae62..cd1f53dfa 100644
> --- a/lib/eal/windows/include/rte_os_shim.h
> +++ b/lib/eal/windows/include/rte_os_shim.h
> @@ -77,4 +77,42 @@ rte_timespec_get(struct timespec *now, int base)
>  
>  #endif /* RTE_TOOLCHAIN_GCC */
>  
> +/* Identifier for system-wide realtime clock. */
> +#define CLOCK_REALTIME                  0
> +/* Monotonic system-wide clock. */
> +#define CLOCK_MONOTONIC                 1
> +/* High-resolution timer from the CPU. */
> +#define CLOCK_PROCESS_CPUTIME_ID        2
> +/* Thread-specific CPU-time clock. */
> +#define CLOCK_THREAD_CPUTIME_ID         3

Are the last two constants needed?

> +
> +#define NS_PER_SEC 1E9

NS_PER_SEC isn't provided by any interface that we shim,
but it can be defined by applications (like testpmd does),
so it's better to make this constant private to rte_clock_gettime().

IMO, we should provide such constants with RTE_ prefix someday.
rte_time.h provides NSEC_PER_SEC without RTE_ prefix already.

> +
> +typedef int clockid_t;
> +
> +static inline int
> +rte_clock_gettime(clockid_t clock_id, struct timespec *tp)
> +{
> +	LARGE_INTEGER pf, pc;
> +	LONGLONG nsec;
> +	switch (clock_id) {
> +	case CLOCK_REALTIME:
> +		if (timespec_get(tp, TIME_UTC) != TIME_UTC)
> +			return -1;
> +		return 0;
> +	case CLOCK_MONOTONIC:
> +		if (QueryPerformanceFrequency(&pf) == 0)
> +			return -1;
> +		if (QueryPerformanceCounter(&pc) == 0)
> +			return -1;

These calls never fail on any supported Windows version.

> +		nsec = pc.QuadPart * NS_PER_SEC / pf.QuadPart;
> +		tp->tv_sec = nsec / NS_PER_SEC;
> +		tp->tv_nsec = nsec - tp->tv_sec * NS_PER_SEC;
> +		return 0;
> +	default:
> +		return -1;

By clock_getttime() contract, errno must be set to ENOTSUP here.

> +	}
> +}
> +#define clock_gettime(clock_id, tp) rte_clock_gettime(clock_id, tp)
> +
>  #endif /* _RTE_OS_SHIM_ */
  
Jie Zhou June 23, 2021, 8:57 p.m. UTC | #2
On Mon, Jun 21, 2021 at 02:30:36AM +0300, Dmitry Kozlyuk wrote:
> 2021-05-05 12:12 (UTC-0700), Jie Zhou:
> > Add clock_gettime on Windows in rte_os_shim.h
> > 
> > Signed-off-by: Jie Zhou <jizh@microsoft.com>
> > Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>
> > ---
> >  lib/eal/windows/include/rte_os_shim.h | 38 +++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> > 
> > diff --git a/lib/eal/windows/include/rte_os_shim.h b/lib/eal/windows/include/rte_os_shim.h
> > index 3763cae62..cd1f53dfa 100644
> > --- a/lib/eal/windows/include/rte_os_shim.h
> > +++ b/lib/eal/windows/include/rte_os_shim.h
> > @@ -77,4 +77,42 @@ rte_timespec_get(struct timespec *now, int base)
> >  
> >  #endif /* RTE_TOOLCHAIN_GCC */
> >  
> > +/* Identifier for system-wide realtime clock. */
> > +#define CLOCK_REALTIME                  0
> > +/* Monotonic system-wide clock. */
> > +#define CLOCK_MONOTONIC                 1
> > +/* High-resolution timer from the CPU. */
> > +#define CLOCK_PROCESS_CPUTIME_ID        2
> > +/* Thread-specific CPU-time clock. */
> > +#define CLOCK_THREAD_CPUTIME_ID         3
> 
> Are the last two constants needed?

Will remove these two unnecessary ones in V14.

> 
> > +
> > +#define NS_PER_SEC 1E9
> 
> NS_PER_SEC isn't provided by any interface that we shim,
> but it can be defined by applications (like testpmd does),
> so it's better to make this constant private to rte_clock_gettime().

Sure, will move into rte_clock_gettime()


> IMO, we should provide such constants with RTE_ prefix someday.
> rte_time.h provides NSEC_PER_SEC without RTE_ prefix already.
> 
> > +
> > +typedef int clockid_t;
> > +
> > +static inline int
> > +rte_clock_gettime(clockid_t clock_id, struct timespec *tp)
> > +{
> > +	LARGE_INTEGER pf, pc;
> > +	LONGLONG nsec;
> > +	switch (clock_id) {
> > +	case CLOCK_REALTIME:
> > +		if (timespec_get(tp, TIME_UTC) != TIME_UTC)
> > +			return -1;
> > +		return 0;
> > +	case CLOCK_MONOTONIC:
> > +		if (QueryPerformanceFrequency(&pf) == 0)
> > +			return -1;
> > +		if (QueryPerformanceCounter(&pc) == 0)
> > +			return -1;
> 
> These calls never fail on any supported Windows version.

Agree, these two zero check is redundant as per ms document, systems run Windows XP or later will always succeed and will never return zero. Will remove the check.
  

Patch

diff --git a/lib/eal/windows/include/rte_os_shim.h b/lib/eal/windows/include/rte_os_shim.h
index 3763cae62..cd1f53dfa 100644
--- a/lib/eal/windows/include/rte_os_shim.h
+++ b/lib/eal/windows/include/rte_os_shim.h
@@ -77,4 +77,42 @@  rte_timespec_get(struct timespec *now, int base)
 
 #endif /* RTE_TOOLCHAIN_GCC */
 
+/* Identifier for system-wide realtime clock. */
+#define CLOCK_REALTIME                  0
+/* Monotonic system-wide clock. */
+#define CLOCK_MONOTONIC                 1
+/* High-resolution timer from the CPU. */
+#define CLOCK_PROCESS_CPUTIME_ID        2
+/* Thread-specific CPU-time clock. */
+#define CLOCK_THREAD_CPUTIME_ID         3
+
+#define NS_PER_SEC 1E9
+
+typedef int clockid_t;
+
+static inline int
+rte_clock_gettime(clockid_t clock_id, struct timespec *tp)
+{
+	LARGE_INTEGER pf, pc;
+	LONGLONG nsec;
+	switch (clock_id) {
+	case CLOCK_REALTIME:
+		if (timespec_get(tp, TIME_UTC) != TIME_UTC)
+			return -1;
+		return 0;
+	case CLOCK_MONOTONIC:
+		if (QueryPerformanceFrequency(&pf) == 0)
+			return -1;
+		if (QueryPerformanceCounter(&pc) == 0)
+			return -1;
+		nsec = pc.QuadPart * NS_PER_SEC / pf.QuadPart;
+		tp->tv_sec = nsec / NS_PER_SEC;
+		tp->tv_nsec = nsec - tp->tv_sec * NS_PER_SEC;
+		return 0;
+	default:
+		return -1;
+	}
+}
+#define clock_gettime(clock_id, tp) rte_clock_gettime(clock_id, tp)
+
 #endif /* _RTE_OS_SHIM_ */