From 6a62e4f6151fee8a9ccfb3e9fe92fd20f31f3d88 Mon Sep 17 00:00:00 2001
From: Sergiu Moga <sergiu@unikraft.io>
Date: Fri, 7 Feb 2025 12:17:48 +0200
Subject: [PATCH] lib/vfscore: Use kernel internal `clock_gettime` variant

Use the kernel internal variant of `clock_gettime`,
`uk_sys_clock_gettime`. This helps avoid unnecessary execution of
the syscall wrappers' logic of syscall shim that would have otherwise
been run through `uk_syscall_r_clock_gettime`. Additionally, make
sure to also handle errors of said syscall.

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
Approved-by: Andrei Tatar <andrei@unikraft.io>
Reviewed-by: Andrei Tatar <andrei@unikraft.io>
GitHub-Closes: #1586
---
 lib/vfscore/syscalls.c | 83 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 67 insertions(+), 16 deletions(-)

diff --git a/lib/vfscore/syscalls.c b/lib/vfscore/syscalls.c
index 5b6e2570b..76f000d8c 100644
--- a/lib/vfscore/syscalls.c
+++ b/lib/vfscore/syscalls.c
@@ -58,6 +58,8 @@
 #include "vfs.h"
 #include <vfscore/fs.h>
 
+#include <uk/posix-time.h>
+
 extern struct task *main_task;
 
 static int
@@ -1316,14 +1318,15 @@ static int is_timeval_valid(const struct timeval *time)
 /*
  * Convert a timeval struct to a timespec one.
  */
-static void convert_timeval(struct timespec *to, const struct timeval *from)
+static int convert_timeval(struct timespec *to, const struct timeval *from)
 {
 	if (from) {
 		to->tv_sec = from->tv_sec;
 		to->tv_nsec = from->tv_usec * 1000; // Convert microseconds to nanoseconds
-	} else {
-		clock_gettime(CLOCK_REALTIME, to);
+		return 0;
 	}
+
+	return uk_sys_clock_gettime(CLOCK_REALTIME, to);
 }
 
 int
@@ -1339,8 +1342,27 @@ sys_utimes(char *path, const struct timeval *times, int flags)
 		return EINVAL;
 
 	// Convert each element of timeval array to the timespec type
-	convert_timeval(&timespec_times[0], times ? times + 0 : NULL);
-	convert_timeval(&timespec_times[1], times ? times + 1 : NULL);
+	error = convert_timeval(&timespec_times[0], times ? times + 0 : NULL);
+	if (unlikely(error)) {
+		/*
+		 * convert_timeval calls clock_gettime which should not have
+		 * positive return values so do a sanity check here in the
+		 * error case instead of having it in the success path as
+		 * well.
+		 */
+		UK_ASSERT(error < 0);
+		/*
+		 * However, at the end, this function returns positive
+		 * error values.
+		 */
+		return -error;
+	}
+
+	error = convert_timeval(&timespec_times[1], times ? times + 1 : NULL);
+	if (unlikely(error)) {
+		UK_ASSERT(error < 0);
+		return -error;
+	}
 
 	if (flags & AT_SYMLINK_NOFOLLOW) {
 		struct dentry *ddp;
@@ -1382,14 +1404,15 @@ static int timespec_is_valid(const struct timespec *time)
 		time->tv_nsec == UTIME_OMIT);
 }
 
-static void timespec_init(struct timespec *out, const struct timespec *in)
+static int timespec_init(struct timespec *out, const struct timespec *in)
 {
-	if (in == NULL || in->tv_nsec == UTIME_NOW) {
-		clock_gettime(CLOCK_REALTIME, out);
-	} else {
-		out->tv_sec = in->tv_sec;
-		out->tv_nsec = in->tv_nsec;
-	}
+	if (in == NULL || in->tv_nsec == UTIME_NOW)
+		return uk_sys_clock_gettime(CLOCK_REALTIME, out);
+
+	out->tv_sec = in->tv_sec;
+	out->tv_nsec = in->tv_nsec;
+
+	return 0;
 }
 
 int
@@ -1413,11 +1436,39 @@ sys_utimensat(int dirfd, const char *pathname, const struct timespec times[2],
 			     !timespec_is_valid(&times[1])))
 			return EINVAL;
 
-		timespec_init(&timespec_times[0], times + 0);
-		timespec_init(&timespec_times[1], times + 1);
+		error = timespec_init(&timespec_times[0], times + 0);
+		if (unlikely(error)) {
+			/*
+			 * timespec_init calls clock_gettime which should not
+			 * have positive return values so do a sanity check
+			 * here in the error case instead of having it in the
+			 * success path as well.
+			 */
+			UK_ASSERT(error < 0);
+			/*
+			 * However, at the end, this function returns positive
+			 * error values.
+			 */
+			return -error;
+		}
+
+		error = timespec_init(&timespec_times[1], times + 1);
+		if (unlikely(error)) {
+			UK_ASSERT(error < 0);
+			return -error;
+		}
 	} else {
-		timespec_init(&timespec_times[0], NULL);
-		timespec_init(&timespec_times[1], NULL);
+		error = timespec_init(&timespec_times[0], NULL);
+		if (unlikely(error)) {
+			UK_ASSERT(error < 0);
+			return -error;
+		}
+
+		error = timespec_init(&timespec_times[1], NULL);
+		if (unlikely(error)) {
+			UK_ASSERT(error < 0);
+			return -error;
+		}
 	}
 
 	/* utimensat should return ENOENT when pathname is empty */
-- 
2.39.5