Remove obsolete Trace::Session::subject_info RPC

Issue #3610
Fixes #4349

repos/base-okl4/src/lib/base/ipc.cc

@@ -111,7 +111,7 @@ static void copy_msg_to_utcb(Msgbuf_base const &snd_msg, L4_Word_t local_name)
 	uint8_t const num_msg_words    = num_data_words + num_header_words;
 
 	if (num_msg_words >= L4_GetMessageRegisters()) {
-		raw("Message does not fit into UTCB message registers");
+		raw("Message does not fit into UTCB message registers, num_msg_words=", num_msg_words);
 		L4_LoadMR(0, 0);
 		return;
 	}

repos/base/include/trace_session/client.h

@@ -126,9 +126,6 @@ struct Genode::Trace::Session_client : Genode::Rpc_client<Genode::Trace::Session
 		void resume(Subject_id subject) override {
 			call<Rpc_resume>(subject); }
 
-		Subject_info subject_info(Subject_id subject) override {
-			return call<Rpc_subject_info>(subject); }
-
 		Dataspace_capability buffer(Subject_id subject) override {
 			return call<Rpc_buffer>(subject); }
 

repos/base/include/trace_session/trace_session.h

@@ -85,13 +85,6 @@ struct Genode::Trace::Session : Genode::Session
 	 */
 	virtual void resume(Subject_id) = 0;
 
-	/**
-	 * Obtain details about tracing subject
-	 *
-	 * \throw Nonexistent_subject
-	 */
-	virtual Subject_info subject_info(Subject_id) = 0;
-
 	/**
 	 * Obtain trace buffer of given subject
 	 *
@@ -144,8 +137,6 @@ struct Genode::Trace::Session : Genode::Session
 	                 GENODE_TYPE_LIST(Out_of_ram, Out_of_caps));
 	GENODE_RPC_THROW(Rpc_subject_infos, size_t, subject_infos,
 	                 GENODE_TYPE_LIST(Out_of_ram, Out_of_caps));
-	GENODE_RPC_THROW(Rpc_subject_info, Subject_info, subject_info,
-	                 GENODE_TYPE_LIST(Nonexistent_subject), Subject_id);
 	GENODE_RPC_THROW(Rpc_buffer, Dataspace_capability, buffer,
 	                 GENODE_TYPE_LIST(Nonexistent_subject, Subject_not_traced),
 	                 Subject_id);
@@ -154,7 +145,7 @@ struct Genode::Trace::Session : Genode::Session
 
 	GENODE_RPC_INTERFACE(Rpc_dataspace, Rpc_alloc_policy, Rpc_policy,
 	                     Rpc_unload_policy, Rpc_trace, Rpc_rule, Rpc_pause,
-	                     Rpc_resume, Rpc_subjects, Rpc_subject_info, Rpc_buffer,
+	                     Rpc_resume, Rpc_subjects, Rpc_buffer,
 	                     Rpc_free, Rpc_subject_infos);
 };
 

repos/base/src/core/include/trace/session_component.h

@@ -81,7 +81,6 @@ class Genode::Trace::Session_component
 		void rule(Session_label const &, Thread_name const &, Policy_id, size_t) override;
 		void pause(Subject_id) override;
 		void resume(Subject_id) override;
-		Subject_info subject_info(Subject_id) override;
 		Dataspace_capability buffer(Subject_id) override;
 		void free(Subject_id) override;
 };

repos/base/src/core/trace_session_component.cc

@@ -146,12 +146,6 @@ void Session_component::resume(Subject_id subject_id)
 }
 
 
-Subject_info Session_component::subject_info(Subject_id subject_id)
-{
-	return _subjects.lookup_by_id(subject_id).info();
-}
-
-
 Dataspace_capability Session_component::buffer(Subject_id subject_id)
 {
 	return _subjects.lookup_by_id(subject_id).buffer();

repos/base/src/test/migrate/main.cc

@@ -65,7 +65,6 @@ struct Migrate
 	Signal_handler<Migrate> timer_handler { env.ep(), *this,
 	                                        &Migrate::check_traces };
 
-	Trace::Subject_id  trace_id { };
 	Affinity::Location location { };
 	unsigned           loc_same { 0 };
 	unsigned           loc_pos  { 0 };
@@ -95,13 +94,12 @@ struct Migrate
 		switch (state) {
 		case LOOKUP_TRACE_ID:
 		{
-			auto count = trace.for_each_subject_info([&](Trace::Subject_id const &id,
+			auto count = trace.for_each_subject_info([&](Trace::Subject_id const &,
 			                                             Trace::Subject_info const &info)
 			{
 				if (info.thread_name() != "migrate")
 					return;
 
-				trace_id = id;
 				location = info.affinity();
 				state    = CHECK_AFFINITY;
 
@@ -116,32 +114,38 @@ struct Migrate
 		}
 		case CHECK_AFFINITY:
 		{
-			Trace::Subject_info const info = trace.subject_info(trace_id);
+			trace.for_each_subject_info([&](Trace::Subject_id const &,
-
+			                                Trace::Subject_info const &info)
-			loc_same ++;
-
-			if ((location.xpos() == info.affinity().xpos()) &&
-			    (location.ypos() == info.affinity().ypos()) &&
-			    (location.width() == info.affinity().width()) &&
-			    (location.height() == info.affinity().height()))
 			{
-				if (loc_same >= 1) {
+				if (info.thread_name() != "migrate")
-					loc_same = 0;
+					return;
-					state = MIGRATE;
+
+				loc_same ++;
+
+				if ((location.xpos() == info.affinity().xpos()) &&
+				    (location.ypos() == info.affinity().ypos()) &&
+				    (location.width() == info.affinity().width()) &&
+				    (location.height() == info.affinity().height()))
+				{
+					if (loc_same >= 1) {
+						loc_same = 0;
+						state = MIGRATE;
+					}
+					log ("[ep] .");
+					return;
 				}
-				log ("[ep] .");
-				break;
-			}
 
-			loc_same = 0;
+				loc_same = 0;
-			location = info.affinity();
+				location = info.affinity();
 
-			log("[ep] thread '", info.thread_name(), "' migrated,",
+				log("[ep] thread '", info.thread_name(), "' migrated,",
-			    " location=", location.xpos(), "x", location.ypos());
+				    " location=", location.xpos(), "x", location.ypos());
+
+				test_rounds ++;
+				if (test_rounds == 4)
+					log("--- test completed successfully ---");
+			});
 
-			test_rounds ++;
-			if (test_rounds == 4)
-				log("--- test completed successfully ---");
 			break;
 		}
 		case MIGRATE:

repos/os/src/app/trace_logger/main.cc

@@ -90,23 +90,22 @@ class Main
 			_monitors_switch = !_monitors_switch;
 
 			/* update available subject IDs and iterate over them */
