[v2,3/3] eal/windows: fix build by enabling trace compilation

Message ID 20200426152819.2496610-4-dmitry.kozliuk@gmail.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers
Series eal/windows: fix build by enabling trace compilation |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Dmitry Kozlyuk April 26, 2020, 3:28 p.m. UTC
  Replace clock_gettime(CLOCK_REALTIME) with C11 timespec_get().
Implementation is provided for MinGW-w64 that misses this function.

Provide minimum viable implementations of malloc and timer functions
used by tracing. Regex stubs are already present in Windows EAL.

Fixes: 185b7dc1d467 ("trace: save bootup timestamp")
Fixes: 321dd5f8fa62 ("trace: add internal init and fini interface")
Reported-by: Pallavi Kadam <pallavi.kadam@intel.com>
Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 config/meson.build                            |  2 +
 .../common/eal_common_trace_utils.c           |  3 +-
 lib/librte_eal/common/meson.build             |  5 +
 .../include/generic/rte_byteorder.h           |  4 +-
 lib/librte_eal/windows/eal.c                  | 92 +++++++++++++++++++
 lib/librte_eal/windows/eal_thread.c           |  9 ++
 lib/librte_eal/windows/eal_windows.h          |  3 +
 lib/librte_eal/windows/include/rte_os.h       | 33 +++++--
 8 files changed, 141 insertions(+), 10 deletions(-)
  

Comments

Thomas Monjalon April 26, 2020, 3:38 p.m. UTC | #1
I think this patch is doing too many things at once.
Why not just disabling tracing on Windows for now,
and apply proper patches for memory management, timer, endianness, etc
in 20.08?

Some cosmetic comments below,

26/04/2020 17:28, Dmitry Kozlyuk:
> Replace clock_gettime(CLOCK_REALTIME) with C11 timespec_get().
> Implementation is provided for MinGW-w64 that misses this function.
> 
> Provide minimum viable implementations of malloc and timer functions
> used by tracing. Regex stubs are already present in Windows EAL.
> 
> Fixes: 185b7dc1d467 ("trace: save bootup timestamp")
> Fixes: 321dd5f8fa62 ("trace: add internal init and fini interface")
> Reported-by: Pallavi Kadam <pallavi.kadam@intel.com>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

Blank line is required between "Fixes" block and " Reported/SoB".

[...]
> --- a/lib/librte_eal/include/generic/rte_byteorder.h
> +++ b/lib/librte_eal/include/generic/rte_byteorder.h
> -#ifdef RTE_EXEC_ENV_FREEBSD
> +#if defined(RTE_EXEC_ENV_FREEBSD)

No need to change above line.

>  #include <sys/endian.h>
> -#else
> +#elif defined(RTE_EXEC_ENV_LINUX)

Parenthesis are useless.

>  #include <endian.h>
>  #endif
  
Dmitry Kozlyuk April 26, 2020, 4:42 p.m. UTC | #2
On 2020-04-26 17:38 GMT+0200 Thomas Monjalon wrote:
> I think this patch is doing too many things at once.
> Why not just disabling tracing on Windows for now,
> and apply proper patches for memory management, timer, endianness, etc
> in 20.08?

Sounds reasonable since tracing cannot be fully supported anyway and we're
past deadline. Replacing the series with a trivial patch for now.

> Some cosmetic comments below,

Thanks.
  

Patch

diff --git a/config/meson.build b/config/meson.build
index e851b407b..91cba9313 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -267,6 +267,8 @@  if is_windows
 	# Minimum supported API is Windows 7.
 	add_project_arguments('-D_WIN32_WINNT=0x0601', language: 'c')
 
+	add_project_link_arguments(['-lshell32', '-lshlwapi'], language: 'c')
+
 	# Use MinGW-w64 stdio, because DPDK assumes ANSI-compliant formatting.
 	if cc.get_id() == 'gcc'
 		add_project_arguments('-D__USE_MINGW_ANSI_STDIO', language: 'c')
diff --git a/lib/librte_eal/common/eal_common_trace_utils.c b/lib/librte_eal/common/eal_common_trace_utils.c
index 78df3b41e..1fb5bc772 100644
--- a/lib/librte_eal/common/eal_common_trace_utils.c
+++ b/lib/librte_eal/common/eal_common_trace_utils.c
@@ -7,6 +7,7 @@ 
 
 #include <rte_common.h>
 #include <rte_errno.h>
