Re: [dpdk-dev] [PATCH v13 04/10] eal/Windows: add clock_gettime on Windows

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

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_ */