From b2e502e533173594431b53ef551bd64bbb85ae4f Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 14 Jul 2019 14:51:54 +0200 Subject: [PATCH] Kernel: Add Thread::block_until(Condition). Replace the class-based snooze alarm mechanism with a per-thread callback. This makes it easy to block the current thread on an arbitrary condition: void SomeDevice::wait_for_irq() { m_interrupted = false; current->block_until([this] { return m_interrupted; }); } void SomeDevice::handle_irq() { m_interrupted = true; } Use this in the SB16 driver, and in NetworkTask :^) --- Kernel/Devices/SB16.cpp | 8 +------- Kernel/Devices/SB16.h | 17 ----------------- Kernel/Net/NetworkAdapter.cpp | 6 ------ Kernel/Net/NetworkAdapter.h | 17 ----------------- Kernel/Net/NetworkTask.cpp | 28 +++++++++------------------- Kernel/Scheduler.cpp | 7 +++---- Kernel/Thread.cpp | 10 +++++----- Kernel/Thread.h | 7 ++++--- 8 files changed, 22 insertions(+), 78 deletions(-) diff --git a/Kernel/Devices/SB16.cpp b/Kernel/Devices/SB16.cpp index 67be19922be..b97cf545fd9 100644 --- a/Kernel/Devices/SB16.cpp +++ b/Kernel/Devices/SB16.cpp @@ -49,7 +49,6 @@ static SB16* s_the; SB16::SB16() : IRQHandler(5) , CharacterDevice(42, 42) // ### ? - , m_interrupt_alarm(*this) { s_the = this; initialize(); @@ -144,7 +143,7 @@ void SB16::wait_for_irq() #ifdef SB16_DEBUG kprintf("SB16: waiting for interrupt...\n"); #endif - current->snooze_until(m_interrupt_alarm); + current->block_until([this] { return m_interrupted; }); #ifdef SB16_DEBUG kprintf("SB16: got interrupt!\n"); #endif @@ -195,8 +194,3 @@ ssize_t SB16::write(FileDescription&, const u8* data, ssize_t length) wait_for_irq(); return length; } - -bool SB16InterruptAlarm::is_ringing() const -{ - return m_device.got_interrupt({}); -} diff --git a/Kernel/Devices/SB16.h b/Kernel/Devices/SB16.h index bef47f5f075..d91de1c2d4d 100644 --- a/Kernel/Devices/SB16.h +++ b/Kernel/Devices/SB16.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include #include @@ -9,18 +8,6 @@ class SB16; -class SB16InterruptAlarm final : public Alarm { -public: - SB16InterruptAlarm(SB16& device) - : m_device(device) - { - } - virtual bool is_ringing() const override; - -private: - SB16& m_device; -}; - class SB16 final : public IRQHandler , public CharacterDevice { public: @@ -35,8 +22,6 @@ public: virtual ssize_t write(FileDescription&, const u8*, ssize_t) override; virtual bool can_write(FileDescription&) const override { return true; } - bool got_interrupt(Badge) const { return m_interrupted; } - private: // ^IRQHandler virtual void handle_irq() override; @@ -54,6 +39,4 @@ private: RefPtr m_dma_buffer_page; bool m_interrupted { false }; int m_major_version { 0 }; - - SB16InterruptAlarm m_interrupt_alarm; }; diff --git a/Kernel/Net/NetworkAdapter.cpp b/Kernel/Net/NetworkAdapter.cpp index f1eaffc245d..f4ef012c4f8 100644 --- a/Kernel/Net/NetworkAdapter.cpp +++ b/Kernel/Net/NetworkAdapter.cpp @@ -33,7 +33,6 @@ NetworkAdapter* NetworkAdapter::from_ipv4_address(const IPv4Address& address) } NetworkAdapter::NetworkAdapter() - : m_packet_queue_alarm(*this) { // FIXME: I wanna lock :( all_adapters().resource().set(this); @@ -106,8 +105,3 @@ void NetworkAdapter::set_interface_name(const StringView& basename) builder.append('0'); m_name = builder.to_string(); } - -bool PacketQueueAlarm::is_ringing() const -{ - return m_adapter.has_queued_packets(); -} diff --git a/Kernel/Net/NetworkAdapter.h b/Kernel/Net/NetworkAdapter.h index 118bb137742..2052b3cc438 100644 --- a/Kernel/Net/NetworkAdapter.h +++ b/Kernel/Net/NetworkAdapter.h @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include @@ -12,19 +11,6 @@ class NetworkAdapter; -class PacketQueueAlarm final : public Alarm { -public: - PacketQueueAlarm(NetworkAdapter& adapter) - : m_adapter(adapter) - { - } - virtual ~PacketQueueAlarm() override {} - virtual bool is_ringing() const override; - -private: - NetworkAdapter& m_adapter; -}; - class NetworkAdapter { public: static void for_each(Function); @@ -44,8 +30,6 @@ public: ByteBuffer dequeue_packet(); - Alarm& packet_queue_alarm() { return m_packet_queue_alarm; } - bool has_queued_packets() const { return !m_packet_queue.is_empty(); } protected: @@ -58,7 +42,6 @@ protected: private: MACAddress m_mac_address; IPv4Address m_ipv4_address; - PacketQueueAlarm m_packet_queue_alarm; SinglyLinkedList m_packet_queue; String m_name; }; diff --git a/Kernel/Net/NetworkTask.cpp b/Kernel/Net/NetworkTask.cpp index c680a661355..5c553e7cd27 100644 --- a/Kernel/Net/NetworkTask.cpp +++ b/Kernel/Net/NetworkTask.cpp @@ -33,22 +33,6 @@ Lockable>& arp_table() return *the; } -class CombinedPacketQueueAlarm : public Alarm { -public: - CombinedPacketQueueAlarm() {} - - virtual bool is_ringing() const override - { - if (LoopbackAdapter::the().has_queued_packets()) - return true; - if (auto* e1000 = E1000NetworkAdapter::the()) { - if (e1000->has_queued_packets()) - return true; - } - return false; - } -}; - void NetworkTask_main() { LoopbackAdapter::the(); @@ -71,13 +55,19 @@ void NetworkTask_main() return {}; }; - CombinedPacketQueueAlarm queue_alarm; - kprintf("NetworkTask: Enter main loop.\n"); for (;;) { auto packet = dequeue_packet(); if (packet.is_null()) { - current->snooze_until(queue_alarm); + current->block_until([] { + if (LoopbackAdapter::the().has_queued_packets()) + return true; + if (auto* e1000 = E1000NetworkAdapter::the()) { + if (e1000->has_queued_packets()) + return true; + } + return false; + }); continue; } if (packet.size() < (int)(sizeof(EthernetFrameHeader))) { diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp index 479bc4be465..4b78495faad 100644 --- a/Kernel/Scheduler.cpp +++ b/Kernel/Scheduler.cpp @@ -1,5 +1,4 @@ #include -#include #include #include #include @@ -159,9 +158,9 @@ bool Scheduler::pick_next() return IterationDecision::Continue; } - if (thread.state() == Thread::BlockedSnoozing) { - if (thread.m_snoozing_alarm->is_ringing()) { - thread.m_snoozing_alarm = nullptr; + if (thread.state() == Thread::BlockedCondition) { + if (thread.m_block_until_condition()) { + thread.m_block_until_condition = nullptr; thread.unblock(); } return IterationDecision::Continue; diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index bb02093e772..4680b80809f 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -108,10 +108,10 @@ void Thread::unblock() set_state(Thread::Runnable); } -void Thread::snooze_until(Alarm& alarm) +void Thread::block_until(Function&& condition) { - m_snoozing_alarm = &alarm; - block(Thread::BlockedSnoozing); + m_block_until_condition = move(condition); + block(Thread::BlockedCondition); Scheduler::yield(); } @@ -179,8 +179,8 @@ const char* to_string(Thread::State state) return "Connect"; case Thread::BlockedReceive: return "Receive"; - case Thread::BlockedSnoozing: - return "Snoozing"; + case Thread::BlockedCondition: + return "Condition"; case Thread::__Begin_Blocked_States__: case Thread::__End_Blocked_States__: break; diff --git a/Kernel/Thread.h b/Kernel/Thread.h index d955d207df4..b9f8da5e47a 100644 --- a/Kernel/Thread.h +++ b/Kernel/Thread.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -72,7 +73,7 @@ public: BlockedSelect, BlockedConnect, BlockedReceive, - BlockedSnoozing, + BlockedCondition, __End_Blocked_States__ }; @@ -102,7 +103,7 @@ public: void set_wakeup_time(u64 t) { m_wakeup_time = t; } u64 wakeup_time() const { return m_wakeup_time; } - void snooze_until(Alarm&); + void block_until(Function&&); KResult wait_for_connect(FileDescription&); const FarPtr& far_ptr() const { return m_far_ptr; } @@ -187,7 +188,7 @@ private: timeval m_select_timeout; SignalActionData m_signal_action_data[32]; Region* m_signal_stack_user_region { nullptr }; - Alarm* m_snoozing_alarm { nullptr }; + Function m_block_until_condition; Vector m_select_read_fds; Vector m_select_write_fds; Vector m_select_exceptional_fds;