+#include <rte_os.h>
 #include <rte_string_fns.h>
 
 #include "eal_filesystem.h"
@@ -300,7 +301,7 @@  trace_epoch_time_save(void)
 	uint64_t avg, start, end;
 
 	start = rte_get_tsc_cycles();
-	if (clock_gettime(CLOCK_REALTIME, &epoch) < 0) {
+	if (timespec_get(&epoch, TIME_UTC) < 0) {
 		trace_err("failed to get the epoch time");
 		return -1;
 	}
diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
index 155da29b4..f7393b8c3 100644
--- a/lib/librte_eal/common/meson.build
+++ b/lib/librte_eal/common/meson.build
@@ -14,6 +14,11 @@  if is_windows
 		'eal_common_log.c',
 		'eal_common_options.c',
 		'eal_common_thread.c',
+		'eal_common_trace.c',
+		'eal_common_trace_ctf.c',
+		'eal_common_trace_utils.c',
+		'eal_common_string_fns.c',
+		'eal_common_uuid.c',
 		'rte_option.c',
 	)
 	subdir_done()
diff --git a/lib/librte_eal/include/generic/rte_byteorder.h b/lib/librte_eal/include/generic/rte_byteorder.h
index 38e8cfd32..bc3ad8e23 100644
--- a/lib/librte_eal/include/generic/rte_byteorder.h
+++ b/lib/librte_eal/include/generic/rte_byteorder.h
@@ -15,9 +15,9 @@ 
  */
 
 #include <stdint.h>
-#ifdef RTE_EXEC_ENV_FREEBSD
+#if defined(RTE_EXEC_ENV_FREEBSD)
 #include <sys/endian.h>
-#else
+#elif defined(RTE_EXEC_ENV_LINUX)
 #include <endian.h>
 #endif
 
diff --git a/lib/librte_eal/windows/eal.c b/lib/librte_eal/windows/eal.c
index 2cf7a04ef..fec7e5001 100644
--- a/lib/librte_eal/windows/eal.c
+++ b/lib/librte_eal/windows/eal.c
@@ -17,6 +17,7 @@ 
 #include <eal_filesystem.h>
 #include <eal_options.h>
 #include <eal_private.h>
+#include <eal_trace.h>
 
 #include "eal_windows.h"
 
@@ -208,6 +209,91 @@  eal_parse_args(int argc, char **argv)
 	return ret;
 }
 
