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

diff --git a/lib/vfscore/syscalls.c b/lib/vfscore/syscalls.c
index 5b6e2570b1db3256c0877ed5b308b9ea3fc0d65a..76f000d8c95bbbdf8e4d073854d7ca10ec5eb042 100644 (file)
--- 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 */