From c040e64b7d231fc374e99882751c58f50e116339 Mon Sep 17 00:00:00 2001
From: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>
Date: Sat, 27 Feb 2021 23:56:16 +0100
Subject: [PATCH] Kernel: Make TimeManagement use AK::Time internally

I don't dare touch the multi-threading logic and locking mechanism, so it stays
timespec for now. However, this could and should be changed to AK::Time, and I
bet it will simplify the "increment_time_since_boot()" code.
---
 Kernel/Process.cpp             |  2 +-
 Kernel/Process.h               |  1 +
 Kernel/Syscalls/alarm.cpp      |  3 +--
 Kernel/Syscalls/clock.cpp      | 12 ++++++------
 Kernel/ThreadBlockers.cpp      |  6 ++----
 Kernel/Time/TimeManagement.cpp | 26 +++++++++++++-------------
 Kernel/Time/TimeManagement.h   | 18 ++++++++++++------
 Kernel/TimerQueue.cpp          |  9 +++------
 8 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index ec26de63af..8bace0d8f2 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -383,7 +383,7 @@ int Process::alloc_fd(int first_candidate_fd)
 
 timeval kgettimeofday()
 {
-    return TimeManagement::now_as_timeval();
+    return TimeManagement::now().to_timeval();
 }
 
 void kgettimeofday(timeval& tv)
