Merge branch 'check-instr' into 'master'

Add configurable switch for instrumentation

See merge request nos-v/nos-v!63

Makefile.am

@@ -35,6 +35,7 @@ pkgconfig_DATA = nos-v.pc
 
 libnosv_la_SOURCES = \
     src/bootstrap.c		\
+    src/instr.c		\
 	src/config/config.c \
 	src/config/toml.c \
 	src/hardware/cpus.c \

nosv.toml

@@ -1,6 +1,6 @@
 # This file is part of nOS-V and is licensed under the terms contained in the COPYING file
 #
-# Copyright (C) 2021-2022 Barcelona Supercomputing Center (BSC)
+# Copyright (C) 2021-2023 Barcelona Supercomputing Center (BSC)
 
 # Default nOS-V configuration file.
 # Note that every process in the same nOS-V instance should load an identical configuration file,
@@ -124,3 +124,8 @@
 	enabled = false
 	# Whether to print verbose information if monitoring is enabled
 	verbose = false
+
+# Instrumentation. Only applies if nOS-V was configured with --with-ovni
+[instrumentation]
+	# Select the instrumentation to be used. Current options are "none" or "ovni".
+	version = "none"

src/config/config.c

@@ -1,7 +1,7 @@
 /*
 	This file is part of Nanos6 and nOS-V and is licensed under the terms contained in the COPYING file.
 
-	Copyright (C) 2020-2022 Barcelona Supercomputing Center (BSC)
+	Copyright (C) 2020-2023 Barcelona Supercomputing Center (BSC)
 */
 
 #include <assert.h>
@@ -84,6 +84,8 @@ static inline void config_init(rt_config_t *config)
 
 	config->monitoring_enabled = 0;
 	config->monitoring_verbose = 0;
+
+	config->instrumentation_version = strdup(INSTR_DEFAULT_VERSION);
 }
 
 #define sanity_check(cond, explanation)                             \
@@ -130,6 +132,8 @@ static inline int config_check(rt_config_t *config)
 
 	sanity_check_str(config->hwcounters_backend, "Currently available hardware counter backends: papi, none", "none", "papi");
 
+	sanity_check_str(config->instrumentation_version, "Currently available instrumentations: ovni, none", "ovni", "none");
+
 	return ret;
 }
 

src/include/config/config.h

@@ -1,7 +1,7 @@
 /*
 	This file is part of Nanos6 and nOS-V and is licensed under the terms contained in the COPYING file.
 
-	Copyright (C) 2020-2022 Barcelona Supercomputing Center (BSC)
+	Copyright (C) 2020-2023 Barcelona Supercomputing Center (BSC)
 */
 
 #ifndef CONFIG_H
@@ -77,6 +77,9 @@ typedef struct rt_config {
 	// Monitoring
 	int monitoring_enabled;
 	int monitoring_verbose;
+
+	// Instrumentation
+	const char *instrumentation_version;
 } rt_config_t;
 
 __internal extern rt_config_t nosv_config;

src/include/config/configspec.h

@@ -1,7 +1,7 @@
 /*
 	This file is part of Nanos6 and nOS-V and is licensed under the terms contained in the COPYING file.
 
-	Copyright (C) 2020-2022 Barcelona Supercomputing Center (BSC)
+	Copyright (C) 2020-2023 Barcelona Supercomputing Center (BSC)
 */
 
 #ifndef CONFIG_PARSE_H
@@ -83,6 +83,7 @@ static config_spec_t config_spec_list[] = {
 	DECLARE_CONFIG(TYPE_BOOL, "turbo.enabled", turbo_enabled),
 	DECLARE_CONFIG(TYPE_BOOL, "monitoring.enabled", monitoring_enabled),
 	DECLARE_CONFIG(TYPE_BOOL, "monitoring.verbose", monitoring_verbose),
+	DECLARE_CONFIG(TYPE_STR, "instrumentation.version", instrumentation_version),
 };
 
 #endif // CONFIG_PARSE_H

src/include/defaults.h

@@ -68,4 +68,7 @@
 // The enabled HWCounter backend, "none" by default
 #define HWCOUNTERS_BACKEND "none"
 
+// Enabled instrumentation. None by default
+#define INSTR_DEFAULT_VERSION "none"
+
 #endif // DEFAULTS_H

src/include/instr.h

@@ -22,69 +22,80 @@
 
 #include "common.h"
 #include "compat.h"