-			try { _num_subjects = _trace.subjects(_subjects, MAX_SUBJECTS); }
+			_trace.for_each_subject_info([&] (Trace::Subject_id   const id,
-			catch (Out_of_ram ) { warning("Cannot list subjects: Out_of_ram" ); return; }
+			                                  Trace::Subject_info const &info) {
-			catch (Out_of_caps) { warning("Cannot list subjects: Out_of_caps"); return; }
-			for (unsigned i = 0; i < _num_subjects; i++) {
 
-				Trace::Subject_id const id = _subjects[i];
 				try {
 					/* skip dead subjects */
-					if (_trace.subject_info(id).state() == Trace::Subject_info::DEAD)
+					if (info.state() == Trace::Subject_info::DEAD)
-						continue;
+						return;
 
 					/* check if there is a matching policy in the XML config */
-					Session_policy session_policy = _session_policy(id);
+					Session_policy session_policy = _session_policy(info);
 					try {
 						/* lookup monitor by subject ID */
 						Monitor &monitor = old_monitors.find_by_subject_id(id);
 
+						monitor.update_info(info);
+
 						/* move monitor from old to new tree */
 						old_monitors.remove(&monitor);
 						new_monitors.insert(&monitor);
@@ -114,12 +113,13 @@ class Main
 					} catch (Monitor_tree::No_match) {
 
 						/* create monitor for subject in the new tree */
-						_new_monitor(new_monitors, id, session_policy);
+						_new_monitor(new_monitors, id, info, session_policy);
 					}
 				}
-				catch (Trace::Nonexistent_subject       ) { continue; }
+				catch (Trace::Nonexistent_subject       ) { return; }
-				catch (Session_policy::No_policy_defined) { continue; }
+				catch (Session_policy::No_policy_defined) { return; }
-			}
+			});
+
 			/* all monitors in the old tree are deprecated, destroy them */
 			while (Monitor *monitor = old_monitors.first())
 				_destroy_monitor(old_monitors, *monitor);
@@ -144,9 +144,10 @@ class Main
 			_num_monitors--;
 		}
 
