From 847b80a1124a084a309a7c3dee7aba023b899eff Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 25 Feb 2022 23:28:59 +0000 Subject: [PATCH 1/3] Introduce the Service::CheckConsole() method The Service::Start() method is so long that its length negatively affects readability of the code. Hence this patch that splits Service::Start(). Test: Booted Android in Cuttlefish. Change-Id: Ib8e1e87fbd335520cbe3aac2a88d250fcf3b4ff0 Signed-off-by: Bart Van Assche --- init/service.cpp | 36 ++++++++++++++++++++++-------------- init/service.h | 1 + 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/init/service.cpp b/init/service.cpp index f6dd9b9fa..c053761e9 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -405,6 +405,26 @@ static void ClosePipe(const std::array* pipe) { } } +Result Service::CheckConsole() { + if (!(flags_ & SVC_CONSOLE)) { + return {}; + } + + if (proc_attr_.console.empty()) { + proc_attr_.console = "/dev/" + GetProperty("ro.boot.console", "console"); + } + + // Make sure that open call succeeds to ensure a console driver is + // properly registered for the device node + int console_fd = open(proc_attr_.console.c_str(), O_RDWR | O_CLOEXEC); + if (console_fd < 0) { + flags_ |= SVC_DISABLED; + return ErrnoError() << "Couldn't open console '" << proc_attr_.console << "'"; + } + close(console_fd); + return {}; +} + Result Service::Start() { auto reboot_on_failure = make_scope_guard([this] { if (on_failure_reboot_target_) { @@ -442,20 +462,8 @@ Result Service::Start() { return ErrnoError() << "pipe()"; } - bool needs_console = (flags_ & SVC_CONSOLE); - if (needs_console) { - if (proc_attr_.console.empty()) { - proc_attr_.console = "/dev/" + GetProperty("ro.boot.console", "console"); - } - - // Make sure that open call succeeds to ensure a console driver is - // properly registered for the device node - int console_fd = open(proc_attr_.console.c_str(), O_RDWR | O_CLOEXEC); - if (console_fd < 0) { - flags_ |= SVC_DISABLED; - return ErrnoError() << "Couldn't open console '" << proc_attr_.console << "'"; - } - close(console_fd); + if (Result result = CheckConsole(); !result.ok()) { + return result; } struct stat sb; diff --git a/init/service.h b/init/service.h index 3289f5407..7339370ad 100644 --- a/init/service.h +++ b/init/service.h @@ -145,6 +145,7 @@ class Service { void KillProcessGroup(int signal, bool report_oneshot = false); void SetProcessAttributesAndCaps(); void ResetFlagsForStart(); + Result CheckConsole(); static unsigned long next_start_order_; static bool is_exec_service_running_; From f2222aab6a00941d621b18d01e4c6c3ee2cfc60d Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 25 Feb 2022 22:44:40 +0000 Subject: [PATCH 2/3] Introduce the ConfigureMemcg() method The Service::Start() method is so long that its length negatively affects readability of the code. Hence this patch that splits Service::Start(). Test: Booted Android in Cuttlefish. Change-Id: I972f4e60844bb0d133b1cca1fd4e06bb89fc5f37 Signed-off-by: Bart Van Assche --- init/service.cpp | 81 +++++++++++++++++++++++++----------------------- init/service.h | 1 + 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/init/service.cpp b/init/service.cpp index c053761e9..a777b1b2f 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -425,6 +425,48 @@ Result Service::CheckConsole() { return {}; } +// Configures the memory cgroup properties for the service. +void Service::ConfigureMemcg() { + if (swappiness_ != -1) { + if (!setProcessGroupSwappiness(proc_attr_.uid, pid_, swappiness_)) { + PLOG(ERROR) << "setProcessGroupSwappiness failed"; + } + } + + if (soft_limit_in_bytes_ != -1) { + if (!setProcessGroupSoftLimit(proc_attr_.uid, pid_, soft_limit_in_bytes_)) { + PLOG(ERROR) << "setProcessGroupSoftLimit failed"; + } + } + + size_t computed_limit_in_bytes = limit_in_bytes_; + if (limit_percent_ != -1) { + long page_size = sysconf(_SC_PAGESIZE); + long num_pages = sysconf(_SC_PHYS_PAGES); + if (page_size > 0 && num_pages > 0) { + size_t max_mem = SIZE_MAX; + if (size_t(num_pages) < SIZE_MAX / size_t(page_size)) { + max_mem = size_t(num_pages) * size_t(page_size); + } + computed_limit_in_bytes = + std::min(computed_limit_in_bytes, max_mem / 100 * limit_percent_); + } + } + + if (!limit_property_.empty()) { + // This ends up overwriting computed_limit_in_bytes but only if the + // property is defined. + computed_limit_in_bytes = + android::base::GetUintProperty(limit_property_, computed_limit_in_bytes, SIZE_MAX); + } + + if (computed_limit_in_bytes != size_t(-1)) { + if (!setProcessGroupLimit(proc_attr_.uid, pid_, computed_limit_in_bytes)) { + PLOG(ERROR) << "setProcessGroupLimit failed"; + } + } +} + Result Service::Start() { auto reboot_on_failure = make_scope_guard([this] { if (on_failure_reboot_target_) { @@ -603,44 +645,7 @@ Result Service::Start() { PLOG(ERROR) << "createProcessGroup(" << proc_attr_.uid << ", " << pid_ << ") failed for service '" << name_ << "'"; } else if (use_memcg) { - if (swappiness_ != -1) { - if (!setProcessGroupSwappiness(proc_attr_.uid, pid_, swappiness_)) { - PLOG(ERROR) << "setProcessGroupSwappiness failed"; - } - } - - if (soft_limit_in_bytes_ != -1) { - if (!setProcessGroupSoftLimit(proc_attr_.uid, pid_, soft_limit_in_bytes_)) { - PLOG(ERROR) << "setProcessGroupSoftLimit failed"; - } - } - - size_t computed_limit_in_bytes = limit_in_bytes_; - if (limit_percent_ != -1) { - long page_size = sysconf(_SC_PAGESIZE); - long num_pages = sysconf(_SC_PHYS_PAGES); - if (page_size > 0 && num_pages > 0) { - size_t max_mem = SIZE_MAX; - if (size_t(num_pages) < SIZE_MAX / size_t(page_size)) { - max_mem = size_t(num_pages) * size_t(page_size); - } - computed_limit_in_bytes = - std::min(computed_limit_in_bytes, max_mem / 100 * limit_percent_); - } - } - - if (!limit_property_.empty()) { - // This ends up overwriting computed_limit_in_bytes but only if the - // property is defined. - computed_limit_in_bytes = android::base::GetUintProperty( - limit_property_, computed_limit_in_bytes, SIZE_MAX); - } - - if (computed_limit_in_bytes != size_t(-1)) { - if (!setProcessGroupLimit(proc_attr_.uid, pid_, computed_limit_in_bytes)) { - PLOG(ERROR) << "setProcessGroupLimit failed"; - } - } + ConfigureMemcg(); } if (oom_score_adjust_ != DEFAULT_OOM_SCORE_ADJUST) { diff --git a/init/service.h b/init/service.h index 7339370ad..54c923bba 100644 --- a/init/service.h +++ b/init/service.h @@ -146,6 +146,7 @@ class Service { void SetProcessAttributesAndCaps(); void ResetFlagsForStart(); Result CheckConsole(); + void ConfigureMemcg(); static unsigned long next_start_order_; static bool is_exec_service_running_; From bd73665e6831d185027738d51a9204dd2b8cb263 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 25 Feb 2022 22:52:52 +0000 Subject: [PATCH 3/3] Introduce the RunService() method The Service::Start() method is so long that its length negatively affects readability of the code. Hence this patch that splits Service::Start(). Test: Booted Android in Cuttlefish. Change-Id: I5a6f587ecc5e6470137de6cceda7e685bce28ced Signed-off-by: Bart Van Assche --- init/service.cpp | 81 +++++++++++++++++++++++++----------------------- init/service.h | 4 +++ 2 files changed, 46 insertions(+), 39 deletions(-) diff --git a/init/service.cpp b/init/service.cpp index a777b1b2f..8a9cc0a10 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -467,6 +467,47 @@ void Service::ConfigureMemcg() { } } +// Enters namespaces, sets environment variables, writes PID files and runs the service executable. +void Service::RunService(const std::optional& override_mount_namespace, + const std::vector& descriptors, + std::unique_ptr, decltype(&ClosePipe)> pipefd) { + if (auto result = EnterNamespaces(namespaces_, name_, override_mount_namespace); !result.ok()) { + LOG(FATAL) << "Service '" << name_ << "' failed to set up namespaces: " << result.error(); + } + + for (const auto& [key, value] : environment_vars_) { + setenv(key.c_str(), value.c_str(), 1); + } + + for (const auto& descriptor : descriptors) { + descriptor.Publish(); + } + + if (auto result = WritePidToFiles(&writepid_files_); !result.ok()) { + LOG(ERROR) << "failed to write pid to files: " << result.error(); + } + + // Wait until the cgroups have been created and until the cgroup controllers have been + // activated. + if (std::byte byte; read((*pipefd)[0], &byte, 1) < 0) { + PLOG(ERROR) << "failed to read from notification channel"; + } + pipefd.reset(); + + if (task_profiles_.size() > 0 && !SetTaskProfiles(getpid(), task_profiles_)) { + LOG(ERROR) << "failed to set task profiles"; + } + + // As requested, set our gid, supplemental gids, uid, context, and + // priority. Aborts on failure. + SetProcessAttributesAndCaps(); + + if (!ExpandArgsAndExecv(args_, sigstop_)) { + PLOG(ERROR) << "cannot execv('" << args_[0] + << "'). See the 'Debugging init' section of init's README.md for tips"; + } +} + Result Service::Start() { auto reboot_on_failure = make_scope_guard([this] { if (on_failure_reboot_target_) { @@ -577,45 +618,7 @@ Result Service::Start() { if (pid == 0) { umask(077); - - if (auto result = EnterNamespaces(namespaces_, name_, override_mount_namespace); - !result.ok()) { - LOG(FATAL) << "Service '" << name_ - << "' failed to set up namespaces: " << result.error(); - } - - for (const auto& [key, value] : environment_vars_) { - setenv(key.c_str(), value.c_str(), 1); - } - - for (const auto& descriptor : descriptors) { - descriptor.Publish(); - } - - if (auto result = WritePidToFiles(&writepid_files_); !result.ok()) { - LOG(ERROR) << "failed to write pid to files: " << result.error(); - } - - // Wait until the cgroups have been created and until the cgroup controllers have been - // activated. - if (std::byte byte; read((*pipefd)[0], &byte, 1) < 0) { - PLOG(ERROR) << "failed to read from notification channel"; - } - pipefd.reset(); - - if (task_profiles_.size() > 0 && !SetTaskProfiles(getpid(), task_profiles_)) { - LOG(ERROR) << "failed to set task profiles"; - } - - // As requested, set our gid, supplemental gids, uid, context, and - // priority. Aborts on failure. - SetProcessAttributesAndCaps(); - - if (!ExpandArgsAndExecv(args_, sigstop_)) { - PLOG(ERROR) << "cannot execv('" << args_[0] - << "'). See the 'Debugging init' section of init's README.md for tips"; - } - + RunService(override_mount_namespace, descriptors, std::move(pipefd)); _exit(127); } diff --git a/init/service.h b/init/service.h index 54c923bba..3f12aa22f 100644 --- a/init/service.h +++ b/init/service.h @@ -147,6 +147,10 @@ class Service { void ResetFlagsForStart(); Result CheckConsole(); void ConfigureMemcg(); + void RunService( + const std::optional& override_mount_namespace, + const std::vector& descriptors, + std::unique_ptr, void (*)(const std::array* pipe)> pipefd); static unsigned long next_start_order_; static bool is_exec_service_running_;