From 73e8d64c344b87301855b33341eb778792ecf6bb Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Thu, 1 Apr 2021 16:32:59 +0200 Subject: [PATCH] init/sandbox: avoid repetitive state reports Fixes #4064 --- repos/os/src/lib/sandbox/child.cc | 25 +++++++++---- repos/os/src/lib/sandbox/child.h | 33 ++++++++++++++++- repos/os/src/lib/sandbox/child_registry.h | 11 ++++++ repos/os/src/lib/sandbox/library.cc | 12 +++++-- repos/os/src/lib/sandbox/state_reporter.h | 23 ++++++++++-- repos/os/src/lib/sandbox/types.h | 43 +++++++++++++++++++++++ repos/os/src/lib/sandbox/utils.h | 17 --------- 7 files changed, 133 insertions(+), 31 deletions(-) diff --git a/repos/os/src/lib/sandbox/child.cc b/repos/os/src/lib/sandbox/child.cc index 41eb719fc6..0a39c8cda9 100644 --- a/repos/os/src/lib/sandbox/child.cc +++ b/repos/os/src/lib/sandbox/child.cc @@ -327,9 +327,6 @@ void Sandbox::Child::report_state(Xml_generator &xml, Report_detail const &detai if (abandoned()) return; - /* true if it's safe to call the PD for requesting resource information */ - bool const pd_alive = !abandoned() && !_exited; - xml.node("child", [&] () { xml.attribute("name", _unique_name); @@ -356,8 +353,8 @@ void Sandbox::Child::report_state(Xml_generator &xml, Report_detail const &detai xml.attribute("assigned", String<32> { Number_of_bytes(_resources.assigned_ram_quota.value) }); - if (pd_alive) - generate_ram_info(xml, _child.pd()); + if (_pd_alive()) + Ram_info::from_pd(_child.pd()).generate(xml); if (_requested_resources.constructed() && _requested_resources->ram.value) xml.attribute("requested", String<32>(_requested_resources->ram)); @@ -369,8 +366,8 @@ void Sandbox::Child::report_state(Xml_generator &xml, Report_detail const &detai xml.attribute("assigned", String<32>(_resources.assigned_cap_quota)); - if (pd_alive) - generate_caps_info(xml, _child.pd()); + if (_pd_alive()) + Cap_info::from_pd(_child.pd()).generate(xml); if (_requested_resources.constructed() && _requested_resources->caps.value) xml.attribute("requested", String<32>(_requested_resources->caps)); @@ -402,6 +399,20 @@ void Sandbox::Child::report_state(Xml_generator &xml, Report_detail const &detai } +Sandbox::Child::Sample_state_result Sandbox::Child::sample_state() +{ + if (!_pd_alive()) + return Sample_state_result::UNCHANGED; + + Sampled_state const orig_state = _sampled_state; + + _sampled_state = Sampled_state::from_pd(_child.pd()); + + return (orig_state != _sampled_state) ? Sample_state_result::CHANGED + : Sample_state_result::UNCHANGED; +} + + void Sandbox::Child::init(Pd_session &session, Pd_session_capability cap) { session.ref_account(_env.pd_session_cap()); diff --git a/repos/os/src/lib/sandbox/child.h b/repos/os/src/lib/sandbox/child.h index f338597f9c..e0385616f6 100644 --- a/repos/os/src/lib/sandbox/child.h +++ b/repos/os/src/lib/sandbox/child.h @@ -64,6 +64,8 @@ class Sandbox::Child : Child_policy, Routed_service::Wakeup typedef Resource_limit_accessor Ram_limit_accessor; typedef Resource_limit_accessor Cap_limit_accessor; + enum class Sample_state_result { CHANGED, UNCHANGED }; + private: friend class Child_registry; @@ -446,8 +448,35 @@ class Sandbox::Child : Child_policy, Routed_service::Wakeup bool _exited { false }; int _exit_value { -1 }; + /** + * Return true if it's safe to call the PD for requesting resource + * information + */ + bool _pd_alive() const + { + return !abandoned() && !_exited; + } + void _destroy_services(); + struct Sampled_state + { + Ram_info ram; + Cap_info caps; + + static Sampled_state from_pd(Pd_session &pd) + { + return { .ram = Ram_info::from_pd(pd), + .caps = Cap_info::from_pd(pd) }; + } + + bool operator != (Sampled_state const &other) const + { + return (ram != other.ram) || (caps != other.caps); + } + + } _sampled_state { }; + public: /** @@ -581,7 +610,9 @@ class Sandbox::Child : Child_policy, Routed_service::Wakeup return _heartbeat_expected() ? _child.skipped_heartbeats() : 0; } - void report_state(Xml_generator &xml, Report_detail const &detail) const; + void report_state(Xml_generator &, Report_detail const &) const; + + Sample_state_result sample_state(); /**************************** diff --git a/repos/os/src/lib/sandbox/child_registry.h b/repos/os/src/lib/sandbox/child_registry.h index a328ae5de7..9b0cbfc03c 100644 --- a/repos/os/src/lib/sandbox/child_registry.h +++ b/repos/os/src/lib/sandbox/child_registry.h @@ -136,6 +136,17 @@ class Sandbox::Child_registry : public Name_registry, Child_list } } + Child::Sample_state_result sample_state() + { + auto result = Child::Sample_state_result::UNCHANGED; + + for_each_child([&] (Child &child) { + if (result == Child::Sample_state_result::UNCHANGED) + result = child.sample_state(); }); + + return result; + } + Child::Name deref_alias(Child::Name const &name) override { for (Alias const *a = _aliases.first(); a; a = a->next()) diff --git a/repos/os/src/lib/sandbox/library.cc b/repos/os/src/lib/sandbox/library.cc index 2c5ba133b9..5023c06ece 100644 --- a/repos/os/src/lib/sandbox/library.cc +++ b/repos/os/src/lib/sandbox/library.cc @@ -39,6 +39,8 @@ struct Genode::Sandbox::Library : ::Sandbox::State_reporter::Producer, using Alias = ::Sandbox::Alias; using Child = ::Sandbox::Child; using Prio_levels = ::Sandbox::Prio_levels; + using Ram_info = ::Sandbox::Ram_info; + using Cap_info = ::Sandbox::Cap_info; Env &_env; Heap &_heap; @@ -122,15 +124,20 @@ struct Genode::Sandbox::Library : ::Sandbox::State_reporter::Producer, void produce_state_report(Xml_generator &xml, Report_detail const &detail) const override { if (detail.init_ram()) - xml.node("ram", [&] () { ::Sandbox::generate_ram_info (xml, _env.pd()); }); + xml.node("ram", [&] () { Ram_info::from_pd(_env.pd()).generate(xml); }); if (detail.init_caps()) - xml.node("caps", [&] () { ::Sandbox::generate_caps_info(xml, _env.pd()); }); + xml.node("caps", [&] () { Cap_info::from_pd(_env.pd()).generate(xml); }); if (detail.children()) _children.report_state(xml, detail); } + Child::Sample_state_result sample_children_state() override + { + return _children.sample_state(); + } + /** * Default_route_accessor interface */ @@ -630,4 +637,3 @@ Genode::Sandbox::Sandbox(Env &env, State_handler &state_handler) _heap(env.ram(), env.rm()), _library(*new (_heap) Library(env, _heap, _local_services, state_handler)) { } - diff --git a/repos/os/src/lib/sandbox/state_reporter.h b/repos/os/src/lib/sandbox/state_reporter.h index 52d3c97653..abb65e0fd6 100644 --- a/repos/os/src/lib/sandbox/state_reporter.h +++ b/repos/os/src/lib/sandbox/state_reporter.h @@ -20,6 +20,7 @@ /* local includes */ #include "report.h" +#include "child.h" namespace Sandbox { class State_reporter; } @@ -32,6 +33,8 @@ class Sandbox::State_reporter : public Report_update_trigger { virtual void produce_state_report(Xml_generator &xml, Report_detail const &) const = 0; + + virtual Child::Sample_state_result sample_children_state() = 0; }; private: @@ -60,7 +63,7 @@ class Sandbox::State_reporter : public Report_update_trigger _env.ep(), *this, &State_reporter::_handle_timer }; Signal_handler _timer_periodic_handler { - _env.ep(), *this, &State_reporter::_handle_timer }; + _env.ep(), *this, &State_reporter::_handle_periodic_timer }; Signal_handler _immediate_handler { _env.ep(), *this, &State_reporter::_handle_timer }; @@ -69,6 +72,21 @@ class Sandbox::State_reporter : public Report_update_trigger State_handler &_state_handler; + bool _periodic_sampling_needed() const + { + return _report_detail->child_ram() + || _report_detail->child_caps(); + } + + void _handle_periodic_timer() + { + if (!_periodic_sampling_needed()) + return; + + if (_producer.sample_children_state() == Child::Sample_state_result::CHANGED) + _handle_timer(); + } + void _handle_timer() { _scheduled = false; @@ -140,8 +158,7 @@ class Sandbox::State_reporter : public Report_update_trigger */ uint64_t const period_ms = max(1000U, _report_delay_ms); bool const period_changed = (_report_period_ms != period_ms); - bool const report_periodically = _report_detail->child_ram() - || _report_detail->child_caps(); + bool const report_periodically = _periodic_sampling_needed(); if (report_periodically && !_timer_periodic.constructed()) { _timer_periodic.construct(_env); diff --git a/repos/os/src/lib/sandbox/types.h b/repos/os/src/lib/sandbox/types.h index 25e2fc0093..7e54780b6f 100644 --- a/repos/os/src/lib/sandbox/types.h +++ b/repos/os/src/lib/sandbox/types.h @@ -29,6 +29,49 @@ namespace Sandbox { struct Prio_levels { long value; }; typedef List > Child_list; + + template + struct Resource_info + { + T quota, used, avail; + + static Resource_info from_pd(Pd_session const &pd); + + void generate(Xml_generator &xml) const + { + typedef String<32> Value; + xml.attribute("quota", Value(quota)); + xml.attribute("used", Value(used)); + xml.attribute("avail", Value(avail)); + } + + bool operator != (Resource_info const &other) const + { + return quota.value != other.quota.value + || used.value != other.used.value + || avail.value != other.avail.value; + } + }; + + typedef Resource_info Ram_info; + typedef Resource_info Cap_info; + + template <> + inline Ram_info Ram_info::from_pd(Pd_session const &pd) + { + return { .quota = pd.ram_quota(), + .used = pd.used_ram(), + .avail = pd.avail_ram() }; + } + + template <> + inline Cap_info Cap_info::from_pd(Pd_session const &pd) + { + return { .quota = pd.cap_quota(), + .used = pd.used_caps(), + .avail = pd.avail_caps() }; + } + } #endif /* _LIB__SANDBOX__TYPES_H_ */ diff --git a/repos/os/src/lib/sandbox/utils.h b/repos/os/src/lib/sandbox/utils.h index 7afcaebf00..a21fcca8ea 100644 --- a/repos/os/src/lib/sandbox/utils.h +++ b/repos/os/src/lib/sandbox/utils.h @@ -161,23 +161,6 @@ namespace Sandbox { return *service; } - - inline void generate_ram_info(Xml_generator &xml, Pd_session const &pd) - { - typedef String<32> Value; - xml.attribute("quota", Value(pd.ram_quota())); - xml.attribute("used", Value(pd.used_ram())); - xml.attribute("avail", Value(pd.avail_ram())); - } - - inline void generate_caps_info(Xml_generator &xml, Pd_session const &pd) - { - typedef String<32> Value; - xml.attribute("quota", Value(pd.cap_quota())); - xml.attribute("used", Value(pd.used_caps())); - xml.attribute("avail", Value(pd.avail_caps())); - } - /** * Read priority-levels declaration from config */