From 90bea1499ed4b26c5aa695f34fbbce1f09f81f3d Mon Sep 17 00:00:00 2001 From: Alexander Boettcher Date: Wed, 23 Sep 2020 07:41:39 +0200 Subject: [PATCH] core: store new affinity on successful migration Adjust the base-* platforms to acknowledge new thread location solely if migration is supported and succeeded. Otherwise the wrong thread locations are observed via the trace session and utilization time calculation get wrong. Issue #3842 --- repos/base-foc/src/core/platform_thread.cc | 9 +++------ repos/base-hw/src/core/platform_thread.cc | 7 ++++--- repos/base-hw/src/core/platform_thread.h | 2 +- .../src/core/include/platform_thread.h | 9 +++++---- repos/base-pistachio/src/core/platform_thread.cc | 7 ++++--- .../base-sel4/src/core/spec/arm/platform_thread.cc | 13 ++++++++----- .../base-sel4/src/core/spec/x86/platform_thread.cc | 7 ++++--- 7 files changed, 29 insertions(+), 25 deletions(-) diff --git a/repos/base-foc/src/core/platform_thread.cc b/repos/base-foc/src/core/platform_thread.cc index 388405e418..4aaa62580b 100644 --- a/repos/base-foc/src/core/platform_thread.cc +++ b/repos/base-foc/src/core/platform_thread.cc @@ -221,19 +221,16 @@ Foc_thread_state Platform_thread::state() } -void Platform_thread::affinity(Affinity::Location location) +void Platform_thread::affinity(Affinity::Location const location) { - _location = location; - int const cpu = location.xpos(); l4_sched_param_t params = l4_sched_param(_prio); params.affinity = l4_sched_cpu_set(cpu, 0, 1); l4_msgtag_t tag = l4_scheduler_run_thread(L4_BASE_SCHEDULER_CAP, _thread.local.data()->kcap(), ¶ms); - if (l4_error(tag)) - warning("setting affinity of ", Hex(_thread.local.data()->kcap()), - " to ", cpu, " failed!"); + if (!l4_error(tag)) + _location = location; } diff --git a/repos/base-hw/src/core/platform_thread.cc b/repos/base-hw/src/core/platform_thread.cc index 60512fa4c4..673546a8fd 100644 --- a/repos/base-hw/src/core/platform_thread.cc +++ b/repos/base-hw/src/core/platform_thread.cc @@ -66,6 +66,7 @@ Platform_thread::Platform_thread(Label const &label, Native_utcb &utcb) _utcb_core_addr(&utcb), _utcb_pd_addr(&utcb), _main_thread(false), + _location(Affinity::Location()), _kobj(true, _label.string()) { /* create UTCB for a core thread */ @@ -92,6 +93,7 @@ Platform_thread::Platform_thread(size_t const quota, _priority(_scale_priority(virt_prio)), _quota(quota), _main_thread(false), + _location(location), _kobj(true, _priority, _quota, _label.string()) { try { @@ -101,7 +103,6 @@ Platform_thread::Platform_thread(size_t const quota, throw Out_of_ram(); } _utcb_core_addr = (Native_utcb *)core_env().rm_session()->attach(_utcb); - affinity(location); } @@ -121,9 +122,9 @@ void Platform_thread::join_pd(Platform_pd * pd, bool const main_thread, } -void Platform_thread::affinity(Affinity::Location const & location) +void Platform_thread::affinity(Affinity::Location const &) { - _location = location; + /* yet no migration support, don't claim wrong location, e.g. for tracing */ } diff --git a/repos/base-hw/src/core/platform_thread.h b/repos/base-hw/src/core/platform_thread.h index e5fd4abb22..783b215229 100644 --- a/repos/base-hw/src/core/platform_thread.h +++ b/repos/base-hw/src/core/platform_thread.h @@ -72,7 +72,7 @@ namespace Genode { */ bool _main_thread; - Affinity::Location _location { }; + Affinity::Location _location; Kernel_object _kobj; diff --git a/repos/base-pistachio/src/core/include/platform_thread.h b/repos/base-pistachio/src/core/include/platform_thread.h index 47dd48bd7f..e91631ba83 100644 --- a/repos/base-pistachio/src/core/include/platform_thread.h +++ b/repos/base-pistachio/src/core/include/platform_thread.h @@ -63,7 +63,7 @@ namespace Genode { Platform_pd *_platform_pd = nullptr; unsigned _priority = 0; Pager_object *_pager = nullptr; - Affinity::Location _location { }; + Affinity::Location _location; public: @@ -74,15 +74,16 @@ namespace Genode { * Constructor */ Platform_thread(size_t, char const *name, unsigned priority, - Affinity::Location, addr_t) + Affinity::Location location, addr_t) : - _name(name), _priority(priority) + _name(name), _priority(priority), _location(location) { } /** * Constructor used for core-internal threads */ - Platform_thread(char const *name) : _name(name) { } + Platform_thread(char const *name) + : _name(name), _location(Affinity::Location()) { } /** * Destructor diff --git a/repos/base-pistachio/src/core/platform_thread.cc b/repos/base-pistachio/src/core/platform_thread.cc index 8e73f57a15..b681adba8a 100644 --- a/repos/base-pistachio/src/core/platform_thread.cc +++ b/repos/base-pistachio/src/core/platform_thread.cc @@ -42,8 +42,6 @@ static const bool verbose2 = true; void Platform_thread::affinity(Affinity::Location location) { - _location = location; - unsigned const cpu_no = location.xpos(); if (cpu_no >= L4_NumProcessors(get_kip())) { @@ -51,9 +49,12 @@ void Platform_thread::affinity(Affinity::Location location) return; } - if (_l4_thread_id != L4_nilthread) + if (_l4_thread_id != L4_nilthread) { if (L4_Set_ProcessorNo(_l4_thread_id, cpu_no) == 0) error("could not set processor number"); + else + _location = location; + } } diff --git a/repos/base-sel4/src/core/spec/arm/platform_thread.cc b/repos/base-sel4/src/core/spec/arm/platform_thread.cc index 97e5810d34..2f7500487e 100644 --- a/repos/base-sel4/src/core/spec/arm/platform_thread.cc +++ b/repos/base-sel4/src/core/spec/arm/platform_thread.cc @@ -13,10 +13,13 @@ #include -void Genode::Platform_thread::affinity(Affinity::Location location) +void Genode::Platform_thread::affinity(Affinity::Location const location) { - _location = location; - - Genode::error("could not set affinity"); - //seL4_TCB_SetAffinity(tcb_sel().value(), location.xpos()); +#if CONFIG_MAX_NUM_NODES > 1 + seL4_Error const res = seL4_TCB_SetAffinity(tcb_sel().value(), location.xpos()); + if (res == seL4_NoError) + _location = location; +#else + (void)location; +#endif } diff --git a/repos/base-sel4/src/core/spec/x86/platform_thread.cc b/repos/base-sel4/src/core/spec/x86/platform_thread.cc index 246282ccbf..7da93a0e75 100644 --- a/repos/base-sel4/src/core/spec/x86/platform_thread.cc +++ b/repos/base-sel4/src/core/spec/x86/platform_thread.cc @@ -23,10 +23,11 @@ Phys_allocator& Genode::phys_alloc_16k(Allocator * core_mem_alloc) return phys_alloc_16k; } -void Genode::Platform_thread::affinity(Affinity::Location location) +void Genode::Platform_thread::affinity(Affinity::Location const location) { - _location = location; - seL4_TCB_SetAffinity(tcb_sel().value(), location.xpos()); + seL4_Error const res = seL4_TCB_SetAffinity(tcb_sel().value(), location.xpos()); + if (res == seL4_NoError) + _location = location; } bool Genode::Thread_info::init_vcpu(Platform &platform, Cap_sel ept)