ealanos: Groom habitat and free CPU cores when a cell is destroyed involuntarily, e.g. through removing its start node from the habitat's configuration.

repos/ealanos/include/sandbox/sandbox.h

@@ -14,6 +14,7 @@
 #ifndef _INCLUDE__SANDBOX__SANDBOX_H_
 #define _INCLUDE__SANDBOX__SANDBOX_H_
 
+#include "base/mutex.h"
 #include <util/xml_node.h>
 #include <util/callable.h>
 #include <util/noncopyable.h>
@@ -115,7 +116,7 @@ class Genode::Sandbox : Noncopyable
 		 */
 		void generate_state_report(Xml_generator &) const;
 
-		Xml_node* update(::Sandbox::Child &child, Xml_node *config);
+		Xml_node* update(::Sandbox::Child &child, Xml_node *config, Genode::Mutex &config_lock);
 };
 
 class Genode::Sandbox::Local_service_base : public Service

repos/ealanos/src/hoitaja/main.cc

@@ -142,22 +142,7 @@ class Ealan::Hoitaja : Genode::Sandbox::State_handler, Genode::Sandbox::Local_se
 		void handle_sandbox_state() override
 		{
 			Genode::log("Sandbox state changed");
-			try
+			_sandbox.apply_config(*_habitat_config);
-			{
-				Reporter::Xml_generator xml(*_reporter, [&] () {
-					_sandbox.generate_state_report(xml); });
-			}
-			catch (Xml_generator::Buffer_exceeded) {
-
-				error("state report exceeds maximum size");
-
-				/* try to reflect the error condition as state report */
-				try {
-					Reporter::Xml_generator xml(*_reporter, [&] () {
-						xml.attribute("error", "report buffer exceeded"); });
-				}
-				catch (...) { }
-			}
 		}
 
 		void handle_child_state(::Sandbox::Child &child) override {
@@ -165,22 +150,16 @@ class Ealan::Hoitaja : Genode::Sandbox::State_handler, Genode::Sandbox::Local_se
 			do
 			{
 				try {
-					_config_lock.acquire();
+					_habitat_config = _sandbox.update(child, _habitat_config, _config_lock);
-					Genode::log("Updating state of child ", child.name());
-					_habitat_config = _sandbox.update(child, _habitat_config);
-					Genode::log("Updated config length:", _habitat_config->content_size());
-					_config_lock.release();
 				}
 				catch (Genode::Quota_guard<Genode::Cap_quota>::Limit_exceeded)
 				{
 					Genode::log("Caps exceeded while handling child state");
-					_config_lock.release();
 					_env.parent().exit(1);
 				}
 				catch (Genode::Ipc_error)
 				{
 					Genode::error("Failed to update child state for <", child.name(), ">");
-					_config_lock.release();
 					repeat = true;
 				}
 			} while (repeat);

repos/ealanos/src/lib/sandbox/child.h

@@ -654,7 +654,6 @@ class Sandbox::Child : Child_policy, Routed_service::Wakeup
 					 	_cell_cap = _habitat.create_cell(_child.pd_session_cap(), _resources.affinity, static_cast<uint16_t>(_priority), Genode::Session_label(_unique_name), _is_brick);
 					} catch (Ealan::Cell::Cell_creation_error) {
 						Genode::error("Failed to create cell");
-						abandon();
 					}
 					 Genode::log("Created new cell ", _unique_name, " ", _cell_cap);
 
@@ -826,7 +825,8 @@ class Sandbox::Child : Child_policy, Routed_service::Wakeup
 
 		void session_state_changed() override
 		{
-			_report_update_trigger.trigger_report_update();
+			_habitat_handler.handle_child_state(*this);
+			//_report_update_trigger.trigger_report_update();
 		}
 
 		bool initiate_env_sessions() const override { return false; }

repos/ealanos/src/lib/sandbox/core_allocator.h

