From be0a1742ac3fb2c84239b1f969e569d63112427c Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Fri, 11 Mar 2022 14:06:16 +0100 Subject: [PATCH] base: distinct TRACED from ATTACHED trace subjects This patch makes the trace-subject state as reflected to the trace monitor more accurate. Until now, a subject could be in UNTRACED or TRACED state. In reality, however, there exists an intermediate state after the trace monitor called 'trace' for the subject but before the subject locally activated the tracing (done when passing a trace point). This intermediate state was reflected as UNTRACED. Consequently, threads that never pass a trace point (e.g., just waiting for I/O) would remain to appear as UNTRACED even after enabling its tracing by the trace monitor. This is confusing. This patch replaces the former UNTRACED and TRACED states by three distinct states: UNATTACHED prior any call of 'trace' ATTACHED after a trace monitor called 'trace' but before the tracing is active TRACE tracing is active Fixes #4447 --- repos/base/include/base/trace/types.h | 15 ++++++----- .../src/core/include/trace/subject_registry.h | 25 ++++++++++------- repos/os/src/test/trace/main.cc | 27 +++++-------------- 3 files changed, 30 insertions(+), 37 deletions(-) diff --git a/repos/base/include/base/trace/types.h b/repos/base/include/base/trace/types.h index 7a414924a4..60f8673337 100644 --- a/repos/base/include/base/trace/types.h +++ b/repos/base/include/base/trace/types.h @@ -107,17 +107,18 @@ class Genode::Trace::Subject_info { public: - enum State { INVALID, UNTRACED, TRACED, FOREIGN, ERROR, DEAD }; + enum State { INVALID, UNATTACHED, ATTACHED, TRACED, FOREIGN, ERROR, DEAD }; static char const *state_name(State state) { switch (state) { - case INVALID: return "INVALID"; - case UNTRACED: return "UNTRACED"; - case TRACED: return "TRACED"; - case FOREIGN: return "FOREIGN"; - case ERROR: return "ERROR"; - case DEAD: return "DEAD"; + case INVALID: return "INVALID"; + case UNATTACHED: return "UNATTACHED"; + case ATTACHED: return "ATTACHED"; + case TRACED: return "TRACED"; + case FOREIGN: return "FOREIGN"; + case ERROR: return "ERROR"; + case DEAD: return "DEAD"; } return "INVALID"; } diff --git a/repos/base/src/core/include/trace/subject_registry.h b/repos/base/src/core/include/trace/subject_registry.h index 267756845c..374aa5d05d 100644 --- a/repos/base/src/core/include/trace/subject_registry.h +++ b/repos/base/src/core/include/trace/subject_registry.h @@ -156,24 +156,29 @@ class Genode::Trace::Subject if (!source.valid()) return Subject_info::DEAD; - if (source->enabled()) - return source->owned_by(*this) ? Subject_info::TRACED - : Subject_info::FOREIGN; if (source->error()) return Subject_info::ERROR; - return Subject_info::UNTRACED; + if (source->enabled() && !source->owned_by(*this)) + return Subject_info::FOREIGN; + + if (source->owned_by(*this)) + return source->enabled() ? Subject_info::TRACED + : Subject_info::ATTACHED; + + return Subject_info::UNATTACHED; } void _traceable_or_throw() { switch(_state()) { - case Subject_info::DEAD : throw Source_is_dead(); - case Subject_info::FOREIGN : throw Traced_by_other_session(); - case Subject_info::ERROR : throw Source_is_dead(); - case Subject_info::INVALID : throw Nonexistent_subject(); - case Subject_info::UNTRACED: return; - case Subject_info::TRACED : return; + case Subject_info::DEAD : throw Source_is_dead(); + case Subject_info::FOREIGN : throw Traced_by_other_session(); + case Subject_info::ERROR : throw Source_is_dead(); + case Subject_info::INVALID : throw Nonexistent_subject(); + case Subject_info::UNATTACHED : return; + case Subject_info::ATTACHED : return; + case Subject_info::TRACED : return; } } diff --git a/repos/os/src/test/trace/main.cc b/repos/os/src/test/trace/main.cc index 9ea793f8f4..a098f61150 100644 --- a/repos/os/src/test/trace/main.cc +++ b/repos/os/src/test/trace/main.cc @@ -202,19 +202,6 @@ struct Test_tracing Rom_dataspace_capability policy_module_rom_ds { }; - char const *state_name(Trace::Subject_info::State state) - { - switch (state) { - case Trace::Subject_info::INVALID: return "INVALID"; - case Trace::Subject_info::UNTRACED: return "UNTRACED"; - case Trace::Subject_info::TRACED: return "TRACED"; - case Trace::Subject_info::FOREIGN: return "FOREIGN"; - case Trace::Subject_info::ERROR: return "ERROR"; - case Trace::Subject_info::DEAD: return "DEAD"; - } - return "undefined"; - } - struct Failed : Genode::Exception { }; Test_tracing(Env &env) : env(env) @@ -262,7 +249,7 @@ struct Test_tracing log("ID:", id.id, " " "label:\"", info.session_label(), "\" " "name:\"", info.thread_name(), "\" " - "state:", state_name(info.state()), " " + "state:", Trace::Subject_info::state_name(info.state()), " " "policy:", info.policy_id().id, " " "thread context time:", info.execution_time().thread_context, " " "scheduling context time:", info.execution_time().scheduling_context, " ", @@ -272,20 +259,20 @@ struct Test_tracing trace.for_each_subject_info(print_info); - auto check_untraced = [this] (Trace::Subject_id id, Trace::Subject_info info) { + auto check_unattached = [this] (Trace::Subject_id id, Trace::Subject_info info) { - if (info.state() != Trace::Subject_info::UNTRACED) - error("Subject ", id.id, " is not UNTRACED"); + if (info.state() != Trace::Subject_info::UNATTACHED) + error("Subject ", id.id, " is not UNATTACHED"); }; - trace.for_each_subject_info(check_untraced); + trace.for_each_subject_info(check_unattached); /* enable tracing for test-thread */ auto enable_tracing = [this, &env] (Trace::Subject_id id, Trace::Subject_info info) { - if ( info.session_label() != policy_label - || info.thread_name() != policy_thread) { + if (info.session_label() != policy_label + || info.thread_name() != policy_thread) { return; }