diff options
author | Tom Cherry <tomcherry@google.com> | 2019-09-06 10:52:31 -0700 |
---|---|---|
committer | Luca Stefani <luca.stefani.ge1@gmail.com> | 2020-01-04 14:49:13 +0100 |
commit | 6b664ff0596b91553e64aae1f79f338c4fc3d051 (patch) | |
tree | 66e4115df78cb2293df1437314d320a794ebc83c | |
parent | 59da9910b3a6da545a690643bec9252ab800a3d7 (diff) | |
download | system_core-6b664ff0596b91553e64aae1f79f338c4fc3d051.tar.gz system_core-6b664ff0596b91553e64aae1f79f338c4fc3d051.tar.bz2 system_core-6b664ff0596b91553e64aae1f79f338c4fc3d051.zip |
ueventd: make parallel restorecon functionality optional
5aa6197d5f387579ff04c330001840d6988e825f added the ability to
parallelize restorecon to speed up boot for devices that have not
completely moved to genfscon. This parallel restorecon happens after
the parallel ueventd handling.
This causes a performance regression for devices that have moved to
genfscon, since previously, the restorecon() was done in the main
ueventd thread in parallel with the uevent handlers.
I also tried to run the fully parallelized restorecon in parallel with
the uevent handlers, but that did not make any change to the cold boot
time, likely due to the additional overhead of parallelizing the work.
Bug: 140458170
Test: blueline coldboot time returns to pre-regression time.
Change-Id: I3cd6a869cc9b62792466813d94ad6c69834e854e
-rw-r--r-- | init/ueventd.cpp | 33 | ||||
-rw-r--r-- | init/ueventd_parser.cpp | 16 | ||||
-rw-r--r-- | init/ueventd_parser.h | 6 | ||||
-rw-r--r-- | init/ueventd_parser_test.cpp | 29 |
4 files changed, 62 insertions, 22 deletions
diff --git a/init/ueventd.cpp b/init/ueventd.cpp index 2668b6575..aa0d46222 100644 --- a/init/ueventd.cpp +++ b/init/ueventd.cpp @@ -112,10 +112,12 @@ namespace init { class ColdBoot { public: ColdBoot(UeventListener& uevent_listener, - std::vector<std::unique_ptr<UeventHandler>>& uevent_handlers) + std::vector<std::unique_ptr<UeventHandler>>& uevent_handlers, + bool enable_parallel_restorecon) : uevent_listener_(uevent_listener), uevent_handlers_(uevent_handlers), - num_handler_subprocesses_(std::thread::hardware_concurrency() ?: 4) {} + num_handler_subprocesses_(std::thread::hardware_concurrency() ?: 4), + enable_parallel_restorecon_(enable_parallel_restorecon) {} void Run(); @@ -131,6 +133,8 @@ class ColdBoot { std::vector<std::unique_ptr<UeventHandler>>& uevent_handlers_; unsigned int num_handler_subprocesses_; + bool enable_parallel_restorecon_; + std::vector<Uevent> uevent_queue_; std::set<pid_t> subprocess_pids_; @@ -154,7 +158,6 @@ void ColdBoot::RestoreConHandler(unsigned int process_num, unsigned int total_pr selinux_android_restorecon(dir.c_str(), SELINUX_ANDROID_RESTORECON_RECURSE); } - _exit(EXIT_SUCCESS); } void ColdBoot::GenerateRestoreCon(const std::string& directory) { @@ -194,7 +197,10 @@ void ColdBoot::ForkSubProcesses() { if (pid == 0) { UeventHandlerMain(i, num_handler_subprocesses_); - RestoreConHandler(i, num_handler_subprocesses_); + if (enable_parallel_restorecon_) { + RestoreConHandler(i, num_handler_subprocesses_); + } + _exit(EXIT_SUCCESS); } subprocess_pids_.emplace(pid); @@ -239,14 +245,20 @@ void ColdBoot::Run() { RegenerateUevents(); - selinux_android_restorecon("/sys", 0); - selinux_android_restorecon("/sys/devices", 0); - GenerateRestoreCon("/sys"); - // takes long time for /sys/devices, parallelize it - GenerateRestoreCon("/sys/devices"); + if (enable_parallel_restorecon_) { + selinux_android_restorecon("/sys", 0); + selinux_android_restorecon("/sys/devices", 0); + GenerateRestoreCon("/sys"); + // takes long time for /sys/devices, parallelize it + GenerateRestoreCon("/sys/devices"); + } ForkSubProcesses(); + if (!enable_parallel_restorecon_) { + selinux_android_restorecon("/sys", SELINUX_ANDROID_RESTORECON_RECURSE); + } + WaitForSubProcesses(); close(open(COLDBOOT_DONE, O_WRONLY | O_CREAT | O_CLOEXEC, 0000)); @@ -291,7 +303,8 @@ int ueventd_main(int argc, char** argv) { UeventListener uevent_listener(ueventd_configuration.uevent_socket_rcvbuf_size); if (access(COLDBOOT_DONE, F_OK) != 0) { - ColdBoot cold_boot(uevent_listener, uevent_handlers); + ColdBoot cold_boot(uevent_listener, uevent_handlers, + ueventd_configuration.enable_parallel_restorecon); cold_boot.Run(); } diff --git a/init/ueventd_parser.cpp b/init/ueventd_parser.cpp index aac3fe5c1..a099aedcf 100644 --- a/init/ueventd_parser.cpp +++ b/init/ueventd_parser.cpp @@ -88,18 +88,17 @@ Result<Success> ParseFirmwareDirectoriesLine(std::vector<std::string>&& args, return Success(); } -Result<Success> ParseModaliasHandlingLine(std::vector<std::string>&& args, - bool* enable_modalias_handling) { +Result<Success> ParseEnabledDisabledLine(std::vector<std::string>&& args, bool* feature) { if (args.size() != 2) { - return Error() << "modalias_handling lines take exactly one parameter"; + return Error() << args[0] << " lines take exactly one parameter"; } if (args[1] == "enabled") { - *enable_modalias_handling = true; + *feature = true; } else if (args[1] == "disabled") { - *enable_modalias_handling = false; + *feature = false; } else { - return Error() << "modalias_handling takes either 'enabled' or 'disabled' as a parameter"; + return Error() << args[0] << " takes either 'enabled' or 'disabled' as a parameter"; } return Success(); @@ -220,11 +219,14 @@ UeventdConfiguration ParseConfig(const std::vector<std::string>& configs) { std::bind(ParseFirmwareDirectoriesLine, _1, &ueventd_configuration.firmware_directories)); parser.AddSingleLineParser("modalias_handling", - std::bind(ParseModaliasHandlingLine, _1, + std::bind(ParseEnabledDisabledLine, _1, &ueventd_configuration.enable_modalias_handling)); parser.AddSingleLineParser("uevent_socket_rcvbuf_size", std::bind(ParseUeventSocketRcvbufSizeLine, _1, &ueventd_configuration.uevent_socket_rcvbuf_size)); + parser.AddSingleLineParser("parallel_restorecon", + std::bind(ParseEnabledDisabledLine, _1, + &ueventd_configuration.enable_parallel_restorecon)); for (const auto& config : configs) { parser.ParseConfig(config); diff --git a/init/ueventd_parser.h b/init/ueventd_parser.h index d476decc9..b54dba88e 100644 --- a/init/ueventd_parser.h +++ b/init/ueventd_parser.h @@ -14,8 +14,7 @@ * limitations under the License. */ -#ifndef _INIT_UEVENTD_PARSER_H -#define _INIT_UEVENTD_PARSER_H +#pragma once #include <string> #include <vector> @@ -32,11 +31,10 @@ struct UeventdConfiguration { std::vector<std::string> firmware_directories; bool enable_modalias_handling = false; size_t uevent_socket_rcvbuf_size = 0; + bool enable_parallel_restorecon = false; }; UeventdConfiguration ParseConfig(const std::vector<std::string>& configs); } // namespace init } // namespace android - -#endif diff --git a/init/ueventd_parser_test.cpp b/init/ueventd_parser_test.cpp index 9c1cedf8b..885e79ddd 100644 --- a/init/ueventd_parser_test.cpp +++ b/init/ueventd_parser_test.cpp @@ -147,6 +147,24 @@ uevent_socket_rcvbuf_size 8M TestUeventdFile(ueventd_file, {{}, {}, {}, {}, false, 8 * 1024 * 1024}); } +TEST(ueventd_parser, EnabledDisabledLines) { + auto ueventd_file = R"( +modalias_handling enabled +parallel_restorecon enabled +modalias_handling disabled +)"; + + TestUeventdFile(ueventd_file, {{}, {}, {}, {}, false, 0, true}); + + auto ueventd_file2 = R"( +parallel_restorecon enabled +modalias_handling enabled +parallel_restorecon disabled +)"; + + TestUeventdFile(ueventd_file2, {{}, {}, {}, {}, true, 0, false}); +} + TEST(ueventd_parser, AllTogether) { auto ueventd_file = R"( @@ -179,6 +197,8 @@ subsystem test_devpath_dirname firmware_directories /more uevent_socket_rcvbuf_size 6M +modalias_handling enabled +parallel_restorecon enabled #ending comment )"; @@ -211,7 +231,7 @@ uevent_socket_rcvbuf_size 6M size_t uevent_socket_rcvbuf_size = 6 * 1024 * 1024; TestUeventdFile(ueventd_file, {subsystems, sysfs_permissions, permissions, firmware_directories, - false, uevent_socket_rcvbuf_size}); + true, uevent_socket_rcvbuf_size, true}); } // All of these lines are ill-formed, so test that there is 0 output. @@ -230,6 +250,13 @@ uevent_socket_rcvbuf_size blah subsystem #no name +modalias_handling +modalias_handling enabled enabled +modalias_handling blah + +parallel_restorecon +parallel_restorecon enabled enabled +parallel_restorecon blah )"; TestUeventdFile(ueventd_file, {}); |