Skip to content

Commit

Permalink
Kernel: Make Thread use AK::Time internally
Browse files Browse the repository at this point in the history
This commit is very invasive, because Thread likes to take a pointer and write
to it. This means that translating between timespec/timeval/Time would have been
more difficult than just changing everything that hands a raw pointer to Thread,
in bulk.
  • Loading branch information
BenWiederhake authored and awesomekling committed Mar 2, 2021
1 parent 65b36e4 commit 2b6546c
Show file tree
Hide file tree
Showing 14 changed files with 66 additions and 82 deletions.
2 changes: 1 addition & 1 deletion Kernel/Devices/AsyncDeviceRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void AsyncDeviceRequest::request_finished()
m_queue.wake_all();
}

auto AsyncDeviceRequest::wait(timeval* timeout) -> RequestWaitResult
auto AsyncDeviceRequest::wait(Time* timeout) -> RequestWaitResult
{
VERIFY(!m_parent_request);
auto request_result = get_request_result();
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Devices/AsyncDeviceRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class AsyncDeviceRequest : public RefCounted<AsyncDeviceRequest> {

void add_sub_request(NonnullRefPtr<AsyncDeviceRequest>);

[[nodiscard]] RequestWaitResult wait(timeval* = nullptr);
[[nodiscard]] RequestWaitResult wait(Time* = nullptr);

void do_start(Badge<Device>)
{
Expand Down
6 changes: 2 additions & 4 deletions Kernel/Devices/USB/UHCIController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -398,10 +398,8 @@ void UHCIController::do_debug_transfer()
void UHCIController::spawn_port_proc()
{
RefPtr<Thread> usb_hotplug_thread;
timespec sleep_time {};

sleep_time.tv_sec = 1;
Process::create_kernel_process(usb_hotplug_thread, "UHCIHotplug", [&, sleep_time] {
Process::create_kernel_process(usb_hotplug_thread, "UHCIHotplug", [&] {
for (;;) {
for (int port = 0; port < UHCI_ROOT_PORT_COUNT; port++) {
u16 port_data = 0;
Expand Down Expand Up @@ -448,7 +446,7 @@ void UHCIController::spawn_port_proc()
}
}
}
(void)Thread::current()->sleep(sleep_time);
(void)Thread::current()->sleep(Time::from_seconds(1));
}
});
}
Expand Down
10 changes: 5 additions & 5 deletions Kernel/Syscalls/alarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,18 @@ KResultOr<unsigned> Process::sys$alarm(unsigned seconds)
if (auto alarm_timer = move(m_alarm_timer)) {
if (TimerQueue::the().cancel_timer(*alarm_timer)) {
// The timer hasn't fired. Round up the remaining time (if any)
timespec remaining;
timespec_add(alarm_timer->remaining(), { 0, 1000000000 - 1 }, remaining);
previous_alarm_remaining = remaining.tv_sec;
Time remaining = alarm_timer->remaining() + Time::from_nanoseconds(999'999'999);
previous_alarm_remaining = remaining.to_truncated_seconds();
}
// We had an existing alarm, must return a non-zero value here!
if (previous_alarm_remaining == 0)
previous_alarm_remaining = 1;
}

if (seconds > 0) {
auto deadline = TimeManagement::the().current_time(CLOCK_REALTIME_COARSE).value();
timespec_add(deadline, { seconds, 0 }, deadline);
// FIXME: Should use AK::Time internally
auto deadline = Time::from_timespec(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);
});
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Syscalls/beep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace Kernel {
KResultOr<int> Process::sys$beep()
{
PCSpeaker::tone_on(440);
auto result = Thread::current()->sleep({ 0, 200 });
auto result = Thread::current()->sleep(Time::from_nanoseconds(200'000'000));
PCSpeaker::tone_off();
if (result.was_interrupted())
return EINTR;
Expand Down
11 changes: 5 additions & 6 deletions Kernel/Syscalls/clock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,12 @@ KResultOr<int> Process::sys$clock_nanosleep(Userspace<const Syscall::SC_clock_na

bool was_interrupted;
if (is_absolute) {
// FIXME: Should use AK::Time internally
was_interrupted = Thread::current()->sleep_until(params.clock_id, requested_sleep->to_timespec()).was_interrupted();
was_interrupted = Thread::current()->sleep_until(params.clock_id, requested_sleep.value()).was_interrupted();
} else {
timespec remaining_sleep;
// FIXME: Should use AK::Time internally
was_interrupted = Thread::current()->sleep(params.clock_id, requested_sleep->to_timespec(), &remaining_sleep).was_interrupted();
if (was_interrupted && params.remaining_sleep && !copy_to_user(params.remaining_sleep, &remaining_sleep))
Time remaining_sleep;
was_interrupted = Thread::current()->sleep(params.clock_id, requested_sleep.value(), &remaining_sleep).was_interrupted();
timespec remaining_sleep_ts = remaining_sleep.to_timespec();
if (was_interrupted && params.remaining_sleep && !copy_to_user(params.remaining_sleep, &remaining_sleep_ts))
return EFAULT;
}
if (was_interrupted)
Expand Down
4 changes: 1 addition & 3 deletions Kernel/Syscalls/futex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,7 @@ KResultOr<int> Process::sys$futex(Userspace<const Syscall::SC_futex_params*> use
return EFAULT;
clockid_t clock_id = (params.futex_op & FUTEX_CLOCK_REALTIME) ? CLOCK_REALTIME_COARSE : CLOCK_MONOTONIC_COARSE;
bool is_absolute = cmd != FUTEX_WAIT;
// FIXME: Should use AK::Time internally
timespec timeout_copy = timeout_time->to_timespec();
timeout = Thread::BlockTimeout(is_absolute, &timeout_copy, nullptr, clock_id);
timeout = Thread::BlockTimeout(is_absolute, &timeout_time.value(), nullptr, clock_id);
}
if (cmd == FUTEX_WAIT_BITSET && params.val3 == FUTEX_BITSET_MATCH_ANY)
cmd = FUTEX_WAIT;
Expand Down
8 changes: 2 additions & 6 deletions Kernel/Syscalls/select.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ KResultOr<int> Process::sys$select(Userspace<const Syscall::SC_select_params*> u
Optional<Time> timeout_time = copy_time_from_user(params.timeout);
if (!timeout_time.has_value())
return EFAULT;
auto timeout_copy = timeout_time->to_timespec();
// FIXME: Should use AK::Time internally
timeout = Thread::BlockTimeout(false, &timeout_copy);
timeout = Thread::BlockTimeout(false, &timeout_time.value());
}

auto current_thread = Thread::current();
Expand Down Expand Up @@ -156,9 +154,7 @@ KResultOr<int> Process::sys$poll(Userspace<const Syscall::SC_poll_params*> user_
auto timeout_time = copy_time_from_user(params.timeout);
if (!timeout_time.has_value())
return EFAULT;
timespec timeout_copy = timeout_time->to_timespec();
// FIXME: Should use AK::Time internally
timeout = Thread::BlockTimeout(false, &timeout_copy);
timeout = Thread::BlockTimeout(false, &timeout_time.value());
}

sigset_t sigmask = {};
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Tasks/SyncTask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ void SyncTask::spawn()
dbgln("SyncTask is running");
for (;;) {
VFS::the().sync();
(void)Thread::current()->sleep({ 1, 0 });
(void)Thread::current()->sleep(Time::from_seconds(1));
}
});
}
Expand Down
4 changes: 2 additions & 2 deletions Kernel/Thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,13 +333,13 @@ void Thread::relock_process(LockMode previous_locked, u32 lock_count_to_restore)
}
}