@@ -31,7 +31,7 @@ class Hoitaja::Core_allocator
     private:
         Genode::Affinity::Space &_affinity_space;
 
-        ::Sandbox::Prio_levels &_prio_levels;
+        ::Sandbox::Prio_levels _prio_levels;
 
         double _resource_coeff; // Coefficient used for calculating resource shares
 
@@ -45,7 +45,7 @@ class Hoitaja::Core_allocator
 
         Core_allocator(Genode::Affinity::Space &affinity_space, ::Sandbox::Prio_levels prio_levels) : _affinity_space(affinity_space), _prio_levels(prio_levels), _resource_coeff(0.0), _cores_for_cells(_affinity_space.total())
         {
-            Genode::log("Created core allocator for ", affinity_space.total(), " cores and ", prio_levels.value, " priorities.");
+            Genode::log("Created core allocator for ", affinity_space.total(), " cores and ", _prio_levels.value, " priorities.");
             //Nova::create_habitat(0, affinity_space.total());
         }
 
@@ -54,17 +54,20 @@ class Hoitaja::Core_allocator
         }
 
         Genode::Affinity::Location allocate_cores_for_cell(Genode::Xml_node const &start_node)
-        {
+		{
             if (::Sandbox::is_brick_from_xml(start_node)) {
                 Genode::Affinity::Location brick = ::Sandbox::affinity_location_from_xml(_affinity_space, start_node);
                 _cores_for_cells -= brick.width();
                 return brick;
             }
 
-            // Calculate affinity from global affinity space and priority
+			// Calculate affinity from global affinity space and priority
+			Genode::log("Allocating cores for cell in habitat with ", _prio_levels.value,
+			            " priorities.");
             long priority = ::Sandbox::priority_from_xml(start_node, _prio_levels);
             priority = (priority >= 0) ? 1 : priority;
             _resource_coeff += (1.0/static_cast<double>(priority)); // treat priority 0 same as 1, to avoid division by zero here
+			Genode::log("Resource coefficient: ", _resource_coeff);
 
             unsigned int cores_share = _calculate_resource_share(priority);
             
@@ -81,7 +84,8 @@ class Hoitaja::Core_allocator
 		
             /* Remove cell's coefficient from the global resource coefficient.
              * This is necessary in order to be able to redistribute the freed resources correctly. We do not trigger the redistribution itself here, because the child has not been fully destroyed yet, thus its resources might still be occupied at this point. */
-            _resource_coeff -= 1.0 / static_cast<double>(cell.resources().priority);
+            _resource_coeff -= 1.0 / static_cast<double>(cell.resources().priority == 0? 1 : cell.resources().priority);
+			Genode::log("Resource coefficient after free: ", _resource_coeff);
         }
 
         /**

repos/ealanos/src/lib/sandbox/library.cc

@@ -12,6 +12,8 @@
  */
 
 /* Genode includes */
+#include "base/child.h"
+#include "base/mutex.h"
 #include <base/attached_rom_dataspace.h>
 #include <sandbox/sandbox.h>
 
@@ -214,6 +216,8 @@ struct Genode::Sandbox::Library : ::Sandbox::State_reporter::Producer,
 	void _update_children_config(Xml_node const &);
 	void _destroy_abandoned_parent_services();
 	void _destroy_abandoned_children();
+	void _remove_leftovers(Child &child);
+	void _groom();
 
 	Server _server { _env, _heap, _child_services, _state_reporter };
 
@@ -310,13 +314,14 @@ struct Genode::Sandbox::Library : ::Sandbox::State_reporter::Producer,
 
 	void maintain_cells();
 
-	Genode::Xml_node* update(Child &child, Genode::Xml_node *config) {
+	Genode::Xml_node* update(Child &child, Genode::Xml_node *config, Genode::Mutex &config_lock) {
 		if (child.exited()) {
 			_children.remove(&child);
 			_core_allocator->free_cores_from_cell(child);
 			/* Remove child from config */
 			try {
 				/* Find XML node for the child */
+				config_lock.acquire();
 				Xml_node node = config->sub_node("start");
 				while (node.attribute_value<Genode::Child_policy::Name>("name", Genode::Child_policy::Name()) != child.name())
 				{
@@ -348,6 +353,7 @@ struct Genode::Sandbox::Library : ::Sandbox::State_reporter::Producer,
 				Xml_node *new_config = new (_heap) Xml_node(config_ptr, config_len - len);
 				_heap.free(config, sizeof(Xml_node));
 				config = new_config;
+				config_lock.release();
 			}
 			catch (Genode::Xml_node::Nonexistent_sub_node)
 			{
@@ -357,9 +363,13 @@ struct Genode::Sandbox::Library : ::Sandbox::State_reporter::Producer,
 
 			Genode::log("Removed child ", child.name());
 			_habitat->groom();
+			config_lock.acquire();
 			apply_config(*config);
+			config_lock.release();
 			maintain_cells();
-		}
+		} else {
+			_groom();
+		} 
 		return config;
 	}
 };
@@ -386,21 +396,34 @@ void Genode::Sandbox::Library::_destroy_abandoned_children()
 
 		/* destroy child once all environment sessions are gone */
 		if (child.env_sessions_closed()) {
-			_core_allocator->free_cores_from_cell(child);
+			_remove_leftovers(child);
-			_children.remove(&child);
-
-			Cpu_quota const child_cpu_quota = child.cpu_quota();
-
-			destroy(_heap, &child);
-
-			/* replenish available CPU quota */
-			_avail_cpu.percent       += child_cpu_quota.percent;
-			_transferred_cpu.percent -= min(_transferred_cpu.percent,
-			                                child_cpu_quota.percent);
 		}
 	});
 }
 
