core: filter trace subjects by TRACE session label

This patch changes core's TRACE service to expose trace subjects only if
their PD label matches the label of the TRACE monitor. Hence, by default,
a trace monitor can only observe itself and its child components. Only
if the trace monitor's parent rewrites the trace-session's label, the
view of trace monitor can become broader. For example, when rewriting
the trace label to an empty string "", the trace monitor becomes able to
observe the sibling components hosted in the same init instance as the
trace monitor.

To grant a trace session the special privilege of obtaining a global
system view (including the kernel's trace subjects), the top-level init
has to rewrite the session's label to an empty string. At core, this
specific label "init -> " is handled as a special case that discharges
the filtering/namespacing of trace subjects.

Note that the trace-subject label as reported as subject info is now
given relative to the label of the trace session. As a nice side effect
of this change, the pkg/test-trace_logger works now when executed by the
depot_autopilot as well as via the test.run script.

Issue #847

repos/base/src/core/cpu_session_component.cc

@@ -384,11 +384,11 @@ size_t Cpu_session_component::_weight_to_quota(size_t const weight) const
  ** Trace::Source_registry **
  ****************************/
 
-unsigned Core::Trace::Source::_alloc_unique_id()
+Core::Trace::Source::Id Core::Trace::Source::_alloc_unique_id()
 {
 	static Mutex lock;
 	static unsigned cnt;
 	Mutex::Guard guard(lock);
-	return cnt++;
+	return { cnt++ };
 }
 

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

@@ -38,7 +38,6 @@ class Core::Trace::Root : public Root_component<Session_component>
 		{
 			size_t ram_quota       = Arg_string::find_arg(args, "ram_quota").ulong_value(0);
 			size_t arg_buffer_size = Arg_string::find_arg(args, "arg_buffer_size").ulong_value(0);
-			unsigned parent_levels = (unsigned)Arg_string::find_arg(args, "parent_levels").ulong_value(0);
 
 			if (arg_buffer_size > ram_quota)
 				throw Insufficient_ram_quota();
@@ -49,7 +48,7 @@ class Core::Trace::Root : public Root_component<Session_component>
 			                         session_label_from_args(args),
 			                         session_diag_from_args(args),
 			                         _ram, _local_rm,
-			                         arg_buffer_size, parent_levels,
+			                         arg_buffer_size,
 			                         _sources, _policies);
 		}
 

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

@@ -47,6 +47,17 @@ class Core::Trace::Session_component
 		unsigned                     _policy_cnt { 0 };
 		Attached_ram_dataspace       _argument_buffer;
 
+		/*
+		 * Whenever a trace session is deliberately labeled as empty by the
+		 * top-level init instance, the session is granted global reach.
+		 * Otherwise, the label is taken a prefix filter for the visibility
+		 * of trace subjects within the session.
+		 */
+		Filter _filter() const
+		{
+			return (_label == "init -> ") ? Filter("") : Filter(_label);
+		}
+
 	public:
 
 		/**
@@ -59,7 +70,6 @@ class Core::Trace::Session_component
 		                  Ram_allocator   &ram,
 		                  Region_map      &local_rm,
 		                  size_t           arg_buffer_size,
-		                  unsigned         parent_levels,
 		                  Source_registry &sources,
 		                  Policy_registry &policies);
 

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

@@ -30,6 +30,8 @@ namespace Core { namespace Trace {
 
 	using namespace Genode::Trace;
 
+	using Filter = String<Session_label::capacity()>;
+
 	class Source;
 	class Source_owner;
 	class Source_registry;
@@ -71,16 +73,18 @@ class Core::Trace::Source
 			virtual Info trace_source_info() const = 0;
 		};
 
+		struct Id { unsigned value; };
+
 	private:
 
-		unsigned      const  _unique_id;
+		Id            const  _unique_id;
 		Info_accessor const &_info;
 		Control             &_control;
 		Dataspace_capability _policy { };
 		Dataspace_capability _buffer { };
 		Source_owner  const *_owner_ptr = nullptr;
 
-		static unsigned _alloc_unique_id();
+		static Id _alloc_unique_id();
 
 		/*
 		 * Noncopyable
@@ -144,7 +148,7 @@ class Core::Trace::Source
 
 		Dataspace_capability buffer()    const { return _buffer; }
 		Dataspace_capability policy()    const { return _policy; }
-		unsigned             unique_id() const { return _unique_id; }
+		Id                   id()        const { return _unique_id; }
 };
 
 
@@ -184,16 +188,11 @@ class Core::Trace::Source_registry
 		 ** Interface used by TRACE service **
 		 *************************************/
 
-		template <typename TEST, typename INSERT>
-		void export_sources(TEST &test, INSERT &insert)
+		void for_each_source(auto const &fn)
 		{
 			for (Source *s = _entries.first(); s; s = s->next())
-				if (!test(s->unique_id())) {
-					Source::Info const info = s->info();
-					insert(s->unique_id(), s->weak_ptr(), info.label, info.name);
-				}
+				fn(*s);
 		}