auto Thread::sleep(clockid_t clock_id, const timespec& duration, timespec* remaining_time) -> BlockResult
auto Thread::sleep(clockid_t clock_id, const Time& duration, Time* remaining_time) -> BlockResult
{
VERIFY(state() == Thread::Running);
return Thread::current()->block<Thread::SleepBlocker>({}, Thread::BlockTimeout(false, &duration, nullptr, clock_id), remaining_time);
}

auto Thread::sleep_until(clockid_t clock_id, const timespec& deadline) -> BlockResult
auto Thread::sleep_until(clockid_t clock_id, const Time& deadline) -> BlockResult
{
VERIFY(state() == Thread::Running);
return Thread::current()->block<Thread::SleepBlocker>({}, Thread::BlockTimeout(true, &deadline, nullptr, clock_id));
Expand Down
49 changes: 11 additions & 38 deletions Kernel/Thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,44 +208,17 @@ class Thread
: m_infinite(true)
{
}
explicit BlockTimeout(bool is_absolute, const timeval* time, const timespec* start_time = nullptr, clockid_t clock_id = CLOCK_MONOTONIC_COARSE)
: m_clock_id(clock_id)
, m_infinite(!time)
{
if (!m_infinite) {
if (time->tv_sec > 0 || time->tv_usec > 0) {
timeval_to_timespec(*time, m_time);
m_should_block = true;
}
m_start_time = start_time ? *start_time : TimeManagement::the().current_time(clock_id).value();
if (!is_absolute)
timespec_add(m_time, m_start_time, m_time);
}
}
explicit BlockTimeout(bool is_absolute, const timespec* time, const timespec* start_time = nullptr, clockid_t clock_id = CLOCK_MONOTONIC_COARSE)
: m_clock_id(clock_id)
, m_infinite(!time)
{
if (!m_infinite) {
if (time->tv_sec > 0 || time->tv_nsec > 0) {
m_time = *time;
m_should_block = true;
}
m_start_time = start_time ? *start_time : TimeManagement::the().current_time(clock_id).value();
if (!is_absolute)
timespec_add(m_time, m_start_time, m_time);
}
}
explicit BlockTimeout(bool is_absolute, const Time* time, const Time* start_time = nullptr, clockid_t clock_id = CLOCK_MONOTONIC_COARSE);

const timespec& absolute_time() const { return m_time; }
const timespec* start_time() const { return !m_infinite ? &m_start_time : nullptr; }
const Time& absolute_time() const { return m_time; }
const Time* start_time() const { return !m_infinite ? &m_start_time : nullptr; }
clockid_t clock_id() const { return m_clock_id; }
bool is_infinite() const { return m_infinite; }
bool should_block() const { return m_infinite || m_should_block; };

private:
timespec m_time { 0, 0 };
timespec m_start_time { 0, 0 };
Time m_time {};
Time m_start_time {};
clockid_t m_clock_id { CLOCK_MONOTONIC_COARSE };
bool m_infinite { false };
bool m_should_block { false };
Expand Down Expand Up @@ -640,7 +613,7 @@ class Thread

class SleepBlocker final : public Blocker {
public:
explicit SleepBlocker(const BlockTimeout&, timespec* = nullptr);
explicit SleepBlocker(const BlockTimeout&, Time* = nullptr);
virtual const char* state_string() const override { return "Sleeping"; }
virtual Type blocker_type() const override { return Type::Sleep; }
virtual const BlockTimeout& override_timeout(const BlockTimeout&) override;
Expand All @@ -652,7 +625,7 @@ class Thread
void calculate_remaining();

BlockTimeout m_deadline;
timespec* m_remaining;
Time* m_remaining;
};

class SelectBlocker final : public FileBlocker {
Expand Down Expand Up @@ -955,13 +928,13 @@ class Thread
return block<Thread::QueueBlocker>(timeout, wait_queue, forward<Args>(args)...);
}

BlockResult sleep(clockid_t, const timespec&, timespec* = nullptr);
BlockResult sleep(const timespec& duration, timespec* remaining_time = nullptr)
BlockResult sleep(clockid_t, const Time&, Time* = nullptr);
BlockResult sleep(const Time& duration, Time* remaining_time = nullptr)
{
return sleep(CLOCK_MONOTONIC_COARSE, duration, remaining_time);
}
BlockResult sleep_until(clockid_t, const timespec&);
BlockResult sleep_until(const timespec& duration)
BlockResult sleep_until(clockid_t, const Time&);
BlockResult sleep_until(const Time& duration)
{
return sleep_until(CLOCK_MONOTONIC_COARSE, duration);
}
Expand Down
31 changes: 26 additions & 5 deletions Kernel/ThreadBlockers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,22 @@

namespace Kernel {

Thread::BlockTimeout::BlockTimeout(bool is_absolute, const Time* time, const Time* start_time, clockid_t clock_id)
: m_clock_id(clock_id)
, m_infinite(!time)
{
if (!m_infinite) {
if (*time > Time::zero()) {
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());
if (!is_absolute)
m_time = m_time + m_start_time;
}
}

bool Thread::Blocker::set_block_condition(Thread::BlockCondition& block_condition, void* data)
{
VERIFY(!m_block_condition);
Expand Down Expand Up @@ -270,7 +286,9 @@ auto Thread::WriteBlocker::override_timeout(const BlockTimeout& timeout) -> cons
if (description.is_socket()) {
auto& socket = *description.socket();
if (socket.has_send_timeout()) {
m_timeout = BlockTimeout(false, &socket.send_timeout(), timeout.start_time(), timeout.clock_id());
// FIXME: Should use AK::Time internally
Time send_timeout = Time::from_timeval(socket.send_timeout());
m_timeout = BlockTimeout(false, &send_timeout, timeout.start_time(), timeout.clock_id());
if (timeout.is_infinite() || (!m_timeout.is_infinite() && m_timeout.absolute_time() < timeout.absolute_time()))
return m_timeout;
}
Expand All @@ -289,15 +307,17 @@ auto Thread::ReadBlocker::override_timeout(const BlockTimeout& timeout) -> const
if (description.is_socket()) {
auto& socket = *description.socket();
if (socket.has_receive_timeout()) {
m_timeout = BlockTimeout(false, &socket.receive_timeout(), timeout.start_time(), timeout.clock_id());
// FIXME: Should use AK::Time internally
Time receive_timeout = Time::from_timeval(socket.receive_timeout());
m_timeout = BlockTimeout(false, &receive_timeout, timeout.start_time(), timeout.clock_id());
if (timeout.is_infinite() || (!m_timeout.is_infinite() && m_timeout.absolute_time() < timeout.absolute_time()))
return m_timeout;
}
}
return timeout;
}

Thread::SleepBlocker::SleepBlocker(const BlockTimeout& deadline, timespec* remaining)
Thread::SleepBlocker::SleepBlocker(const BlockTimeout& deadline, Time* remaining)
: m_deadline(deadline)
, m_remaining(remaining)
{
Expand Down Expand Up @@ -329,9 +349,10 @@ void Thread::SleepBlocker::calculate_remaining()
{
if (!m_remaining)
return;
auto time_now = TimeManagement::the().current_time(m_deadline.clock_id()).value();
// FIXME: Should use AK::Time internally
auto time_now = Time::from_timespec(TimeManagement::the().current_time(m_deadline.clock_id()).value());
if (time_now < m_deadline.absolute_time())
timespec_sub(m_deadline.absolute_time(), time_now, *m_remaining);
*m_remaining = m_deadline.absolute_time() - time_now;
else
*m_remaining = {};
}
Expand Down
12 changes: 6 additions & 6 deletions Kernel/TimerQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,9 @@ namespace Kernel {
static AK::Singleton<TimerQueue> s_the;
static SpinLock<u8> g_timerqueue_lock;

timespec Timer::remaining() const
Time Timer::remaining() const
{
// FIXME: Should use AK::Time internally
return m_remaining.to_timespec();
return m_remaining;
}

Time Timer::now(bool is_firing) const
Expand Down Expand Up @@ -79,17 +78,18 @@ UNMAP_AFTER_INIT TimerQueue::TimerQueue()
m_ticks_per_second = TimeManagement::the().ticks_per_second();
}

RefPtr<Timer> TimerQueue::add_timer_without_id(clockid_t clock_id, const timespec& deadline, Function<void()>&& callback)
RefPtr<Timer> TimerQueue::add_timer_without_id(clockid_t clock_id, const Time& deadline, Function<void()>&& callback)
{
if (deadline <= TimeManagement::the().current_time(clock_id).value())
// FIXME: Should use AK::Time internally
if (deadline <= Time::from_timespec(TimeManagement::the().current_time(clock_id).value()))
return {};

// Because timer handlers can execute on any processor and there is
// a race between executing a timer handler and cancel_timer() this
// *must* be a RefPtr<Timer>. Otherwise calling cancel_timer() could
// inadvertently cancel another timer that has been created between
// returning from the timer handler and a call to cancel_timer().
auto timer = adopt(*new Timer(clock_id, Time::from_timespec(deadline), move(callback)));
auto timer = adopt(*new Timer(clock_id, deadline, move(callback)));

ScopedSpinLock lock(g_timerqueue_lock);
timer->m_id = 0; // Don't generate a timer id
Expand Down
5 changes: 2 additions & 3 deletions Kernel/TimerQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class Timer : public RefCounted<Timer>
VERIFY(!is_queued());
}

timespec remaining() const;
Time remaining() const;

private:
TimerId m_id;
Expand Down Expand Up @@ -92,8 +92,7 @@ class TimerQueue {
static TimerQueue& the();

TimerId add_timer(NonnullRefPtr<Timer>&&);
// FIXME: Should use AK::Time internally
RefPtr<Timer> add_timer_without_id(clockid_t, const timespec&, Function<void()>&&);
RefPtr<Timer> add_timer_without_id(clockid_t, const Time&, Function<void()>&&);
TimerId add_timer(clockid_t, timeval& timeout, Function<void()>&& callback);
bool cancel_timer(TimerId id);
bool cancel_timer(Timer&);
Expand Down

0 comments on commit 2b6546c

Please sign in to comment.