From e0d8c281e6f29cd533976252ec644d7ab4e2ecec Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Mon, 16 Sep 2019 11:07:47 +0200 Subject: [PATCH] Swap throttling parentage Now the parent throttles the child to prevent detachment from the terminal. Change-Id: I7241bffbd673f69097acbb19331a44aeb48bc43b Signed-off-by: Michal Bloch --- tests/test-stability-cpu-utils.hpp | 5 +-- tests/test-stability-cpu.cpp | 33 +++++++++++++------ tests/test-stability.cpp | 51 ++++++++++++++++++------------ tests/test-stability.hpp | 2 +- 4 files changed, 58 insertions(+), 33 deletions(-) diff --git a/tests/test-stability-cpu-utils.hpp b/tests/test-stability-cpu-utils.hpp index c508276..8b40fb2 100644 --- a/tests/test-stability-cpu-utils.hpp +++ b/tests/test-stability-cpu-utils.hpp @@ -9,9 +9,10 @@ bool run_cpu_test (thread_func_t * func, size_t processors, size_t test_time, db struct cpu_handler : public dbus_signal_handler { const std::string limitType; + pid_t child_pid; static constexpr const char * const gtype = "(isdda{sv})"; - cpu_handler (const char * lt) : limitType (lt) { } + cpu_handler (const char * lt) : limitType (lt), child_pid (-1) { } bool match (const gchar * objpath, const gchar * iface, GVariant * parameters) const override { if ("/Org/Tizen/StabilityMonitor/tsm_cpu"s != objpath @@ -22,6 +23,6 @@ struct cpu_handler : public dbus_signal_handler { const char * paramType = nullptr; int pid = -1; g_variant_get (parameters, gtype, & pid, & paramType, nullptr, nullptr, nullptr); - return pid == getpid() && limitType == paramType; + return (pid == child_pid || pid == getpid()) && limitType == paramType; } }; diff --git a/tests/test-stability-cpu.cpp b/tests/test-stability-cpu.cpp index d0563ee..15095a9 100644 --- a/tests/test-stability-cpu.cpp +++ b/tests/test-stability-cpu.cpp @@ -58,16 +58,31 @@ int main (int argc, char ** argv) return ! run_cpu_test (cpu_heavy, procs, TEST_TIME_PEAK, & handler_peak); } , "heavy all CPU load, expecting peak signal; then toning down to light load, expecting no signal" } , { []() { - /* Run for a longer time (for avg) at half speed (so as - * not to trigger peak detection). */ - return run_throttled (60 - , [] () { - if (! run_cpu_test (cpu_heavy, 1, TEST_TIME_PEAK, & handler_peak)) - return false; - return ! run_cpu_test (cpu_heavy, 1, TEST_TIME_AVG - TEST_TIME_PEAK, & handler_avg); + /* Have a child run what would normally be 100% CPU usage, + * but with the parent throttling it to ~60% using SIGSTOP/SIGCONT. + * This both allows for fairly precise control over CPU usage + * and tests how stability-monitor handles child processes. */ + + pid_t child = fork (); + if (!child) { + global_stop = false; + cpu_heavy (nullptr); + return false; // unreachable; killed by parent beforehand + } else { + handler_peak.child_pid = child; + handler_avg .child_pid = child; + + if (! run_throttled (child, 60, & handler_peak, TEST_TIME_PEAK)) { + kill (child, SIGKILL); + return false; } - ); - } , "running at ~60% CPU (using SIGSTOP/SIGCONT from child), expecting average signal (but not peak)" + + std::this_thread::sleep_for (CATCH_STRAGGLER_SIGNALS_TIME); + bool ret = ! run_throttled (child, 60, & handler_avg , TEST_TIME_AVG - TEST_TIME_PEAK); + kill (child, SIGKILL); + return ret; + } + } , "running at ~60% CPU (using SIGSTOP/SIGCONT on child), expecting average signal (but not peak)" } }; diff --git a/tests/test-stability.cpp b/tests/test-stability.cpp index b8abbfe..bb3365a 100644 --- a/tests/test-stability.cpp +++ b/tests/test-stability.cpp @@ -185,34 +185,43 @@ bool run_test (thread_func_t * func, size_t processors, const dbus_signal_handle return timed_out; } -// FIXME: throttling doesn't end so only works correctly on the last test in a batch -void throttle (size_t percent, pid_t pid) +static int throttlee; // globals as a workaround for early misdesign +static size_t throttle_percent; + +static void * throttle (void * arg) { + /* Throttling with SIGSTOP + SIGCONT allows to set + * desired CPU usage fairly accurately. Usually the + * parent should throttle the child since otherwise + * SIGSTOP detaches the process from terminals and + * messes things up. */ + int current_sig = SIGSTOP; - while (true) { - std::this_thread::sleep_for (std::chrono::microseconds (100 * percent)); - if (kill (pid, current_sig) == -1) - break; - percent = 100 - percent; + + while (!global_stop) { + std::this_thread::sleep_for (std::chrono::microseconds (100 * throttle_percent)); + + /* NB: failure here would be very worrisome since dead children + * aren't fully terminated but become zombies (so shouldn't + * generate ESRCH). */ + if (kill (throttlee, current_sig) == -1) + throw_errno ("kill(" + std::to_string (throttlee) + ", " + (current_sig == SIGSTOP ? "SIGSTOP" : "SIGCONT") + ") while throttling failed"); + throttle_percent = 100 - throttle_percent; current_sig = (SIGSTOP + SIGCONT) - current_sig; } + + /* Finish with a SIGSTOP so that heavy throttlees do not + * keep running at full speed and trigger something. */ + kill (throttlee, SIGSTOP); + + return nullptr; } -bool run_throttled (size_t percent, bool (*func) ()) +bool run_throttled (pid_t child_pid, size_t percent, dbus_signal_handler * handler, size_t test_time) { - /* The child throttles the parent, not the other way around. - * This is because the parent is the one to return a relevant - * value to the caller. */ - pid_t parent_pid = getpid (); - int child_pid = fork (); - if (child_pid == -1) - throw_errno ("fork failed"); - - if (!child_pid) { - throttle (percent, parent_pid); - exit (EXIT_SUCCESS); - } else - return func (); + throttlee = child_pid; + throttle_percent = percent; + return run_test (throttle, 1, handler, test_time); } static bool run_single_testcase (size_t index, const std::pair & test_case) diff --git a/tests/test-stability.hpp b/tests/test-stability.hpp index 8546de3..14f4fcd 100644 --- a/tests/test-stability.hpp +++ b/tests/test-stability.hpp @@ -43,7 +43,7 @@ extern GMainLoop * loop; typedef void * thread_func_t (void *); -bool run_throttled (size_t percent, bool (*func) ()); +bool run_throttled (pid_t child_pid, size_t percent, dbus_signal_handler * handler, size_t test_time); bool run_test (thread_func_t * func, size_t processors, const dbus_signal_handler * handler, size_t test_time); int standard_main (const std::vector > & test_cases, int argc, char ** argv); -- 2.34.1