-
 };
 
 #endif /* _CORE__INCLUDE__TRACE__SOURCE_REGISTRY_H_ */

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

@@ -137,7 +137,7 @@ class Core::Trace::Subject
 		friend class Subject_registry;
 
 		Subject_id    const _id;
-		unsigned      const _source_id;
+		Source::Id    const _source_id;
 		Weak_ptr<Source>    _source;
 		Session_label const _label;
 		Thread_name   const _name;
@@ -184,7 +184,7 @@ class Core::Trace::Subject
 		/**
 		 * Constructor, called from 'Subject_registry' only
 		 */
-		Subject(Subject_id id, unsigned source_id, Weak_ptr<Source> &source,
+		Subject(Subject_id id, Source::Id source_id, Weak_ptr<Source> &source,
 		        Session_label const &label, Thread_name const &name)
 		:
 			_id(id), _source_id(source_id), _source(source),
@@ -212,7 +212,7 @@ class Core::Trace::Subject
 		/**
 		 * Test if subject belongs to the specified unique source ID
 		 */
-		bool has_source_id(unsigned id) const { return id == _source_id; }
+		bool has_source_id(Source::Id id) const { return id.value == _source_id.value; }
 
 		/**
 		 * Start tracing
@@ -325,51 +325,11 @@ class Core::Trace::Subject_registry
 
 		Allocator       &_md_alloc;
 		Source_registry &_sources;
+		Filter     const _filter;
 		unsigned         _id_cnt  { 0 };
 		Mutex            _mutex   { };
 		Subjects         _entries { };
 
-		/**
-		 * Functor for testing the existance of subjects for a given source
-		 *
-		 * This functor is invoked by 'Source_registry::export'.
-		 */
-		struct Tester
-		{
-			Subjects &subjects;
-
-			Tester(Subjects &subjects) : subjects(subjects) { }
-
-			bool operator () (unsigned source_id)
-			{
-				for (Subject *s = subjects.first(); s; s = s->next())
-					if (s->has_source_id(source_id))
-						return true;
-				return false;
-			}
-		} _tester { _entries };
-
-		/**
-		 * Functor for inserting new subjects into the registry
-		 *
-		 * This functor is invoked by 'Source_registry::export'.
-		 */
-		struct Inserter
-		{
-			Subject_registry &registry;
-
-			Inserter(Subject_registry &registry) : registry(registry) { }
-
-			void operator () (unsigned source_id, Weak_ptr<Source> source,
-			                  Session_label const &label, Thread_name const &name)
-			{
-				Subject *subject = new (&registry._md_alloc)
-					Subject(Subject_id(++registry._id_cnt), source_id, source, label, name);
-
-				registry._entries.insert(subject);
-			}
-		} _inserter { *this };
-
 		/**
 		 * Destroy subject, and release policy and trace buffers
 		 */
@@ -398,23 +358,11 @@ class Core::Trace::Subject_registry
 
 	public:
 
-		/**
-		 * Constructor
-		 *
-		 * \param md_alloc  meta-data allocator used for allocating 'Subject'
-		 *                  objects.
-		 * \param ram       allocator used for the allocation of trace
-		 *                  buffers and policy dataspaces.
-		 */
-		Subject_registry(Allocator &md_alloc,
-		                 Source_registry &sources)
+		Subject_registry(Allocator &md_alloc, Source_registry &sources, Filter const &filter)
 		:
-			_md_alloc(md_alloc), _sources(sources)
+			_md_alloc(md_alloc), _sources(sources), _filter(filter)
 		{ }
 
-		/**
-		 * Destructor
-		 */
 		~Subject_registry()
 		{
 			Mutex::Guard guard(_mutex);
@@ -430,7 +378,32 @@ class Core::Trace::Subject_registry
 		{
 			Mutex::Guard guard(_mutex);
 
-			_sources.export_sources(_tester, _inserter);
+			auto already_known = [&] (Source::Id const unique_id)
+			{
+				for (Subject *s = _entries.first(); s; s = s->next())
+					if (s->has_source_id(unique_id))
+						return true;
+				return false;
+			};
+
+			auto filter_matches = [&] (Session_label const &label)
+			{
+				return strcmp(_filter.string(), label.string(), _filter.length() - 1) == 0;
+			};
+
+			_sources.for_each_source([&] (Source &source) {
+
+				Source::Info const info = source.info();
+
+				if (!filter_matches(info.label) || already_known(source.id()))
+					return;
+
+				Weak_ptr<Source> source_ptr = source.weak_ptr();
+
+				_entries.insert(new (_md_alloc)
+					Subject(Subject_id(++_id_cnt), source.id(),
+					        source_ptr, info.label, info.name));
+			});
 		}
 
 		/**
@@ -453,12 +426,25 @@ class Core::Trace::Subject_registry
 		{
 			Mutex::Guard guard(_mutex);
 
+			auto filtered = [&] (Session_label const &label) -> Session_label
+			{
+				return (label.length() <= _filter.length() || !_filter.length())
+				        ? Session_label("") /* this cannot happen */
+				        : Session_label(label.string() + _filter.length() - 1);
+			};
+
 			unsigned i = 0;
 			for (Subject *s = _entries.first(); s && i < len; s = s->next()) {
-				ids[i]   = s->id();
-				dst[i++] = s->info();
-			}
+				ids[i] = s->id();
 
+				Subject_info const info = s->info();
+
+				/* strip filter prefix from reported trace-subject label */
+				dst[i++] = {
+					filtered(info.session_label()), info.thread_name(), info.state(),
+					info.policy_id(), info.execution_time(), info.affinity()
+				};
+			}
 			return i;
 		}
 