+#include "config/config.h"
 #include "hardware/pids.h"
 #include "hardware/threads.h"
 #include "nosv-internal.h"
 
 #ifdef ENABLE_INSTRUMENTATION
-#define INSTR_0ARG(name, mcv)              \
-	static inline void name(void)      \
-	{                                  \
-		struct ovni_ev ev = {0};   \
+__internal extern int instr_ovni_enabled;
+
+#define CHECK_INSTR_ENABLED 			\
+	if (!instr_ovni_enabled)     \
+		return;
+
+#define INSTR_0ARG(name, mcv)                     \
+	static inline void name(void)                 \
+	{                                             \
+		CHECK_INSTR_ENABLED                       \
+		struct ovni_ev ev = {0};                  \
 		ovni_ev_set_clock(&ev, ovni_clock_now()); \
-		ovni_ev_set_mcv(&ev, mcv); \
-		ovni_ev_emit(&ev);              \
+		ovni_ev_set_mcv(&ev, mcv);                \
+		ovni_ev_emit(&ev);                        \
 	}
 
-#define INSTR_1ARG(name, mcv, ta, a)                             \
-	static inline void name(ta a)                            \
-	{                                                        \
-		struct ovni_ev ev = {0};                         \
-		ovni_ev_set_clock(&ev, ovni_clock_now());        \
-		ovni_ev_set_mcv(&ev, mcv);                       \
-		ovni_payload_add(&ev, (uint8_t *)&a, sizeof(a)); \
-		ovni_ev_emit(&ev);                                    \
+#define INSTR_1ARG(name, mcv, ta, a)                      \
+	static inline void name(ta a)                         \
+	{                                                     \
+		CHECK_INSTR_ENABLED                               \
+		struct ovni_ev ev = {0};                          \
+		ovni_ev_set_clock(&ev, ovni_clock_now());         \
+		ovni_ev_set_mcv(&ev, mcv);                        \
+		ovni_payload_add(&ev, (uint8_t *) &a, sizeof(a)); \
+		ovni_ev_emit(&ev);                                \
 	}
 
-#define INSTR_2ARG(name, mcv, ta, a, tb, b)                      \
-	static inline void name(ta a, tb b)                      \
-	{                                                        \
-		struct ovni_ev ev = {0};                         \
-		ovni_ev_set_clock(&ev, ovni_clock_now());        \
-		ovni_ev_set_mcv(&ev, mcv);                       \
-		ovni_payload_add(&ev, (uint8_t *)&a, sizeof(a)); \
-		ovni_payload_add(&ev, (uint8_t *)&b, sizeof(b)); \
-		ovni_ev_emit(&ev);                                    \
+#define INSTR_2ARG(name, mcv, ta, a, tb, b)               \
+	static inline void name(ta a, tb b)                   \
+	{                                                     \
+		CHECK_INSTR_ENABLED                               \
+		struct ovni_ev ev = {0};                          \
+		ovni_ev_set_clock(&ev, ovni_clock_now());         \
+		ovni_ev_set_mcv(&ev, mcv);                        \
+		ovni_payload_add(&ev, (uint8_t *) &a, sizeof(a)); \
+		ovni_payload_add(&ev, (uint8_t *) &b, sizeof(b)); \
+		ovni_ev_emit(&ev);                                \
 	}
 
-#define INSTR_3ARG(name, mcv, ta, a, tb, b, tc, c)               \
-	static inline void name(ta a, tb b, tc c)                \
-	{                                                        \
-		struct ovni_ev ev = {0};                         \
-		ovni_ev_set_clock(&ev, ovni_clock_now());        \
-		ovni_ev_set_mcv(&ev, mcv);                       \
-		ovni_payload_add(&ev, (uint8_t *)&a, sizeof(a)); \
-		ovni_payload_add(&ev, (uint8_t *)&b, sizeof(b)); \
-		ovni_payload_add(&ev, (uint8_t *)&c, sizeof(c)); \
-		ovni_ev_emit(&ev);                                    \
+#define INSTR_3ARG(name, mcv, ta, a, tb, b, tc, c)        \
+	static inline void name(ta a, tb b, tc c)             \
+	{                                                     \
+		CHECK_INSTR_ENABLED                               \
+		struct ovni_ev ev = {0};                          \
+		ovni_ev_set_clock(&ev, ovni_clock_now());         \
+		ovni_ev_set_mcv(&ev, mcv);                        \
+		ovni_payload_add(&ev, (uint8_t *) &a, sizeof(a)); \
+		ovni_payload_add(&ev, (uint8_t *) &b, sizeof(b)); \
+		ovni_payload_add(&ev, (uint8_t *) &c, sizeof(c)); \
+		ovni_ev_emit(&ev);                                \
 	}
 #else // ENABLE_INSTRUMENTATION
-#define INSTR_0ARG(name, mcv)         \
+#define INSTR_0ARG(name, mcv)     \
 	static inline void name(void) \
 	{                             \
 	}
 
-#define INSTR_1ARG(name, mcv, ta, a)     \
+#define INSTR_1ARG(name, mcv, ta, a) \
 	static inline void name(ta a)    \
 	{                                \
 	}
 
-#define INSTR_2ARG(name, mcv, ta, a, tb, b)     \
+#define INSTR_2ARG(name, mcv, ta, a, tb, b) \
 	static inline void name(ta a, tb b)     \
 	{                                       \
 	}
 
-#define INSTR_3ARG(name, mcv, ta, a, tb, b, tc, c)     \
+#define INSTR_3ARG(name, mcv, ta, a, tb, b, tc, c) \
 	static inline void name(ta a, tb b, tc c)      \
 	{                                              \
 	}
@@ -135,6 +146,7 @@ INSTR_1ARG(instr_task_end, "VTe", uint32_t, task_id)
 // A jumbo event is needed to encode a large label
 static inline void instr_type_create(uint32_t typeid, const char *label)
 {
+	CHECK_INSTR_ENABLED
 	size_t bufsize, label_len, size_left;
 	uint8_t buf[1024], *p;
 
@@ -203,11 +215,13 @@ INSTR_0ARG(instr_thread_warm, "OHw")
 
 static inline void instr_cpu_id(int index, int phyid)
 {
+	CHECK_INSTR_ENABLED
 	ovni_add_cpu(index, phyid);
 }
 
 static inline void instr_thread_end(void)
 {
+	CHECK_INSTR_ENABLED
 	struct ovni_ev ev = {0};
 
 	ovni_ev_set_clock(&ev, ovni_clock_now());
@@ -220,6 +234,7 @@ static inline void instr_thread_end(void)
 
 static inline void instr_proc_init(const char *suffix)
 {
+	CHECK_INSTR_ENABLED
 	char hostname[HOST_NAME_MAX + 1];
 	int appid;
 	char *appid_str;
@@ -252,22 +267,26 @@ static inline void instr_proc_init(const char *suffix)
 
 static inline void instr_proc_fini(void)
 {
+	CHECK_INSTR_ENABLED
 	ovni_proc_fini();
 }
 
 static inline void instr_gen_bursts(void)
 {
+	CHECK_INSTR_ENABLED
 	for (int i = 0; i < 100; i++)
 		instr_burst();
 }
 
 static inline void instr_thread_init(void)
 {
+	CHECK_INSTR_ENABLED
 	ovni_thread_init(gettid());
 }
 
 static inline void instr_thread_attach(void)
 {
+	CHECK_INSTR_ENABLED
 	struct ovni_ev ev = {0};
 
 	if (!ovni_thread_isready())
@@ -280,6 +299,7 @@ static inline void instr_thread_attach(void)
 
 static inline void instr_thread_detach(void)
 {
+	CHECK_INSTR_ENABLED
 	struct ovni_ev ev = {0};
 
 	ovni_ev_set_clock(&ev, ovni_clock_now());
@@ -345,13 +365,14 @@ struct kinstr;
 
 #ifdef ENABLE_INSTRUMENTATION
 static int perf_event_open(struct perf_event_attr *attr, pid_t pid,
-		int cpu, int group_fd, unsigned long flags)
+	int cpu, int group_fd, unsigned long flags)
 {
 	return (int) syscall(SYS_perf_event_open, attr, pid, cpu, group_fd, flags);
 }
 
 static inline void instr_kernel_init(struct kinstr **ki_ptr)
 {
+	CHECK_INSTR_ENABLED
 	struct perf_event_attr attr;
 	struct kinstr *ki;
 	long pagesize;
@@ -403,7 +424,7 @@ static inline void instr_kernel_init(struct kinstr **ki_ptr)
 	// Map the buffer: must be 1+2^n pages
 	ki->bufsize = pagesize + ki->ringsize;
 	ki->buf = mmap(NULL, ki->bufsize,
-			PROT_READ | PROT_WRITE, MAP_SHARED, ki->fd, 0);
+		PROT_READ | PROT_WRITE, MAP_SHARED, ki->fd, 0);
 
 	if (ki->buf == MAP_FAILED) {
 		nosv_warn("cannot enable kernel events, mmap failed");
@@ -421,12 +442,12 @@ static inline void instr_kernel_init(struct kinstr **ki_ptr)
 }
 
 struct sample_id {
-        //u32 pid, tid; }   /* if PERF_SAMPLE_TID set */
-        uint64_t time;      /* if PERF_SAMPLE_TIME set */
-        //u64 id;       }   /* if PERF_SAMPLE_ID set */
-        //u64 stream_id;}   /* if PERF_SAMPLE_STREAM_ID set  */
-        //u32 cpu, res; }   /* if PERF_SAMPLE_CPU set */
-        //u64 id;       }   /* if PERF_SAMPLE_IDENTIFIER set */
+	// u32 pid, tid; }   /* if PERF_SAMPLE_TID set */
+	uint64_t time; /* if PERF_SAMPLE_TIME set */
+				   // u64 id;       }   /* if PERF_SAMPLE_ID set */
+	// u64 stream_id;}   /* if PERF_SAMPLE_STREAM_ID set  */
+	// u32 cpu, res; }   /* if PERF_SAMPLE_CPU set */
+	// u64 id;       }   /* if PERF_SAMPLE_IDENTIFIER set */
 };
 
 struct perf_ev {
@@ -453,6 +474,7 @@ static inline void emit_perf_event(struct perf_ev *ev)
 
 static inline void instr_kernel_flush(struct kinstr *ki)
 {
+	CHECK_INSTR_ENABLED
 	struct ovni_ev ev0 = {0}, ev1 = {0};
 	struct perf_ev *ev;
 	uint8_t *p;
@@ -484,6 +506,7 @@ static inline void instr_kernel_flush(struct kinstr *ki)
 
 static inline void instr_kernel_free(struct kinstr *ki)
 {
+	CHECK_INSTR_ENABLED
 	sfree(ki, sizeof(*ki), -1);
 }
 

src/instr.c

@@ -0,0 +1,9 @@
+/*
+	This file is part of nOS-V and is licensed under the terms contained in the COPYING file.
+
+	Copyright (C) 2023 Barcelona Supercomputing Center (BSC)
+*/
+
+#include "instr.h"
+
+int instr_ovni_enabled = 0;

src/memory/sharedmemory.c

@@ -146,6 +146,14 @@ static inline void calculate_shared_memory_permissions(void)
 
 static inline void initialize_instrumentation(void)
 {
+	if (strcmp(nosv_config.instrumentation_version, "ovni") == 0) {
+#ifdef ENABLE_INSTRUMENTATION
+		instr_ovni_enabled = 1;
+#else
+		nosv_warn("ovni instrumentation requested, but nOS-V was built without ovni support");
+#endif
+	}
+
 	instr_proc_init(st_config.smem_name);
 	instr_thread_init();
 	instr_thread_execute(-1, -1, 0);

tests/basic/attach-external.c

@@ -10,11 +10,6 @@
 #include <pthread.h>
 #include <sched.h>
 
-#ifdef ENABLE_INSTRUMENTATION
-// Load instrumentation definitions from nosv header for now
-#include "instr.h"
-#endif
-
 static nosv_task_type_t type;
 static test_t test;
 
@@ -23,22 +18,12 @@ static void *start_routine(void *arg)
 {
 	nosv_task_t task;
 
-#ifdef ENABLE_INSTRUMENTATION
-	// The thread must be instrumented *outside* nosv, before it
-	// calls any external nosv function which may emit events.
-	instr_thread_init();
-	instr_thread_execute(-1, -1, 0);
-#endif
 	nosv_attach(&task, type, 0, NULL, NOSV_ATTACH_NONE);
 	test_ok(&test, "External thread can attach");
 
 	nosv_detach(NOSV_DETACH_NONE);
 	test_ok(&test, "External thread can detach");
 
-#ifdef ENABLE_INSTRUMENTATION
-	// The thread end also flushes the events to disk.
-	instr_thread_end();
-#endif
 	return NULL;
 }