-		void _new_monitor(Monitor_tree      &monitors,
+		void _new_monitor(Monitor_tree              &monitors,
-		                  Trace::Subject_id  id,
+		                  Trace::Subject_id   const  id,
-		                  Session_policy    &session_policy)
+		                  Trace::Subject_info const &info,
+		                  Session_policy      const &session_policy)
 		{
 			try {
 				Number_of_bytes const buffer_sz   = session_policy.attribute_value("buffer", _default_buf_sz);
@@ -158,7 +159,7 @@ class Main
 					_policies.insert(policy);
 					_trace.trace(id.id, policy.id(), buffer_sz);
 				}
-				monitors.insert(new (_heap) Monitor(_trace, _env.rm(), id));
+				monitors.insert(new (_heap) Monitor(_trace, _env.rm(), id, info));
 			}
 			catch (Trace::Already_traced         ) { warning("Cannot activate tracing: Already_traced"         ); return; }
 			catch (Trace::Source_is_dead         ) { warning("Cannot activate tracing: Source_is_dead"         ); return; }
@@ -173,9 +174,8 @@ class Main
 				log("new monitor: subject ", id.id);
 		}
 
-		Session_policy _session_policy(Trace::Subject_id id)
+		Session_policy _session_policy(Trace::Subject_info const &info)
 		{
-			Trace::Subject_info info = _trace.subject_info(id);
 			Session_label const label(info.session_label());
 			Session_policy policy(label, _config);
 			if (policy.has_attribute("thread"))

repos/os/src/app/trace_logger/monitor.cc

@@ -58,38 +58,36 @@ Monitor &Monitor::find_by_subject_id(Trace::Subject_id const subject_id)
 }
 
 
-Monitor::Monitor(Trace::Connection &trace,
+Monitor::Monitor(Trace::Connection         &trace,
-                 Region_map        &rm,
+                 Region_map                &rm,
-                 Trace::Subject_id  subject_id)
+                 Trace::Subject_id   const  subject_id,
+                 Trace::Subject_info const &info)
 :
 	Monitor_base(trace, rm, subject_id),
 	_subject_id(subject_id), _buffer(_buffer_raw)
 {
-	_update_info();
+	update_info(info);
 }
 
 
-void Monitor::_update_info()
+void Monitor::update_info(Trace::Subject_info const &info)
 {
 	try {
-		Trace::Subject_info const &info =
-			_trace.subject_info(_subject_id);
-
 		uint64_t const last_execution_time =
 			_info.execution_time().thread_context;
 
 		_info = info;
+
 		_recent_exec_time =
 			_info.execution_time().thread_context - last_execution_time;
 	}
-	catch (Trace::Nonexistent_subject) { warning("Cannot update subject info: Nonexistent_subject"); }
+	catch (Trace::Nonexistent_subject) {
+		warning("Cannot update subject info: Nonexistent_subject"); }
 }
 
 
 void Monitor::print(bool activity, bool affinity)
 {
-	_update_info();
-
 	/* print general subject information */
 	typedef Trace::Subject_info Subject_info;
 	Subject_info::State const state = _info.state();

repos/os/src/app/trace_logger/monitor.h

@@ -56,17 +56,16 @@ class Monitor : public Monitor_base,
 		Genode::Trace::Subject_id const  _subject_id;
 		Trace_buffer                     _buffer;
 		unsigned long                    _report_id        { 0 };
-		Genode::Trace::Subject_info      _info             { };
+		Genode::Trace::Subject_info      _info             {   };
 		unsigned long long               _recent_exec_time { 0 };
 		char                             _curr_entry_data[MAX_ENTRY_LENGTH];
 
-		void _update_info();
-
 	public:
 
-		Monitor(Genode::Trace::Connection &trace,
+		Monitor(Genode::Trace::Connection         &trace,
-		        Genode::Region_map        &rm,
+		        Genode::Region_map                &rm,
-		        Genode::Trace::Subject_id  subject_id);
+		        Genode::Trace::Subject_id          subject_id,
+		        Genode::Trace::Subject_info const &info);
 
 		void print(bool activity, bool affinity);
 
@@ -86,6 +85,8 @@ class Monitor : public Monitor_base,
 
 		Genode::Trace::Subject_id          subject_id() const { return _subject_id; }
 		Genode::Trace::Subject_info const &info()       const { return _info; }
+
+		void update_info(Genode::Trace::Subject_info const &);
 };
 
 

repos/os/src/app/trace_subject_reporter/main.cc