diff --git a/Kernel/Process.h b/Kernel/Process.h
index 31aba2b744..f2bef271ed 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -54,6 +54,7 @@
 
 namespace Kernel {
 
+// FIXME: Should use AK::Time internally
 timeval kgettimeofday();
 void kgettimeofday(timeval&);
 
diff --git a/Kernel/Syscalls/alarm.cpp b/Kernel/Syscalls/alarm.cpp
index ff3de1350c..18104863ed 100644
--- a/Kernel/Syscalls/alarm.cpp
+++ b/Kernel/Syscalls/alarm.cpp
@@ -45,8 +45,7 @@ KResultOr<unsigned> Process::sys$alarm(unsigned seconds)
     }
 
     if (seconds > 0) {
-        // FIXME: Should use AK::Time internally
-        auto deadline = Time::from_timespec(TimeManagement::the().current_time(CLOCK_REALTIME_COARSE).value());
+        auto deadline = TimeManagement::the().current_time(CLOCK_REALTIME_COARSE).value();
         deadline = deadline + Time::from_seconds(seconds);
         m_alarm_timer = TimerQueue::the().add_timer_without_id(CLOCK_REALTIME_COARSE, deadline, [this]() {
             [[maybe_unused]] auto rc = send_signal(SIGALRM, nullptr);
diff --git a/Kernel/Syscalls/clock.cpp b/Kernel/Syscalls/clock.cpp
index d883e6ac04..5296644141 100644
--- a/Kernel/Syscalls/clock.cpp
+++ b/Kernel/Syscalls/clock.cpp
@@ -34,11 +34,12 @@ KResultOr<int> Process::sys$clock_gettime(clockid_t clock_id, Userspace<timespec
 {
     REQUIRE_PROMISE(stdio);
 
-    auto ts = TimeManagement::the().current_time(clock_id);
-    if (ts.is_error())
-        return ts.error();
+    auto time = TimeManagement::the().current_time(clock_id);
+    if (time.is_error())
+        return time.error();
 
-    if (!copy_to_user(user_ts, &ts.value()))
+    auto ts = time.value().to_timespec();
+    if (!copy_to_user(user_ts, &ts))
         return EFAULT;
     return 0;
 }
@@ -56,8 +57,7 @@ KResultOr<int> Process::sys$clock_settime(clockid_t clock_id, Userspace<const ti
 
     switch (clock_id) {
     case CLOCK_REALTIME:
-        // FIXME: Should use AK::Time internally
-        TimeManagement::the().set_epoch_time(ts->to_timespec());
+        TimeManagement::the().set_epoch_time(ts.value());
         break;
     default:
         return EINVAL;
diff --git a/Kernel/ThreadBlockers.cpp b/Kernel/ThreadBlockers.cpp
index 158dee8862..3def0b7b3d 100644
--- a/Kernel/ThreadBlockers.cpp
+++ b/Kernel/ThreadBlockers.cpp
@@ -42,8 +42,7 @@ Thread::BlockTimeout::BlockTimeout(bool is_absolute, const Time* time, const Tim
             m_time = *time;
             m_should_block = true;
         }
-        // FIXME: Should use AK::Time internally
-        m_start_time = start_time ? *start_time : Time::from_timespec(TimeManagement::the().current_time(clock_id).value());
+        m_start_time = start_time ? *start_time : TimeManagement::the().current_time(clock_id).value();
         if (!is_absolute)
             m_time = m_time + m_start_time;
     }
@@ -349,8 +348,7 @@ void Thread::SleepBlocker::calculate_remaining()
 {
     if (!m_remaining)
         return;
-    // FIXME: Should use AK::Time internally
-    auto time_now = Time::from_timespec(TimeManagement::the().current_time(m_deadline.clock_id()).value());
+    auto time_now = TimeManagement::the().current_time(m_deadline.clock_id()).value();
     if (time_now < m_deadline.absolute_time())
         *m_remaining = m_deadline.absolute_time() - time_now;
     else
diff --git a/Kernel/Time/TimeManagement.cpp b/Kernel/Time/TimeManagement.cpp
index ee4515091f..01efc7c371 100644
--- a/Kernel/Time/TimeManagement.cpp
+++ b/Kernel/Time/TimeManagement.cpp
@@ -64,7 +64,7 @@ bool TimeManagement::is_valid_clock_id(clockid_t clock_id)
     };
 }
 
-KResultOr<timespec> TimeManagement::current_time(clockid_t clock_id) const
+KResultOr<Time> TimeManagement::current_time(clockid_t clock_id) const
 {
     switch (clock_id) {
     case CLOCK_MONOTONIC:
@@ -87,14 +87,15 @@ bool TimeManagement::is_system_timer(const HardwareTimerBase& timer) const
     return &timer == m_system_timer.ptr();
 }
 
-void TimeManagement::set_epoch_time(timespec ts)
+void TimeManagement::set_epoch_time(Time ts)
 {
     InterruptDisabler disabler;
-    m_epoch_time = ts;
+    // FIXME: Should use AK::Time internally
+    m_epoch_time = ts.to_timespec();
     m_remaining_epoch_time_adjustment = { 0, 0 };
 }
 
-timespec TimeManagement::monotonic_time(TimePrecision precision) const
+Time TimeManagement::monotonic_time(TimePrecision precision) const
 {
     // This is the time when last updated by an interrupt.
     u64 seconds;
@@ -122,10 +123,10 @@ timespec TimeManagement::monotonic_time(TimePrecision precision) const
     VERIFY(ticks < m_time_ticks_per_second);
     u64 ns = ((u64)ticks * 1000000000ull) / m_time_ticks_per_second;
     VERIFY(ns < 1000000000ull);
-    return { (long)seconds, (long)ns };
+    return Time::from_timespec({ (i64)seconds, (i32)ns });
 }
 
-timespec TimeManagement::epoch_time(TimePrecision) const
+Time TimeManagement::epoch_time(TimePrecision) const
 {
     // TODO: Take into account precision
     timespec ts;
@@ -134,12 +135,14 @@ timespec TimeManagement::epoch_time(TimePrecision) const
         update_iteration = m_update1.load(AK::MemoryOrder::memory_order_acquire);
         ts = m_epoch_time;
     } while (update_iteration != m_update2.load(AK::MemoryOrder::memory_order_acquire));
-    return ts;
+    return Time::from_timespec(ts);
 }
 
 u64 TimeManagement::uptime_ms() const
 {
-    auto mtime = monotonic_time();
+    auto mtime = monotonic_time().to_timespec();
+    // This overflows after 292 million years of uptime.
+    // Since this is only used for performance timestamps and sys$times, that's probably enough.
     u64 ms = mtime.tv_sec * 1000ull;
     ms += mtime.tv_nsec / 1000000;
     return ms;
@@ -211,12 +214,9 @@ UNMAP_AFTER_INIT TimeManagement::TimeManagement()
     }
 }
 
-timeval TimeManagement::now_as_timeval()
+Time TimeManagement::now()
 {
-    timespec ts = s_the.ptr()->epoch_time();
-    timeval tv;
-    timespec_to_timeval(ts, tv);
-    return tv;
+    return s_the.ptr()->epoch_time();
 }
 
 Vector<HardwareTimerBase*> TimeManagement::scan_and_initialize_periodic_timers()
diff --git a/Kernel/Time/TimeManagement.h b/Kernel/Time/TimeManagement.h
index f6c40aee1e..c7ed371418 100644
--- a/Kernel/Time/TimeManagement.h
+++ b/Kernel/Time/TimeManagement.h
@@ -28,6 +28,7 @@
 
 #include <AK/NonnullRefPtrVector.h>
 #include <AK/RefPtr.h>
+#include <AK/Time.h>
 #include <AK/Types.h>
 #include <Kernel/KResult.h>
 #include <Kernel/UnixTypes.h>
@@ -53,15 +54,15 @@ public:
     static TimeManagement& the();
 
     static bool is_valid_clock_id(clockid_t);
-    KResultOr<timespec> current_time(clockid_t) const;
-    timespec monotonic_time(TimePrecision = TimePrecision::Coarse) const;
-    timespec monotonic_time_raw() const
+    KResultOr<Time> current_time(clockid_t) const;
+    Time monotonic_time(TimePrecision = TimePrecision::Coarse) const;
+    Time monotonic_time_raw() const
     {
         // TODO: implement
         return monotonic_time(TimePrecision::Precise);
     }
-    timespec epoch_time(TimePrecision = TimePrecision::Precise) const;
-    void set_epoch_time(timespec);
+    Time epoch_time(TimePrecision = TimePrecision::Precise) const;
+    void set_epoch_time(Time);
     time_t ticks_per_second() const;
     time_t boot_time() const;
 
@@ -75,9 +76,13 @@ public:
     static bool is_hpet_periodic_mode_allowed();
 
     u64 uptime_ms() const;
-    static timeval now_as_timeval();
+    static Time now();
 
+    // FIXME: Should use AK::Time internally
+    // FIXME: Also, most likely broken, because it does not check m_update[12] for in-progress updates.
     timespec remaining_epoch_time_adjustment() const { return m_remaining_epoch_time_adjustment; }
+    // FIXME: Should use AK::Time internally
+    // FIXME: Also, most likely broken, because it does not check m_update[12] for in-progress updates.
     void set_remaining_epoch_time_adjustment(const timespec& adjustment) { m_remaining_epoch_time_adjustment = adjustment; }
 
     bool can_query_precise_time() const { return m_can_query_precise_time; }
@@ -95,6 +100,7 @@ private:
     Atomic<u32> m_update1 { 0 };
     u32 m_ticks_this_second { 0 };
     u64 m_seconds_since_boot { 0 };
+    // FIXME: Should use AK::Time internally
     timespec m_epoch_time { 0, 0 };
     timespec m_remaining_epoch_time_adjustment { 0, 0 };
     Atomic<u32> m_update2 { 0 };
diff --git a/Kernel/TimerQueue.cpp b/Kernel/TimerQueue.cpp
index 59910c1fb5..11e06d793c 100644
--- a/Kernel/TimerQueue.cpp
+++ b/Kernel/TimerQueue.cpp
@@ -64,8 +64,7 @@ Time Timer::now(bool is_firing) const
             break;
         }
     }
-    // FIXME: Should use AK::Time internally
-    return Time::from_timespec(TimeManagement::the().current_time(clock_id).value());
+    return TimeManagement::the().current_time(clock_id).value();
 }
 
 TimerQueue& TimerQueue::the()
@@ -80,8 +79,7 @@ UNMAP_AFTER_INIT TimerQueue::TimerQueue()
 
 RefPtr<Timer> TimerQueue::add_timer_without_id(clockid_t clock_id, const Time& deadline, Function<void()>&& callback)
 {
-    // FIXME: Should use AK::Time internally
-    if (deadline <= Time::from_timespec(TimeManagement::the().current_time(clock_id).value()))
+    if (deadline <= TimeManagement::the().current_time(clock_id).value())
         return {};
 
     // Because timer handlers can execute on any processor and there is
@@ -139,8 +137,7 @@ void TimerQueue::add_timer_locked(NonnullRefPtr<Timer> timer)
 
 TimerId TimerQueue::add_timer(clockid_t clock_id, const Time& deadline, Function<void()>&& callback)
 {
-    // FIXME: Should use AK::Time internally
-    auto expires = Time::from_timespec(TimeManagement::the().current_time(clock_id).value());
+    auto expires = TimeManagement::the().current_time(clock_id).value();
     expires = expires + deadline;
     return add_timer(adopt(*new Timer(clock_id, expires, move(callback))));
 }