diff options
| -rw-r--r-- | cmds/installd/dexopt.cpp | 65 | ||||
| -rw-r--r-- | cmds/installd/tests/installd_dexopt_test.cpp | 50 |
2 files changed, 103 insertions, 12 deletions
diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp index 838d11d6d8..768d900591 100644 --- a/cmds/installd/dexopt.cpp +++ b/cmds/installd/dexopt.cpp @@ -713,6 +713,7 @@ static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION = 1; static constexpr int PROFMAN_BIN_RETURN_CODE_BAD_PROFILES = 2; static constexpr int PROFMAN_BIN_RETURN_CODE_ERROR_IO = 3; static constexpr int PROFMAN_BIN_RETURN_CODE_ERROR_LOCKING = 4; +static constexpr int PROFMAN_BIN_RETURN_CODE_SUCCESS = 5; class RunProfman : public ExecVHelper { public: @@ -720,7 +721,9 @@ class RunProfman : public ExecVHelper { const unique_fd& reference_profile_fd, const std::vector<unique_fd>& apk_fds, const std::vector<std::string>& dex_locations, - bool copy_and_update) { + bool copy_and_update, + bool for_snapshot, + bool for_boot_image) { // TODO(calin): Assume for now we run in the bg compile job (which is in // most of the invocation). With the current data flow, is not very easy or @@ -752,6 +755,14 @@ class RunProfman : public ExecVHelper { AddArg("--copy-and-update-profile-key"); } + if (for_snapshot) { + AddArg("--force-merge"); + } + + if (for_boot_image) { + AddArg("--boot-image-merge"); + } + // Do not add after dex2oat_flags, they should override others for debugging. PrepareArgs(profman_bin); } @@ -759,12 +770,16 @@ class RunProfman : public ExecVHelper { void SetupMerge(const std::vector<unique_fd>& profiles_fd, const unique_fd& reference_profile_fd, const std::vector<unique_fd>& apk_fds = std::vector<unique_fd>(), - const std::vector<std::string>& dex_locations = std::vector<std::string>()) { + const std::vector<std::string>& dex_locations = std::vector<std::string>(), + bool for_snapshot = false, + bool for_boot_image = false) { SetupArgs(profiles_fd, reference_profile_fd, apk_fds, dex_locations, - /*copy_and_update=*/false); + /*copy_and_update=*/ false, + for_snapshot, + for_boot_image); } void SetupCopyAndUpdate(unique_fd&& profile_fd, @@ -781,7 +796,9 @@ class RunProfman : public ExecVHelper { reference_profile_fd_, apk_fds_, dex_locations, - /*copy_and_update=*/true); + /*copy_and_update=*/true, + /*for_snapshot*/false, + /*for_boot_image*/false); } void SetupDump(const std::vector<unique_fd>& profiles_fd, @@ -795,7 +812,9 @@ class RunProfman : public ExecVHelper { reference_profile_fd, apk_fds, dex_locations, - /*copy_and_update=*/false); + /*copy_and_update=*/false, + /*for_snapshot*/false, + /*for_boot_image*/false); } void Exec() { @@ -872,7 +891,7 @@ static bool analyze_profiles(uid_t uid, const std::string& package_name, break; default: // Unknown return code or error. Unlink profiles. - LOG(WARNING) << "Unknown error code while processing profiles for location " + LOG(WARNING) << "Unexpected error code while processing profiles for location " << location << ": " << return_code; need_to_compile = false; should_clear_current_profiles = true; @@ -2741,7 +2760,7 @@ static bool create_app_profile_snapshot(int32_t app_id, } RunProfman args; - args.SetupMerge(profiles_fd, snapshot_fd, apk_fds, dex_locations); + args.SetupMerge(profiles_fd, snapshot_fd, apk_fds, dex_locations, /*for_snapshot=*/true); pid_t pid = fork(); if (pid == 0) { /* child -- drop privileges before continuing */ @@ -2756,6 +2775,13 @@ static bool create_app_profile_snapshot(int32_t app_id, return false; } + // Verify that profman finished successfully. + int profman_code = WEXITSTATUS(return_code); + if (profman_code != PROFMAN_BIN_RETURN_CODE_SUCCESS) { + LOG(WARNING) << "profman error for " << package_name << ":" << profile_name + << ":" << profman_code; + return false; + } return true; } @@ -2818,19 +2844,29 @@ static bool create_boot_image_profile_snapshot(const std::string& package_name, // We do this to avoid opening a huge a amount of files. static constexpr size_t kAggregationBatchSize = 10; - std::vector<unique_fd> profiles_fd; for (size_t i = 0; i < profiles.size(); ) { + std::vector<unique_fd> profiles_fd; for (size_t k = 0; k < kAggregationBatchSize && i < profiles.size(); k++, i++) { unique_fd fd = open_profile(AID_SYSTEM, profiles[i], O_RDONLY); if (fd.get() >= 0) { profiles_fd.push_back(std::move(fd)); } } + + // We aggregate (read & write) into the same fd multiple times in a row. + // We need to reset the cursor every time to ensure we read the whole file every time. + if (TEMP_FAILURE_RETRY(lseek(snapshot_fd, 0, SEEK_SET)) == static_cast<off_t>(-1)) { + PLOG(ERROR) << "Cannot reset position for snapshot profile"; + return false; + } + RunProfman args; args.SetupMerge(profiles_fd, snapshot_fd, apk_fds, - dex_locations); + dex_locations, + /*for_snapshot=*/true, + /*for_boot_image=*/true); pid_t pid = fork(); if (pid == 0) { /* child -- drop privileges before continuing */ @@ -2843,12 +2879,21 @@ static bool create_boot_image_profile_snapshot(const std::string& package_name, /* parent */ int return_code = wait_child(pid); + if (!WIFEXITED(return_code)) { PLOG(WARNING) << "profman failed for " << package_name << ":" << profile_name; return false; } - return true; + + // Verify that profman finished successfully. + int profman_code = WEXITSTATUS(return_code); + if (profman_code != PROFMAN_BIN_RETURN_CODE_SUCCESS) { + LOG(WARNING) << "profman error for " << package_name << ":" << profile_name + << ":" << profman_code; + return false; + } } + return true; } diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp index 0212bc5564..69fefa199b 100644 --- a/cmds/installd/tests/installd_dexopt_test.cpp +++ b/cmds/installd/tests/installd_dexopt_test.cpp @@ -897,7 +897,9 @@ class ProfileTest : public DexoptTest { std::string expected_profile_content = snap_profile_ + ".expected"; run_cmd("rm -f " + expected_profile_content); run_cmd("touch " + expected_profile_content); - run_cmd("profman --profile-file=" + cur_profile_ + + // We force merging when creating the expected profile to make sure + // that the random profiles do not affect the output. + run_cmd("profman --force-merge --profile-file=" + cur_profile_ + " --profile-file=" + ref_profile_ + " --reference-profile-file=" + expected_profile_content + " --apk=" + apk_path_); @@ -1130,16 +1132,60 @@ TEST_F(ProfileTest, ProfilePrepareFailProfileChangedUid) { class BootProfileTest : public ProfileTest { public: - virtual void setup() { + std::vector<const std::string> extra_apps_; + std::vector<int64_t> extra_ce_data_inodes_; + + virtual void SetUp() { + ProfileTest::SetUp(); intial_android_profiles_dir = android_profiles_dir; + // Generate profiles for some extra apps. + // When merging boot profile we split profiles into small groups to avoid + // opening a lot of file descriptors at the same time. + // (Currently the group size for aggregation is 10) + // + // To stress test that works fine, create profile for more apps. + createAppProfilesForBootMerge(21); } virtual void TearDown() { android_profiles_dir = intial_android_profiles_dir; + deleteAppProfilesForBootMerge(); ProfileTest::TearDown(); } + void createAppProfilesForBootMerge(size_t number_of_profiles) { + for (size_t i = 0; i < number_of_profiles; i++) { + int64_t ce_data_inode; + std::string package_name = "dummy_test_pkg" + std::to_string(i); + LOG(INFO) << package_name; + ASSERT_BINDER_SUCCESS(service_->createAppData( + volume_uuid_, + package_name, + kTestUserId, + kAppDataFlags, + kTestAppUid, + se_info_, + kOSdkVersion, + &ce_data_inode)); + extra_apps_.push_back(package_name); + extra_ce_data_inodes_.push_back(ce_data_inode); + std::string profile = create_current_profile_path( + kTestUserId, package_name, kPrimaryProfile, /*is_secondary_dex*/ false); + SetupProfile(profile, kTestAppUid, kTestAppGid, 0600, 1); + } + } + + void deleteAppProfilesForBootMerge() { + if (kDebug) { + return; + } + for (size_t i = 0; i < extra_apps_.size(); i++) { + service_->destroyAppData( + volume_uuid_, extra_apps_[i], kTestUserId, kAppDataFlags, extra_ce_data_inodes_[i]); + } + } + void UpdateAndroidProfilesDir(const std::string& profile_dir) { android_profiles_dir = profile_dir; // We need to create the reference profile directory in the new profile dir. |