@@ -93,12 +93,8 @@ struct Trace_subject_registry
 
 		void update(Genode::Trace::Connection &trace, Genode::Allocator &alloc)
 		{
-			unsigned const num_subjects = update_subjects(trace);
+			trace.for_each_subject_info([&] (Genode::Trace::Subject_id id,
-
+			                                 Genode::Trace::Subject_info const &info) {
-			/* add and update existing entries */
-			for (unsigned i = 0; i < num_subjects; i++) {
-
-				Genode::Trace::Subject_id const id = _subjects[i];
 
 				Entry *e = _lookup(id);
 				if (!e) {
@@ -106,7 +102,7 @@ struct Trace_subject_registry
 					_entries.insert(e);
 				}
 
-				e->update(trace.subject_info(id));
+				e->update(info);
 
 				/* purge dead threads */
 				if (e->info.state() == Genode::Trace::Subject_info::DEAD) {
@@ -114,7 +110,7 @@ struct Trace_subject_registry
 					_entries.remove(e);
 					Genode::destroy(alloc, e);
 				}
-			}
+			});
 
 			_sort_by_recent_execution_time();
 		}

repos/os/src/server/cpu_balancer/trace.cc

@@ -6,7 +6,7 @@
  */
 
 /*
- * Copyright (C) 2020 Genode Labs GmbH
+ * Copyright (C) 2020-2021 Genode Labs GmbH
  *
  * This file is part of the Genode OS framework, which is distributed
  * under the terms of the GNU Affero General Public License version 3.
@@ -21,38 +21,27 @@ void Cpu::Trace::_read_idle_times(bool skip_max_idle)
 
 	_idle_slot = (_idle_slot + 1) % HISTORY;
 
+	auto count = _trace->for_each_subject_info([&](Subject_id const &,
+	                                               Subject_info const &info)
+	{
+		if (info.session_label() != "kernel" || info.thread_name() != "idle")
+			return;
+
+		if (info.affinity().xpos() < MAX_CORES && info.affinity().ypos() < MAX_THREADS)
+			_idle_times[info.affinity().xpos()][info.affinity().ypos()][_idle_slot] = info.execution_time();
+	});
+
+	if (count.count == count.limit) {
+		Genode::log("reconstruct trace session, subject_count=", count.count);
+		_reconstruct();
+	}
+
+	if (skip_max_idle)
+		return;
+
 	for (unsigned x = 0; x < _space.width(); x++) {
 		for (unsigned y = 0; y < _space.height(); y++) {
 			Affinity::Location const idle_location(x,y);
-			Subject_id         const &subject_id = _idle_id[x][y];
-
-			if (!subject_id.id || _subject_id_reread)
-				_lookup_missing_idle_id(idle_location);
-
-			if (!subject_id.id) {
-				_idle_times[x][y][_idle_slot] = Execution_time(0, 0);
-				continue;
-			}
-
-			Subject_info const info = _trace->subject_info(subject_id);
-
-			Affinity::Location location = info.affinity();
-
-			if (location.xpos() != int(x) || location.ypos() != int(y)) {
-				Subject_id const subject_id_old = subject_id;
-
-				_lookup_missing_idle_id(idle_location);
-
-				Genode::warning("idle location mismatch ", x, "x", y, " vs ",
-				                location.xpos(), "x", location.ypos(), " subject id=",
-				                subject_id_old.id, " vs ", _idle_id[x][y].id);
-			}
-
-			_idle_times[x][y][_idle_slot] = info.execution_time();
-
-			if (skip_max_idle)
-				continue;
-
 			/* determine max available execution time by monitoring idle */
 			auto const  time = diff_idle_times(idle_location);
 			auto       &max  = _idle_max[x][y];
@@ -63,43 +52,6 @@ void Cpu::Trace::_read_idle_times(bool skip_max_idle)
 	}
 }
 
-void Cpu::Trace::_lookup_missing_idle_id(Affinity::Location const &location)
-{
-	Subject_id found_id { };
-
-	do {
-		auto count = _trace->for_each_subject_info([&](Subject_id const &id,
-		                                               Subject_info const &info)
-		{
-			if (found_id.id)
-				return;
-
-			if (info.affinity().xpos() != location.xpos() ||
-			    info.affinity().ypos() != location.ypos())
-				return;
-
-			if (info.session_label() != "kernel" || info.thread_name() != "idle")
-				return;
-
-			_idle_id[location.xpos()][location.ypos()] = id;
-			found_id = id;
-		});
-
-
-		if (count.count == count.limit) {
-			Genode::log("reconstruct trace session, subject_count=", count.count);
-			_reconstruct();
-			found_id = Subject_id();
-			continue;
-		}
-
-		if (!found_id.id) {
-			Genode::error("idle trace id missing");
-			break;
-		}
-	} while (!found_id.id);
-}
-
 Genode::Trace::Subject_id
 Cpu::Trace::lookup_missing_id(Session_label const &label,
                               Thread_name const &thread)

