From c55a49900916ff9a79849c73e8ec7d56419e08de Mon Sep 17 00:00:00 2001 From: Alexander Boettcher Date: Tue, 21 Jul 2020 15:58:46 +0200 Subject: [PATCH] base: remove delayed dispatch from Rpc_entrypoint Fixes #3833 --- .../base-nova/src/lib/base/rpc_entrypoint.cc | 36 +++---------------- repos/base-nova/src/test/platform/main.cc | 12 ++++--- repos/base/include/base/rpc_server.h | 19 +++------- repos/base/lib/symbols/ld | 4 +-- repos/base/src/core/include/core_env.h | 3 +- repos/base/src/core/include/io_port_root.h | 2 +- repos/base/src/core/include/irq_root.h | 2 +- .../base/src/core/signal_transmitter_proxy.cc | 2 +- repos/base/src/lib/base/entrypoint.cc | 6 ++-- repos/base/src/lib/base/rpc_dispatch_loop.cc | 9 ----- repos/base/src/lib/base/rpc_entrypoint.cc | 18 +--------- repos/base/src/test/smp/main.cc | 2 +- 12 files changed, 27 insertions(+), 88 deletions(-) diff --git a/repos/base-nova/src/lib/base/rpc_entrypoint.cc b/repos/base-nova/src/lib/base/rpc_entrypoint.cc index f761fb1d59..408798f3ee 100644 --- a/repos/base-nova/src/lib/base/rpc_entrypoint.cc +++ b/repos/base-nova/src/lib/base/rpc_entrypoint.cc @@ -64,7 +64,7 @@ Untyped_capability Rpc_entrypoint::_manage(Rpc_object_base *obj) } static void cleanup_call(Rpc_object_base *obj, Nova::Utcb * ep_utcb, - Native_capability &cap, Genode::Blockade &delay_start) + Native_capability &cap) { /* effectively invalidate the capability used before */ @@ -84,9 +84,6 @@ static void cleanup_call(Rpc_object_base *obj, Nova::Utcb * ep_utcb, if (utcb == ep_utcb) return; - /* activate entrypoint now - otherwise cleanup call will block forever */ - delay_start.wakeup(); - /* make a IPC to ensure that cap() identifier is not used anymore */ utcb->msg()[0] = 0xdead; utcb->set_msg_word(1); @@ -109,8 +106,7 @@ void Rpc_entrypoint::_dissolve(Rpc_object_base *obj) /* make sure nobody is able to find this object */ remove(obj); - cleanup_call(obj, reinterpret_cast(this->utcb()), _cap, - _delay_start); + cleanup_call(obj, reinterpret_cast(this->utcb()), _cap); } static void reply(Nova::Utcb &utcb, Rpc_exception_code exc, Msgbuf_base &snd_msg) @@ -160,11 +156,6 @@ void Rpc_entrypoint::_activation_entry() reply(utcb, exc, ep._snd_buf); } - /* delay start */ - ep._delay_start.block(); - /* XXX inadequate usage of Blockade here is planned to be removed, see #3612 */ - ep._delay_start.wakeup(); - /* atomically lookup and lock referenced object */ auto lambda = [&] (Rpc_object_base *obj) { if (!obj) { @@ -198,20 +189,6 @@ void Rpc_entrypoint::entry() void Rpc_entrypoint::_block_until_cap_valid() { } -void Rpc_entrypoint::activate() -{ - /* - * In contrast to a normal thread, a server activation is created at - * construction time. However, it executes no code because processing - * time is always provided by the caller of the server activation. To - * delay the processing of requests until the 'activate' function is - * called, we grab the '_delay_start' lock on construction and release it - * here. - */ - _delay_start.wakeup(); -} - - bool Rpc_entrypoint::is_myself() const { return (Thread::myself() == this); @@ -219,8 +196,7 @@ bool Rpc_entrypoint::is_myself() const Rpc_entrypoint::Rpc_entrypoint(Pd_session *pd_session, size_t stack_size, - const char *name, bool start_on_construction, - Affinity::Location location) + const char *name, Affinity::Location location) : Thread(Cpu_session::Weight::DEFAULT_WEIGHT, name, stack_size, location), _pd_session(*pd_session) @@ -246,9 +222,6 @@ Rpc_entrypoint::Rpc_entrypoint(Pd_session *pd_session, size_t stack_size, /* prepare portal receive window of new thread */ if (!rcv_window.prepare_rcv_window(*(Nova::Utcb *)&_stack->utcb())) throw Cpu_session::Thread_creation_failed(); - - if (start_on_construction) - activate(); } @@ -269,8 +242,7 @@ Rpc_entrypoint::~Rpc_entrypoint() /* avoid any incoming IPC */ Nova::revoke(Nova::Obj_crd(obj->cap().local_name(), 0), true); - cleanup_call(obj, reinterpret_cast(this->utcb()), _cap, - _delay_start); + cleanup_call(obj, reinterpret_cast(this->utcb()), _cap); }); if (!_cap.valid()) diff --git a/repos/base-nova/src/test/platform/main.cc b/repos/base-nova/src/test/platform/main.cc index 309ef64bc6..978f3d65bf 100644 --- a/repos/base-nova/src/test/platform/main.cc +++ b/repos/base-nova/src/test/platform/main.cc @@ -41,7 +41,8 @@ using namespace Genode; void test_translate(Genode::Env &env) { enum { STACK_SIZE = 4096 }; - static Rpc_entrypoint ep(&env.pd(), STACK_SIZE, "rpc_ep_translate"); + static Rpc_entrypoint ep(&env.pd(), STACK_SIZE, "rpc_ep_translate", + Affinity::Location()); Test::Component component; Test::Capability session_cap = ep.manage(&component); @@ -136,7 +137,8 @@ void test_translate(Genode::Env &env) void test_revoke(Genode::Env &env) { enum { STACK_SIZE = 4096 }; - static Rpc_entrypoint ep(&env.pd(), STACK_SIZE, "rpc_ep_revoke"); + static Rpc_entrypoint ep(&env.pd(), STACK_SIZE, "rpc_ep_revoke", + Affinity::Location()); Test::Component component; Test::Capability session_cap = ep.manage(&component); @@ -304,7 +306,8 @@ void test_pat(Genode::Env &env) enum { STACK_SIZE = 4096 }; - static Rpc_entrypoint ep(&env.pd(), STACK_SIZE, "rpc_ep_pat"); + static Rpc_entrypoint ep(&env.pd(), STACK_SIZE, "rpc_ep_pat", + Affinity::Location()); Genode::Rm_connection rm(env); Genode::Region_map_client rm_free_area(rm.create(1 << (DS_ORDER + PAGE_4K))); @@ -381,7 +384,8 @@ void test_server_oom(Genode::Env &env) enum { STACK_SIZE = 4096 }; - static Rpc_entrypoint ep(&env.pd(), STACK_SIZE, "rpc_ep_oom"); + static Rpc_entrypoint ep(&env.pd(), STACK_SIZE, "rpc_ep_oom", + Affinity::Location()); Test::Component component; Test::Capability session_cap = ep.manage(&component); diff --git a/repos/base/include/base/rpc_server.h b/repos/base/include/base/rpc_server.h index 4e6707aedf..0061645c8b 100644 --- a/repos/base/include/base/rpc_server.h +++ b/repos/base/include/base/rpc_server.h @@ -283,13 +283,9 @@ struct Genode::Rpc_object : Rpc_object_base, Rpc_dispatcher { @@ -339,7 +335,6 @@ class Genode::Rpc_entrypoint : Thread, public Object_pool Native_capability _caller { }; Blockade _cap_valid { }; /* thread startup synchronization */ - Blockade _delay_start { }; /* delay start of request dispatching */ Blockade _delay_exit { }; /* delay destructor until server settled */ Pd_session &_pd_session; /* for creating capabilities */ Exit_handler _exit_handler { }; @@ -412,8 +407,7 @@ class Genode::Rpc_entrypoint : Thread, public Object_pool * \param location CPU affinity */ Rpc_entrypoint(Pd_session *pd_session, size_t stack_size, - char const *name, bool start_on_construction = true, - Affinity::Location location = Affinity::Location()); + char const *name, Affinity::Location location); ~Rpc_entrypoint(); @@ -436,11 +430,6 @@ class Genode::Rpc_entrypoint : Thread, public Object_pool _dissolve(obj); } - /** - * Activate entrypoint, start processing RPC requests - */ - void activate(); - /** * Request reply capability for current call * diff --git a/repos/base/lib/symbols/ld b/repos/base/lib/symbols/ld index 0fd4ffda59..3045198bae 100644 --- a/repos/base/lib/symbols/ld +++ b/repos/base/lib/symbols/ld @@ -115,10 +115,8 @@ _ZN6Genode14Rpc_entrypoint17reply_signal_infoENS_17Native_capabilityEmm T _ZN6Genode14Rpc_entrypoint22_block_until_cap_validEv T _ZN6Genode14Rpc_entrypoint5entryEv T _ZN6Genode14Rpc_entrypoint7_manageEPNS_15Rpc_object_baseE T -_ZN6Genode14Rpc_entrypoint8activateEv T _ZN6Genode14Rpc_entrypoint9_dissolveEPNS_15Rpc_object_baseE T -_ZN6Genode14Rpc_entrypointC1EPNS_10Pd_sessionEmPKcbNS_8Affinity8LocationE T -_ZN6Genode14Rpc_entrypointC2EPNS_10Pd_sessionEmPKcbNS_8Affinity8LocationE T +_ZN6Genode14Rpc_entrypointC1EPNS_10Pd_sessionEmPKcNS_8Affinity8LocationE T _ZN6Genode14Rpc_entrypointD0Ev T _ZN6Genode14Rpc_entrypointD1Ev T _ZN6Genode14Rpc_entrypointD2Ev T diff --git a/repos/base/src/core/include/core_env.h b/repos/base/src/core/include/core_env.h index 4afb14e0b1..2b145f61d0 100644 --- a/repos/base/src/core/include/core_env.h +++ b/repos/base/src/core/include/core_env.h @@ -58,7 +58,8 @@ class Genode::Core_env : public Env_deprecated, Noncopyable Core_env() : - _entrypoint(nullptr, ENTRYPOINT_STACK_SIZE, "entrypoint"), + _entrypoint(nullptr, ENTRYPOINT_STACK_SIZE, "entrypoint", + Affinity::Location()), _region_map(_entrypoint), _pd_session(_entrypoint, _entrypoint, diff --git a/repos/base/src/core/include/io_port_root.h b/repos/base/src/core/include/io_port_root.h index c5474eb43d..b9ae0de144 100644 --- a/repos/base/src/core/include/io_port_root.h +++ b/repos/base/src/core/include/io_port_root.h @@ -30,7 +30,7 @@ namespace Genode { public: Io_port_handler(Pd_session &pd_session) : - _ep(&pd_session, STACK_SIZE, "ioport") + _ep(&pd_session, STACK_SIZE, "ioport", Affinity::Location()) { } Rpc_entrypoint &entrypoint() { return _ep; } diff --git a/repos/base/src/core/include/irq_root.h b/repos/base/src/core/include/irq_root.h index 3ee982f96a..9a8815122b 100644 --- a/repos/base/src/core/include/irq_root.h +++ b/repos/base/src/core/include/irq_root.h @@ -55,7 +55,7 @@ class Genode::Irq_root : public Root_component Range_allocator &irq_alloc, Allocator &md_alloc) : Root_component(&_session_ep, &md_alloc), - _session_ep(&pd_session, STACK_SIZE, "irq"), + _session_ep(&pd_session, STACK_SIZE, "irq", Affinity::Location()), _irq_alloc(irq_alloc) { } }; diff --git a/repos/base/src/core/signal_transmitter_proxy.cc b/repos/base/src/core/signal_transmitter_proxy.cc index 0063254713..3d861a3849 100644 --- a/repos/base/src/core/signal_transmitter_proxy.cc +++ b/repos/base/src/core/signal_transmitter_proxy.cc @@ -58,6 +58,6 @@ void Signal_transmitter::submit(unsigned cnt) Rpc_entrypoint &Core_env::signal_ep() { static Rpc_entrypoint ep(nullptr, ENTRYPOINT_STACK_SIZE, - "signal_entrypoint"); + "signal_entrypoint", Affinity::Location()); return ep; } diff --git a/repos/base/src/lib/base/entrypoint.cc b/repos/base/src/lib/base/entrypoint.cc index 9c3b9ceb42..8b6981a293 100644 --- a/repos/base/src/lib/base/entrypoint.cc +++ b/repos/base/src/lib/base/entrypoint.cc @@ -157,7 +157,7 @@ void Entrypoint::_process_incoming_signals() init_signal_thread(_env); - _rpc_ep.construct(&_env.pd(), Component::stack_size(), initial_ep_name()); + _rpc_ep.construct(&_env.pd(), Component::stack_size(), initial_ep_name(), Affinity::Location()); init_heartbeat_monitoring(_env); _signal_proxy_cap = manage(_signal_proxy); _sig_rec.construct(); @@ -330,7 +330,7 @@ namespace { Entrypoint::Entrypoint(Env &env) : _env(env), - _rpc_ep(&env.pd(), Component::stack_size(), initial_ep_name()), + _rpc_ep(&env.pd(), Component::stack_size(), initial_ep_name(), Affinity::Location()), /* initialize signalling before creating the first signal receiver */ _signalling_initialized((init_signal_thread(env), true)) @@ -365,7 +365,7 @@ Entrypoint::Entrypoint(Env &env, size_t stack_size, char const *name, Affinity::Location location) : _env(env), - _rpc_ep(&env.pd(), stack_size, name, true, location), + _rpc_ep(&env.pd(), stack_size, name, location), _signalling_initialized(true) { _signal_proxy_thread.construct(env, *this, location, diff --git a/repos/base/src/lib/base/rpc_dispatch_loop.cc b/repos/base/src/lib/base/rpc_dispatch_loop.cc index 52669d2fe9..5fc801983d 100644 --- a/repos/base/src/lib/base/rpc_dispatch_loop.cc +++ b/repos/base/src/lib/base/rpc_dispatch_loop.cc @@ -50,15 +50,6 @@ void Rpc_entrypoint::entry() _cap = srv; _cap_valid.wakeup(); - /* - * Now, the capability of the server activation is initialized - * an can be passed around. However, the processing of capability - * invocations should not happen until activation-using server - * is completely initialized. Thus, we wait until the activation - * gets explicitly unblocked by calling 'Rpc_entrypoint::activate()'. - */ - _delay_start.block(); - Rpc_exception_code exc = Rpc_exception_code(Rpc_exception_code::INVALID_OBJECT); while (!_exit_handler.exit) { diff --git a/repos/base/src/lib/base/rpc_entrypoint.cc b/repos/base/src/lib/base/rpc_entrypoint.cc index 3ffd8a268d..d1dabffe62 100644 --- a/repos/base/src/lib/base/rpc_entrypoint.cc +++ b/repos/base/src/lib/base/rpc_entrypoint.cc @@ -57,12 +57,6 @@ void Rpc_entrypoint::reply_signal_info(Untyped_capability reply_cap, } -void Rpc_entrypoint::activate() -{ - _delay_start.wakeup(); -} - - bool Rpc_entrypoint::is_myself() const { return (Thread::myself() == this); @@ -70,8 +64,7 @@ bool Rpc_entrypoint::is_myself() const Rpc_entrypoint::Rpc_entrypoint(Pd_session *pd_session, size_t stack_size, - char const *name, bool start_on_construction, - Affinity::Location location) + char const *name, Affinity::Location location) : Thread(Cpu_session::Weight::DEFAULT_WEIGHT, name, stack_size, location), _cap(Untyped_capability()), @@ -80,21 +73,12 @@ Rpc_entrypoint::Rpc_entrypoint(Pd_session *pd_session, size_t stack_size, Thread::start(); _block_until_cap_valid(); - if (start_on_construction) - activate(); - _exit_cap = manage(&_exit_handler); } Rpc_entrypoint::~Rpc_entrypoint() { - /* - * We have to make sure the server loop is running which is only the case - * if the Rpc_entrypoint was activated before we execute the RPC call. - */ - _delay_start.wakeup(); - /* leave server loop */ _exit_cap.call(); diff --git a/repos/base/src/test/smp/main.cc b/repos/base/src/test/smp/main.cc index a775ee208d..6eb9b2d540 100644 --- a/repos/base/src/test/smp/main.cc +++ b/repos/base/src/test/smp/main.cc @@ -82,7 +82,7 @@ namespace Mp_server_test { Client cli { cap }; Cpu_compound(Genode::Affinity::Location l, Genode::Env &env) - : rpc(&env.pd(), STACK_SIZE, "rpc en", true, l) {} + : rpc(&env.pd(), STACK_SIZE, "rpc en", l) {} ~Cpu_compound() { rpc.dissolve(&comp); } };