From 1fdbf8d0f87d618deded242e9c22f1cebd9c7c74 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 17 Feb 2023 15:34:49 -0800 Subject: [PATCH 1/2] init: Combine two if-statements Combine two if-statements. This change is fine because: * The code between the two if-statements does not queue actions. * If an action is queued from another thread then WakeMainInitThread() is called after the action has been queued. Bug: 266255006 Change-Id: Id4b9565ff4fdb3ee2a2bbca316c8c78e0f2d38dd Signed-off-by: Bart Van Assche --- init/init.cpp | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/init/init.cpp b/init/init.cpp index c965fe635..05a8a9cc5 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -1109,8 +1109,11 @@ int SecondStageMain(int argc, char** argv) { // Restore prio before main loop setpriority(PRIO_PROCESS, 0, 0); while (true) { - // By default, sleep until something happens. - std::optional epoll_timeout; + // By default, sleep until something happens. Do not convert far_future into + // std::chrono::milliseconds because that would trigger an overflow. The unit of boot_clock + // is 1ns. + const boot_clock::time_point far_future = boot_clock::time_point::max(); + boot_clock::time_point next_action_time = far_future; auto shutdown_command = shutdown_state.CheckShutdown(); if (shutdown_command) { @@ -1122,23 +1125,28 @@ int SecondStageMain(int argc, char** argv) { if (!(prop_waiter_state.MightBeWaiting() || Service::is_exec_service_running())) { am.ExecuteOneCommand(); + // If there's more work to do, wake up again immediately. + if (am.HasMoreCommands()) { + next_action_time = boot_clock::now(); + } } + // Since the above code examined pending actions, no new actions must be + // queued by the code between this line and the Epoll::Wait() call below + // without calling WakeMainInitThread(). if (!IsShuttingDown()) { auto next_process_action_time = HandleProcessActions(); // If there's a process that needs restarting, wake up in time for that. if (next_process_action_time) { - epoll_timeout = std::chrono::ceil( - *next_process_action_time - boot_clock::now()); - if (epoll_timeout < 0ms) epoll_timeout = 0ms; + next_action_time = std::min(next_action_time, *next_process_action_time); } } - if (!(prop_waiter_state.MightBeWaiting() || Service::is_exec_service_running())) { - // If there's more work to do, wake up again immediately. - if (am.HasMoreCommands()) epoll_timeout = 0ms; + std::optional epoll_timeout; + if (next_action_time != far_future) { + epoll_timeout = std::chrono::ceil( + std::max(next_action_time - boot_clock::now(), 0ns)); } - auto epoll_result = epoll.Wait(epoll_timeout); if (!epoll_result.ok()) { LOG(ERROR) << epoll_result.error(); From b4b1b75a35516bb27b854343d040aa9dcd2b436c Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 3 Mar 2023 13:21:19 -0800 Subject: [PATCH 2/2] init: Remove the DebugRebootLogging() function The DebugRebootLogging() function was introduced to help with root-causing b/150863651. Remove this function since this logging functionality is no longer needed. Also remove the functions and methods that are only used by DebugRebootLogging(). Change-Id: Ia150604c6cd70f42b13d655ba43b95445a55b6e2 Signed-off-by: Bart Van Assche --- init/init.cpp | 26 -------------------------- init/init.h | 2 -- init/property_service.cpp | 3 --- 3 files changed, 31 deletions(-) diff --git a/init/init.cpp b/init/init.cpp index 05a8a9cc5..a01ae8765 100644 --- a/init/init.cpp +++ b/init/init.cpp @@ -255,7 +255,6 @@ static class ShutdownState { return {}; } - bool do_shutdown() const { return do_shutdown_; } void set_do_shutdown(bool value) { do_shutdown_ = value; } private: @@ -264,31 +263,6 @@ static class ShutdownState { bool do_shutdown_ = false; } shutdown_state; -static void UnwindMainThreadStack() { - unwindstack::AndroidLocalUnwinder unwinder; - unwindstack::AndroidUnwinderData data; - if (!unwinder.Unwind(data)) { - LOG(ERROR) << __FUNCTION__ - << "sys.powerctl: Failed to unwind callstack: " << data.GetErrorString(); - } - for (const auto& frame : data.frames) { - LOG(ERROR) << "sys.powerctl: " << unwinder.FormatFrame(frame); - } -} - -void DebugRebootLogging() { - LOG(INFO) << "sys.powerctl: do_shutdown: " << shutdown_state.do_shutdown() - << " IsShuttingDown: " << IsShuttingDown(); - if (shutdown_state.do_shutdown()) { - LOG(ERROR) << "sys.powerctl set while a previous shutdown command has not been handled"; - UnwindMainThreadStack(); - } - if (IsShuttingDown()) { - LOG(ERROR) << "sys.powerctl set while init is already shutting down"; - UnwindMainThreadStack(); - } -} - void DumpState() { ServiceList::GetInstance().DumpState(); ActionManager::GetInstance().DumpState(); diff --git a/init/init.h b/init/init.h index 063632a66..9c7e91879 100644 --- a/init/init.h +++ b/init/init.h @@ -42,8 +42,6 @@ void SendLoadPersistentPropertiesMessage(); void PropertyChanged(const std::string& name, const std::string& value); bool QueueControlMessage(const std::string& message, const std::string& name, pid_t pid, int fd); -void DebugRebootLogging(); - int SecondStageMain(int argc, char** argv); int StopServicesFromApex(const std::string& apex_name); diff --git a/init/property_service.cpp b/init/property_service.cpp index 87ffdb917..8da69822c 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -550,9 +550,6 @@ std::optional HandlePropertySet(const std::string& name, const std::st } LOG(INFO) << "Received sys.powerctl='" << value << "' from pid: " << cr.pid << process_log_string; - if (!value.empty()) { - DebugRebootLogging(); - } if (value == "reboot,userspace" && !is_userspace_reboot_supported().value_or(false)) { *error = "Userspace reboot is not supported by this device"; return {PROP_ERROR_INVALID_VALUE};