repos/base/src/core/trace_session_component.cc

@@ -132,7 +132,6 @@ Session_component::Session_component(Rpc_entrypoint  &ep,
                                      Ram_allocator   &ram,
                                      Region_map      &local_rm,
                                      size_t           arg_buffer_size,
-                                     unsigned         /* parent_levels */,
                                      Source_registry &sources,
                                      Policy_registry &policies)
 :
@@ -143,7 +142,7 @@ Session_component::Session_component(Rpc_entrypoint  &ep,
 	_policies_slab(&_md_alloc),
 	_sources(sources),
 	_policies(policies),
-	_subjects(_subjects_slab, _sources),
+	_subjects(_subjects_slab, _sources, _filter()),
 	_argument_buffer(_ram, local_rm, arg_buffer_size)
 { }
 

repos/os/recipes/pkg/test-trace/runtime

@@ -28,6 +28,7 @@
 			<service name="Timer"/>
 		</parent-provides>
 		<default-route>
+			<service name="TRACE"> <parent label=""/> </service>
 			<any-service> <parent/> <any-child/> </any-service>
 		</default-route>
 		<default caps="200"/>
@@ -37,21 +38,21 @@
 				<start name="test-trace">
 					<resource name="RAM" quantum="10M"/>
 					<config>
-						<trace_policy label="init -> dynamic -> test-trace -> sequence -> test-trace" thread="test-thread" module="null"/>
+						<trace_policy label="sequence -> test-trace" thread="test-thread" module="null"/>
 					</config>
 				</start>
 
 				<start name="test-trace">
 					<resource name="RAM" quantum="10M"/>
 					<config>
-						<trace_policy label="init -> dynamic -> test-trace -> top" thread="ep" module="null"/>
+						<trace_policy label="top" thread="ep" module="null"/>
 					</config>
 				</start>
 
 				<start name="test-trace">
 					<resource name="RAM" quantum="10M"/>
 					<config>
-						<trace_policy label="init -> dynamic -> test-trace -> top" thread="ep" module="null"/>
+						<trace_policy label="top" thread="ep" module="null"/>
 					</config>
 				</start>
 			</config>

repos/os/recipes/pkg/test-trace_logger/runtime

@@ -5,11 +5,11 @@
 	<fail after_seconds="20"/>
 	<succeed>
 			[init -> trace_logger] Report *
-			[init -> trace_logger] PD "init -> dynamic -> test-trace_logger -> cpu_burner.*"*
+			[init -> trace_logger] PD "cpu_burner.*"*
 			[init -> trace_logger]  Thread "ep" at (0,0)  TRACED total:* recent:*
-			[init -> trace_logger] PD "init -> dynamic -> test-trace_logger -> dynamic_rom"*
+			[init -> trace_logger] PD "dynamic_rom"*
 			[init -> trace_logger]  Thread "ep" at (0,0)  TRACED total:* recent:*
-			[init -> trace_logger] PD "init -> dynamic -> test-trace_logger -> test-trace_logger"*
+			[init -> trace_logger] PD "test-trace_logger"*
 			[init -> trace_logger]  Thread "ep" at (0,0)  TRACED total:* recent:*
 			[init -> trace_logger]   100 *
 			[init -> trace_logger]   trigger_once
@@ -49,12 +49,16 @@
 			<resource name="RAM" quantum="80M"/>
 			<config verbose="yes" priority="no" sc_time="no" session_ram="10M" session_parent_levels="1" session_arg_buffer="64K" period_sec="3" default_policy="null" default_buffer="1K">
 
-				<policy label_prefix="init -> dynamic -> test-trace_logger -> cpu_burner" thread="ep"/>
+				<policy label_prefix="cpu_burner" thread="ep"/>
 
-				<policy label="init -> dynamic -> test-trace_logger -> test-trace_logger" thread="ep" buffer="4K" policy="rpc_name"/>
+				<policy label="test-trace_logger" thread="ep" buffer="4K" policy="rpc_name"/>
 
-				<policy label="init -> dynamic -> test-trace_logger -> dynamic_rom" thread="ep" buffer="8K" policy="log_output"/>
+				<policy label="dynamic_rom" thread="ep" buffer="8K" policy="log_output"/>
 			</config>
+			<route>
+				<service name="TRACE"> <parent label=""/> </service>
+				<any-service> <parent/> <any-child/> </any-service>
+			</route>
 		</start>
 
 		<start name="dynamic_rom">