From 5291a67ee3bdac708d597729046ccc303843a586 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Mon, 6 Dec 2021 14:29:40 -0800 Subject: [PATCH] storaged: use health HAL Test: storaged_test Test: manual Bug: 208543110 Change-Id: I60d42fc4f2e411f375fe98d1c8e145ce79c52f88 --- storaged/Android.bp | 11 ++- storaged/include/storaged.h | 31 +++++-- storaged/include/storaged_diskstats.h | 17 ++-- storaged/include/storaged_info.h | 14 +-- storaged/storaged.cpp | 120 +++++++++++++++++++------- storaged/storaged_diskstats.cpp | 44 ++++------ storaged/storaged_info.cpp | 34 ++++---- storaged/tests/storaged_test.cpp | 4 +- 8 files changed, 168 insertions(+), 107 deletions(-) diff --git a/storaged/Android.bp b/storaged/Android.bp index b557deed2..f2366cff1 100644 --- a/storaged/Android.bp +++ b/storaged/Android.bp @@ -24,8 +24,10 @@ cc_defaults { shared_libs: [ "android.hardware.health@1.0", "android.hardware.health@2.0", + "android.hardware.health-V1-ndk", "libbase", "libbinder", + "libbinder_ndk", "libcutils", "libhidlbase", "liblog", @@ -35,6 +37,12 @@ cc_defaults { "packagemanager_aidl-cpp", ], + static_libs: [ + "android.hardware.health-translate-ndk", + "libhealthhalutils", + "libhealthshim", + ], + cflags: [ "-Wall", "-Werror", @@ -67,7 +75,6 @@ cc_library_static { ":storaged_aidl_private", ], - static_libs: ["libhealthhalutils"], header_libs: ["libbatteryservice_headers"], logtags: ["EventLogTags.logtags"], @@ -90,7 +97,6 @@ cc_binary { srcs: ["main.cpp"], static_libs: [ - "libhealthhalutils", "libstoraged", ], } @@ -107,7 +113,6 @@ cc_test { srcs: ["tests/storaged_test.cpp"], static_libs: [ - "libhealthhalutils", "libstoraged", ], } diff --git a/storaged/include/storaged.h b/storaged/include/storaged.h index 79b5d41ac..e120271ee 100644 --- a/storaged/include/storaged.h +++ b/storaged/include/storaged.h @@ -28,6 +28,7 @@ #include +#include #include #define FRIEND_TEST(test_case_name, test_name) \ @@ -67,6 +68,8 @@ using namespace android; // UID IO threshold in bytes #define DEFAULT_PERIODIC_CHORES_UID_IO_THRESHOLD ( 1024 * 1024 * 1024ULL ) +class storaged_t; + struct storaged_config { int periodic_chores_interval_unit; int periodic_chores_interval_disk_stats_publish; @@ -75,15 +78,33 @@ struct storaged_config { int event_time_check_usec; // check how much cputime spent in event loop }; -class storaged_t : public android::hardware::health::V2_0::IHealthInfoCallback, - public android::hardware::hidl_death_recipient { +struct HealthServicePair { + std::shared_ptr aidl_health; + android::sp hidl_health; + static HealthServicePair get(); +}; + +class hidl_health_death_recipient : public android::hardware::hidl_death_recipient { + public: + hidl_health_death_recipient(const android::sp& health) + : mHealth(health) {} + void serviceDied(uint64_t cookie, const wp<::android::hidl::base::V1_0::IBase>& who); + + private: + android::sp mHealth; +}; + +class storaged_t : public RefBase { private: time_t mTimer; storaged_config mConfig; unique_ptr mDsm; uid_monitor mUidm; time_t mStarttime; - sp health; + std::shared_ptr health; + sp hidl_death_recp; + ndk::ScopedAIBinder_DeathRecipient aidl_death_recp; + shared_ptr aidl_health_callback; unique_ptr storage_info; static const uint32_t current_version; Mutex proto_lock; @@ -135,10 +156,6 @@ class storaged_t : public android::hardware::health::V2_0::IHealthInfoCallback, void add_user_ce(userid_t user_id); void remove_user_ce(userid_t user_id); - virtual ::android::hardware::Return healthInfoChanged( - const ::android::hardware::health::V2_0::HealthInfo& info); - void serviceDied(uint64_t cookie, const wp<::android::hidl::base::V1_0::IBase>& who); - void report_storage_info(); void flush_protos(unordered_map* protos); diff --git a/storaged/include/storaged_diskstats.h b/storaged/include/storaged_diskstats.h index 0b93ba6c5..3996ef6d1 100644 --- a/storaged/include/storaged_diskstats.h +++ b/storaged/include/storaged_diskstats.h @@ -19,7 +19,7 @@ #include -#include +#include // number of attributes diskstats has #define DISK_STATS_SIZE ( 11 ) @@ -162,7 +162,7 @@ private: const double mSigma; struct disk_perf mMean; struct disk_perf mStd; - android::sp mHealth; + std::shared_ptr mHealth; void update_mean(); void update_std(); @@ -173,14 +173,15 @@ private: void update(struct disk_stats* stats); public: - disk_stats_monitor(const android::sp& healthService, + disk_stats_monitor(const std::shared_ptr& healthService, uint32_t window_size = 5, double sigma = 1.0) : DISK_STATS_PATH( - healthService != nullptr - ? nullptr - : (access(MMC_DISK_STATS_PATH, R_OK) == 0 - ? MMC_DISK_STATS_PATH - : (access(SDA_DISK_STATS_PATH, R_OK) == 0 ? SDA_DISK_STATS_PATH : nullptr))), + healthService != nullptr + ? nullptr + : (access(MMC_DISK_STATS_PATH, R_OK) == 0 + ? MMC_DISK_STATS_PATH + : (access(SDA_DISK_STATS_PATH, R_OK) == 0 ? SDA_DISK_STATS_PATH + : nullptr))), mPrevious(), mAccumulate(), mAccumulate_pub(), diff --git a/storaged/include/storaged_info.h b/storaged/include/storaged_info.h index 9c3d0e784..83c97ad07 100644 --- a/storaged/include/storaged_info.h +++ b/storaged/include/storaged_info.h @@ -21,7 +21,7 @@ #include -#include +#include #include #include "storaged.h" @@ -71,8 +71,8 @@ class storage_info_t { public: static storage_info_t* get_storage_info( - const sp& healthService); - virtual ~storage_info_t() {}; + const shared_ptr& healthService); + virtual ~storage_info_t(){}; virtual void report() {}; void load_perf_history_proto(const IOPerfHistory& perf_history); void refresh(IOPerfHistory* perf_history); @@ -105,14 +105,14 @@ public: class health_storage_info_t : public storage_info_t { private: - using IHealth = hardware::health::V2_0::IHealth; - using StorageInfo = hardware::health::V2_0::StorageInfo; + using IHealth = aidl::android::hardware::health::IHealth; + using StorageInfo = aidl::android::hardware::health::StorageInfo; - sp mHealth; + shared_ptr mHealth; void set_values_from_hal_storage_info(const StorageInfo& halInfo); public: - health_storage_info_t(const sp& service) : mHealth(service){}; + health_storage_info_t(const shared_ptr& service) : mHealth(service){}; virtual ~health_storage_info_t() {} virtual void report(); }; diff --git a/storaged/storaged.cpp b/storaged/storaged.cpp index b7aa89ff7..fb855f750 100644 --- a/storaged/storaged.cpp +++ b/storaged/storaged.cpp @@ -28,12 +28,16 @@ #include #include +#include #include #include #include +#include +#include #include #include #include +#include #include #include #include @@ -64,26 +68,59 @@ constexpr ssize_t min_benchmark_size = 128 * 1024; // 128KB const uint32_t storaged_t::current_version = 4; +using aidl::android::hardware::health::BatteryStatus; +using aidl::android::hardware::health::BnHealthInfoCallback; +using aidl::android::hardware::health::HealthInfo; +using aidl::android::hardware::health::IHealth; +using aidl::android::hardware::health::IHealthInfoCallback; using android::hardware::interfacesEqual; -using android::hardware::Return; -using android::hardware::health::V1_0::BatteryStatus; -using android::hardware::health::V1_0::toString; using android::hardware::health::V2_0::get_health_service; -using android::hardware::health::V2_0::HealthInfo; -using android::hardware::health::V2_0::IHealth; -using android::hardware::health::V2_0::Result; using android::hidl::manager::V1_0::IServiceManager; +using HidlHealth = android::hardware::health::V2_0::IHealth; +using aidl::android::hardware::health::HealthShim; +using ndk::ScopedAIBinder_DeathRecipient; +using ndk::ScopedAStatus; +HealthServicePair HealthServicePair::get() { + HealthServicePair ret; + auto service_name = IHealth::descriptor + "/default"s; + if (AServiceManager_isDeclared(service_name.c_str())) { + ndk::SpAIBinder binder(AServiceManager_waitForService(service_name.c_str())); + ret.aidl_health = IHealth::fromBinder(binder); + if (ret.aidl_health == nullptr) { + LOG(WARNING) << "AIDL health service is declared, but it cannot be retrieved."; + } + } + if (ret.aidl_health == nullptr) { + LOG(INFO) << "Unable to get AIDL health service, trying HIDL..."; + ret.hidl_health = get_health_service(); + if (ret.hidl_health != nullptr) { + ret.aidl_health = ndk::SharedRefBase::make(ret.hidl_health); + } + } + if (ret.aidl_health == nullptr) { + LOG(WARNING) << "health: failed to find IHealth service"; + return {}; + } + return ret; +} inline charger_stat_t is_charger_on(BatteryStatus prop) { return (prop == BatteryStatus::CHARGING || prop == BatteryStatus::FULL) ? CHARGER_ON : CHARGER_OFF; } -Return storaged_t::healthInfoChanged(const HealthInfo& props) { - mUidm.set_charger_state(is_charger_on(props.legacy.batteryStatus)); - return android::hardware::Void(); -} +class HealthInfoCallback : public BnHealthInfoCallback { + public: + HealthInfoCallback(uid_monitor* uidm) : mUidm(uidm) {} + ScopedAStatus healthInfoChanged(const HealthInfo& info) override { + mUidm->set_charger_state(is_charger_on(info.batteryStatus)); + return ScopedAStatus::ok(); + } + + private: + uid_monitor* mUidm; +}; void storaged_t::init() { init_health_service(); @@ -91,42 +128,59 @@ void storaged_t::init() { storage_info.reset(storage_info_t::get_storage_info(health)); } +static void onHealthBinderDied(void*) { + LOG(ERROR) << "health service died, exiting"; + android::hardware::IPCThreadState::self()->stopProcess(); + exit(1); +} + void storaged_t::init_health_service() { if (!mUidm.enabled()) return; - health = get_health_service(); - if (health == NULL) { - LOG(WARNING) << "health: failed to find IHealth service"; - return; - } + auto [aidlHealth, hidlHealth] = HealthServicePair::get(); + health = aidlHealth; + if (health == nullptr) return; BatteryStatus status = BatteryStatus::UNKNOWN; - auto ret = health->getChargeStatus([&](Result r, BatteryStatus v) { - if (r != Result::SUCCESS) { - LOG(WARNING) << "health: cannot get battery status " << toString(r); - return; - } - if (v == BatteryStatus::UNKNOWN) { - LOG(WARNING) << "health: invalid battery status"; - } - status = v; - }); + auto ret = health->getChargeStatus(&status); if (!ret.isOk()) { - LOG(WARNING) << "health: get charge status transaction error " << ret.description(); + LOG(WARNING) << "health: cannot get battery status: " << ret.getDescription(); + } + if (status == BatteryStatus::UNKNOWN) { + LOG(WARNING) << "health: invalid battery status"; } mUidm.init(is_charger_on(status)); // register listener after init uid_monitor - health->registerCallback(this); - health->linkToDeath(this, 0 /* cookie */); + aidl_health_callback = std::make_shared(&mUidm); + ret = health->registerCallback(aidl_health_callback); + if (!ret.isOk()) { + LOG(WARNING) << "health: failed to register callback: " << ret.getDescription(); + } + + if (hidlHealth != nullptr) { + hidl_death_recp = new hidl_health_death_recipient(hidlHealth); + auto ret = hidlHealth->linkToDeath(hidl_death_recp, 0 /* cookie */); + if (!ret.isOk()) { + LOG(WARNING) << "Failed to link to death (HIDL): " << ret.description(); + } + } else { + aidl_death_recp = + ScopedAIBinder_DeathRecipient(AIBinder_DeathRecipient_new(onHealthBinderDied)); + auto ret = AIBinder_linkToDeath(health->asBinder().get(), aidl_death_recp.get(), + nullptr /* cookie */); + if (ret != STATUS_OK) { + LOG(WARNING) << "Failed to link to death (AIDL): " + << ScopedAStatus(AStatus_fromStatus(ret)).getDescription(); + } + } } -void storaged_t::serviceDied(uint64_t cookie, const wp<::android::hidl::base::V1_0::IBase>& who) { - if (health != NULL && interfacesEqual(health, who.promote())) { - LOG(ERROR) << "health service died, exiting"; - android::hardware::IPCThreadState::self()->stopProcess(); - exit(1); +void hidl_health_death_recipient::serviceDied(uint64_t cookie, + const wp<::android::hidl::base::V1_0::IBase>& who) { + if (mHealth != nullptr && interfacesEqual(mHealth, who.promote())) { + onHealthBinderDied(reinterpret_cast(cookie)); } else { LOG(ERROR) << "unknown service died"; } diff --git a/storaged/storaged_diskstats.cpp b/storaged/storaged_diskstats.cpp index 52bd4e003..1eae5a10b 100644 --- a/storaged/storaged_diskstats.cpp +++ b/storaged/storaged_diskstats.cpp @@ -30,11 +30,8 @@ namespace { -using android::sp; -using android::hardware::health::V2_0::DiskStats; -using android::hardware::health::V2_0::IHealth; -using android::hardware::health::V2_0::Result; -using android::hardware::health::V2_0::toString; +using aidl::android::hardware::health::DiskStats; +using aidl::android::hardware::health::IHealth; #ifdef DEBUG void log_debug_disk_perf(struct disk_perf* perf, const char* type) { @@ -121,39 +118,30 @@ void convert_hal_disk_stats(struct disk_stats* dst, const DiskStats& src) { dst->io_in_queue = src.ioInQueue; } -bool get_disk_stats_from_health_hal(const sp& service, struct disk_stats* stats) { +bool get_disk_stats_from_health_hal(const std::shared_ptr& service, + struct disk_stats* stats) { struct timespec ts; if (!get_time(&ts)) { return false; } - bool success = false; - auto ret = service->getDiskStats([&success, stats](auto result, const auto& halStats) { - if (result == Result::NOT_SUPPORTED) { - LOG(DEBUG) << "getDiskStats is not supported on health HAL."; - return; + std::vector halStats; + auto ret = service->getDiskStats(&halStats); + if (ret.isOk()) { + if (halStats.size() > 0) { + convert_hal_disk_stats(stats, halStats[0]); + init_disk_stats_other(ts, stats); + return true; } - if (result != Result::SUCCESS || halStats.size() == 0) { - LOG(ERROR) << "getDiskStats failed with result " << toString(result) << " and size " - << halStats.size(); - return; - } - - convert_hal_disk_stats(stats, halStats[0]); - success = true; - }); - - if (!ret.isOk()) { - LOG(ERROR) << "getDiskStats failed with " << ret.description(); + LOG(ERROR) << "getDiskStats succeeded but size is 0"; return false; } - - if (!success) { + if (ret.getExceptionCode() == EX_UNSUPPORTED_OPERATION) { + LOG(DEBUG) << "getDiskStats is not supported on health HAL."; return false; } - - init_disk_stats_other(ts, stats); - return true; + LOG(ERROR) << "getDiskStats failed with " << ret.getDescription(); + return false; } struct disk_perf get_disk_perf(struct disk_stats* stats) diff --git a/storaged/storaged_info.cpp b/storaged/storaged_info.cpp index bb2182919..33500db2d 100644 --- a/storaged/storaged_info.cpp +++ b/storaged/storaged_info.cpp @@ -36,9 +36,8 @@ using namespace chrono; using namespace android::base; using namespace storaged_proto; -using android::hardware::health::V2_0::IHealth; -using android::hardware::health::V2_0::Result; -using android::hardware::health::V2_0::StorageInfo; +using aidl::android::hardware::health::IHealth; +using aidl::android::hardware::health::StorageInfo; const string emmc_info_t::emmc_sysfs = "/sys/bus/mmc/devices/mmc0:0001/"; const char* emmc_info_t::emmc_ver_str[9] = { @@ -57,7 +56,7 @@ bool FileExists(const std::string& filename) } // namespace -storage_info_t* storage_info_t::get_storage_info(const sp& healthService) { +storage_info_t* storage_info_t::get_storage_info(const shared_ptr& healthService) { if (healthService != nullptr) { return new health_storage_info_t(healthService); } @@ -326,23 +325,22 @@ void ufs_info_t::report() } void health_storage_info_t::report() { - auto ret = mHealth->getStorageInfo([this](auto result, const auto& halInfos) { - if (result == Result::NOT_SUPPORTED) { - LOG(DEBUG) << "getStorageInfo is not supported on health HAL."; + vector halInfos; + auto ret = mHealth->getStorageInfo(&halInfos); + if (ret.isOk()) { + if (halInfos.size() == 0) { + set_values_from_hal_storage_info(halInfos[0]); + publish(); return; } - if (result != Result::SUCCESS || halInfos.size() == 0) { - LOG(ERROR) << "getStorageInfo failed with result " << toString(result) << " and size " - << halInfos.size(); - return; - } - set_values_from_hal_storage_info(halInfos[0]); - publish(); - }); - - if (!ret.isOk()) { - LOG(ERROR) << "getStorageInfo failed with " << ret.description(); + LOG(ERROR) << "getStorageInfo succeeded but size is 0"; + return; } + if (ret.getExceptionCode() == EX_UNSUPPORTED_OPERATION) { + LOG(DEBUG) << "getStorageInfo is not supported on health HAL."; + return; + } + LOG(ERROR) << "getStorageInfo failed with " << ret.getDescription(); } void health_storage_info_t::set_values_from_hal_storage_info(const StorageInfo& halInfo) { diff --git a/storaged/tests/storaged_test.cpp b/storaged/tests/storaged_test.cpp index 004d0b993..e987f76f2 100644 --- a/storaged/tests/storaged_test.cpp +++ b/storaged/tests/storaged_test.cpp @@ -240,9 +240,7 @@ void expect_increasing(struct disk_stats stats1, struct disk_stats stats2) { } TEST(storaged_test, disk_stats_monitor) { - using android::hardware::health::V2_0::get_health_service; - - auto healthService = get_health_service(); + auto [healthService, hidlHealth] = HealthServicePair::get(); // asserting that there is one file for diskstats ASSERT_TRUE(healthService != nullptr || access(MMC_DISK_STATS_PATH, R_OK) >= 0 ||