+void Genode::Sandbox::Library::_remove_leftovers(Child &child)
+{
+	Genode::log("Destroying abandoned child ", child.name());
+	_core_allocator->free_cores_from_cell(child);
+	_children.remove(&child);
+
+	Cpu_quota const child_cpu_quota = child.cpu_quota();
+
+	destroy(_heap, &child);
+
+	/* replenish available CPU quota */
+	_avail_cpu.percent       += child_cpu_quota.percent;
+	_transferred_cpu.percent -= min(_transferred_cpu.percent,
+									child_cpu_quota.percent);
+}
+
+void Genode::Sandbox::Library::_groom()
+{
+	_children.for_each_child([&](Child &child) {
+		if (child.env_sessions_closed())
+			_remove_leftovers(child);	
+	});
+}
 
 bool Genode::Sandbox::Library::ready_to_create_child(Start_model::Name    const &name,
                                                      Start_model::Version const &version) const
@@ -451,6 +474,7 @@ bool Genode::Sandbox::Library::ready_to_create_child(Start_model::Name    const
 	}
 
 	try {
+		log("Creating child ", start_node.attribute_value("name", Child_policy::Name()), "in habitat with ", _prio_levels.value, " priority levels.");
 		Child &child = *new (_heap)
 			Child(_env, _heap, *_verbose,
 			      Child::Id { ++_child_cnt }, _state_reporter,
@@ -752,9 +776,9 @@ void Genode::Sandbox::generate_state_report(Xml_generator &xml) const
 	_library.generate_state_report(xml);
 }
 
-Genode::Xml_node* Genode::Sandbox::update(::Sandbox::Child &child, Genode::Xml_node *config)
+Genode::Xml_node* Genode::Sandbox::update(::Sandbox::Child &child, Genode::Xml_node *config, Genode::Mutex &_config_lock)
 {
-	return _library.update(child, config);
+	return _library.update(child, config, _config_lock);
 }
 
 Genode::Sandbox::Sandbox(Env &env, State_handler &state_handler, Pd_intrinsics &pd_intrinsics)