+void *
+eal_malloc_no_trace(const char *type, size_t size, unsigned int align)
+{
+	/* Simulate failure, so that tracing falls back to malloc(). */
+	RTE_SET_USED(type);
+	RTE_SET_USED(size);
+	RTE_SET_USED(align);
+	return NULL;
+}
+
+void
+eal_free_no_trace(void *addr __rte_unused)
+{
+	/* Nothing to free. */
+}
+
+/* MinGW-w64 does not implement timespec_get(). */
+#ifdef RTE_TOOLCHAIN_GCC
+
+int
+timespec_get(struct timespec *ts, int base)
+{
+	static const uint64_t UNITS_PER_SEC = 10000000; /* 1 unit = 100 ns */
+
+	FILETIME ft;
+	ULARGE_INTEGER ui;
+
+	GetSystemTimePreciseAsFileTime(&ft);
+	ui.LowPart = ft.dwLowDateTime;
+	ui.HighPart = ft.dwHighDateTime;
+	ts->tv_sec = ui.QuadPart / UNITS_PER_SEC;
+	ts->tv_nsec = ui.QuadPart - (ts->tv_sec * UNITS_PER_SEC);
+	return base;
+}
+
+#endif /* RTE_TOOLCHAIN_GCC */
+
+uint64_t
+rte_get_tsc_hz(void)
+{
+	static LARGE_INTEGER freq; /* static so auto-zeroed */
+
+	if (freq.QuadPart != 0)
+		return freq.QuadPart;
+
+	QueryPerformanceFrequency(&freq);
+	return freq.QuadPart;
+}
+
+const char *
+eal_permanent_data_path(void)
+{
+	static char buffer[PATH_MAX]; /* static so auto-zeroed */
+
+	HRESULT ret;
+
+	if (buffer[0] != '\0')
+		return buffer;
+
+	ret = SHGetFolderPathA(
+		NULL, CSIDL_LOCAL_APPDATA, NULL, SHGFP_TYPE_CURRENT, buffer);
+	if (FAILED(ret)) {
+		RTE_LOG_WIN32_ERR("SHGetFolderPathA(CSIDL_LOCAL_APPDATA)");
+		return NULL;
+	}
+
+	return buffer;
+}
+
+int
+eal_dir_create(const char *path)
+{
+	/* CreateDirectoryA() fails if directory exists. */
+	if (PathIsDirectoryA(path))
+		return 0;
+
+	/* Default ACL already restricts access to creator and owner only. */
+	if (!CreateDirectoryA(path, NULL)) {
+		RTE_LOG_WIN32_ERR("CreateDirectoryA(\"%s\", NULL)", path);
+		rte_errno = EINVAL;
+		return -1;
+	}
+	return 0;
+}
+
 static int
 sync_func(void *arg __rte_unused)
 {
@@ -242,6 +328,12 @@  rte_eal_init(int argc, char **argv)
 	if (fctret < 0)
 		exit(1);
 
+	if (eal_trace_init() < 0) {
+		rte_eal_init_alert("Cannot init trace");
+		rte_errno = EFAULT;
+		return -1;
+	}
+
 	eal_thread_init_master(rte_config.master_lcore);
 
 	RTE_LCORE_FOREACH_SLAVE(i) {
diff --git a/lib/librte_eal/windows/eal_thread.c b/lib/librte_eal/windows/eal_thread.c
index e149199a6..9feb2bfd0 100644
--- a/lib/librte_eal/windows/eal_thread.c
+++ b/lib/librte_eal/windows/eal_thread.c
@@ -157,6 +157,15 @@  eal_thread_create(pthread_t *thread)
 	return 0;
 }
 
+int
+rte_thread_getname(pthread_t id, char *name, size_t len)
+{
+	RTE_SET_USED(id);
+	RTE_SET_USED(name);
+	RTE_SET_USED(len);
+	return -ENOTSUP;
+}
+
 int
 rte_thread_setname(__rte_unused pthread_t id, __rte_unused const char *name)
 {
diff --git a/lib/librte_eal/windows/eal_windows.h b/lib/librte_eal/windows/eal_windows.h
index fadd676b2..f259fdc53 100644
--- a/lib/librte_eal/windows/eal_windows.h
+++ b/lib/librte_eal/windows/eal_windows.h
@@ -11,6 +11,9 @@ 
 
 #include <rte_windows.h>
 
+#include <shlobj.h>
+#include <shlwapi.h>
+
 /**
  * Create a map of processors and cores on the system.
  */
diff --git a/lib/librte_eal/windows/include/rte_os.h b/lib/librte_eal/windows/include/rte_os.h
index 510e39e03..6f4333eb5 100644
--- a/lib/librte_eal/windows/include/rte_os.h
+++ b/lib/librte_eal/windows/include/rte_os.h
@@ -46,15 +46,13 @@  extern "C" {
 typedef long long ssize_t;
 
 #ifndef RTE_TOOLCHAIN_GCC
+
 static inline int
-asprintf(char **buffer, const char *format, ...)
+vasprintf(char **buffer, const char *format, va_list arg)
 {
 	int size, ret;
-	va_list arg;
 
-	va_start(arg, format);
 	size = vsnprintf(NULL, 0, format, arg);
-	va_end(arg);
 	if (size < 0)
 		return -1;
 	size++;
@@ -63,16 +61,37 @@  asprintf(char **buffer, const char *format, ...)
 	if (*buffer == NULL)
 		return -1;
 
-	va_start(arg, format);
 	ret = vsnprintf(*buffer, size, format, arg);
-	va_end(arg);
 	if (ret != size - 1) {
 		free(*buffer);
 		return -1;
 	}
 	return ret;
 }
-#endif /* RTE_TOOLCHAIN_GCC */
+
+static inline int
+asprintf(char **buffer, const char *format, ...)
+{
+	int ret;
+	va_list arg;
+
+	va_start(arg, format);
+	ret = vasprintf(buffer, format, arg);
+	va_end(arg);
+
+	return ret;
+}
+
+#else /* RTE_TOOLCHAIN_GCC */
+
+/* value as in time.h from UCRT */
+#define TIME_UTC 1
+
+struct timespec;
+
+int timespec_get(struct timespec *ts, int base);
+
+#endif /* !RTE_TOOLCHAIN_GCC */
 
 #ifdef __cplusplus
 }