repos/os/src/server/cpu_balancer/trace.h

@@ -6,7 +6,7 @@
  */
 
 /*
- * Copyright (C) 2020 Genode Labs GmbH
+ * Copyright (C) 2020-2021 Genode Labs GmbH
  *
  * This file is part of the Genode OS framework, which is distributed
  * under the terms of the GNU Affero General Public License version 3.
@@ -45,15 +45,12 @@ class Cpu::Trace
 
 		enum { MAX_CORES = 64, MAX_THREADS = 2, HISTORY = 4 };
 
-		Subject_id      _idle_id   [MAX_CORES][MAX_THREADS];
 		Execution_time  _idle_times[MAX_CORES][MAX_THREADS][HISTORY];
 		Execution_time  _idle_max  [MAX_CORES][MAX_THREADS];
 		unsigned        _idle_slot { HISTORY - 1 };
 
 		unsigned        _subject_id_reread { 0 };
 
-		void _lookup_missing_idle_id(Affinity::Location const &);
-
 		void _reconstruct(Genode::size_t const upgrade = 4 * 4096)
 		{
 			_ram_quota += upgrade;
@@ -124,14 +121,21 @@ class Cpu::Trace
 		Session_label lookup_my_label();
 
 		template <typename FUNC>
-		void retrieve(Subject_id const id, FUNC const &fn)
+		void retrieve(Subject_id const target_id, FUNC const &fn)
 		{
 			if (!_trace.constructed())
 				return;
 
-			/* Ieegs, XXX, avoid copying whole object */
+			auto count = _trace->for_each_subject_info([&](Subject_id const &id,
-			Subject_info info = _trace->subject_info(id);
+			                                               Subject_info const &info) {
-			fn(info.execution_time(), info.affinity());
+				if (id == target_id)
+					fn(info.execution_time(), info.affinity());
+			});
+
+			if (count.count == count.limit) {
+				Genode::log("reconstruct trace session, subject_count=", count.count);
+				_reconstruct();
+			}
 		}
 
 		Execution_time abs_idle_times(Affinity::Location const &location)

repos/os/src/test/trace/main.cc

@@ -221,16 +221,6 @@ struct Test_tracing
 		return "undefined";
 	}
 
-	template <typename FUNC>
-	void for_each_subject(Trace::Subject_id subjects[],
-	                      size_t max_subjects, FUNC const &func)
-	{
-		for (size_t i = 0; i < max_subjects; i++) {
-			Trace::Subject_info info = trace.subject_info(subjects[i]);
-			func(subjects[i].id, info);
-		}
-	}
-
 	struct Failed : Genode::Exception { };
 
 	Test_tracing(Env &env) : env(env)
@@ -273,14 +263,6 @@ struct Test_tracing
 		/* wait some time before querying the subjects */
 		timer.msleep(1500);
 
-		Trace::Subject_id subjects[MAX_SUBJECTS];
-		size_t num_subjects = trace.subjects(subjects, MAX_SUBJECTS);
-
-		log(num_subjects, " tracing subjects present");
-
-		if (num_subjects == MAX_SUBJECTS)
-			error("Seems we reached the maximum number of subjects.");
-
 		auto print_info = [this] (Trace::Subject_id id, Trace::Subject_info info) {
 
 			log("ID:",      id.id,                    " "
@@ -294,7 +276,7 @@ struct Test_tracing
 			    "quantum:", info.execution_time().quantum);
 		};
 
-		for_each_subject(subjects, num_subjects, print_info);
+		trace.for_each_subject_info(print_info);
 
 		auto check_untraced = [this] (Trace::Subject_id id, Trace::Subject_info info) {
 
@@ -302,7 +284,7 @@ struct Test_tracing
 				error("Subject ", id.id, " is not UNTRACED");
 		};
 
-		for_each_subject(subjects, num_subjects, check_untraced);
+		trace.for_each_subject_info(check_untraced);
 
 		/* enable tracing for test-thread */
 		auto enable_tracing = [this, &env] (Trace::Subject_id id,
@@ -329,12 +311,12 @@ struct Test_tracing
 			}
 		};
 
-		for_each_subject(subjects, num_subjects, enable_tracing);
+		trace.for_each_subject_info(enable_tracing);
 
 		/* give the test thread some time to run */
 		timer.msleep(1000);
 
-		for_each_subject(subjects, num_subjects, print_info);
+		trace.for_each_subject_info(print_info);
 
 		/* read events from trace buffer */
 		if (test_monitor.constructed()) {