From 2433fcf62ed5239d89d442062688902cce41a6a3 Mon Sep 17 00:00:00 2001 From: Kelly Rossmoyer Date: Fri, 1 Apr 2022 20:28:24 +0000 Subject: [PATCH] Revert "powerstats: timeout mechanism for AoC data provider" Revert submission 17442015-powerstats-aoc-timeout Reason for revert: Causes PowerStatsHAL crashes (b/227578192) Reverted Changes: I9a9eb27fa:powerstats: set time limit as timeout for AoC Ib70000580:powerstats: timeout mechanism for AoC data provide... Ied599054c:Revert "powerstats: disable AoC power stats report... Iaf5af43eb:powerstats: set time limit as timeout for AoC Change-Id: I53e280871197aaf003d9ef3e5e0cb4c79f486cc8 --- powerstats/AocStateResidencyDataProvider.cpp | 95 ++----------------- .../include/AocStateResidencyDataProvider.h | 7 +- 2 files changed, 9 insertions(+), 93 deletions(-) diff --git a/powerstats/AocStateResidencyDataProvider.cpp b/powerstats/AocStateResidencyDataProvider.cpp index b8d40ea..c64496d 100644 --- a/powerstats/AocStateResidencyDataProvider.cpp +++ b/powerstats/AocStateResidencyDataProvider.cpp @@ -17,8 +17,6 @@ #include "AocStateResidencyDataProvider.h" #include -#include -#include namespace aidl { namespace android { @@ -26,22 +24,12 @@ namespace hardware { namespace power { namespace stats { -struct async_data_t { - pthread_cond_t *cond; - pthread_mutex_t *lock; - std::unordered_map> *residencies; - std::unordered_map>> *providers; -}; - AocStateResidencyDataProvider::AocStateResidencyDataProvider(std::vector> ids, std::vector> states, - const uint64_t timeoutMillis) { + std::string>> ids, std::vector> states) { // AoC stats are reported in ticks of 244.140625ns. The transform // function converts ticks to milliseconds. // 1000000 / 244.140625 = 4096. static const uint64_t AOC_CLK = 4096; - static const uint64_t TIMEOUT_MILLIS = 120; std::function aocTickToMs = [](uint64_t a) { return a / AOC_CLK; }; GenericStateResidencyDataProvider::StateResidencyConfig config = { .entryCountSupported = true, @@ -66,16 +54,13 @@ AocStateResidencyDataProvider::AocStateResidencyDataProvider(std::vectorresidencies; - +bool AocStateResidencyDataProvider::getStateResidencies( + std::unordered_map> *residencies) { // States from the same power entity are merged. - for (const auto &providerList : *async->providers) { + bool ret = true; + for (const auto &providerList : mProviders) { int32_t stateId = 0; std::string curEntity = providerList.first; std::vector stateResidencies; @@ -83,7 +68,7 @@ void *getStateResidenciesAsync(void *arg) { // Iterate over each provider in the providerList, appending each of the states for (const auto &provider : providerList.second) { std::unordered_map> residency; - provider->getStateResidencies(&residency); + ret &= provider->getStateResidencies(&residency); // Each provider should only return data for curEntity but checking anyway if (residency.find(curEntity) != residency.end()) { @@ -99,74 +84,8 @@ void *getStateResidenciesAsync(void *arg) { } } - if (async->residencies != originalContainer) { - /* - * When provider->gretStateResidencies() exceeds timeout expiration, the main thread - * stops waiting and continue, then the original residency pointer is erased in the - * main thread when AocStateResidencyDataProvider::getStateResidencies() ends, and new - * container will be created in the async thread later. In this situation, we ignore - * delayed residency data and end the async thread. - */ - return 0; - } - - async->residencies->emplace(curEntity, stateResidencies); + residencies->emplace(curEntity, stateResidencies); } - - pthread_mutex_lock(async->lock); - pthread_cond_signal(async->cond); - pthread_mutex_unlock(async->lock); - - return 0; -} - -bool AocStateResidencyDataProvider::getStateResidencies( - std::unordered_map> *residencies) { - bool ret = true; - int condResult = 0; - pthread_t tid; - pthread_cond_t cond = PTHREAD_COND_INITIALIZER; - pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; - std::unordered_map> stateResidencies; - struct timespec start, timeout; - struct async_data_t async = { - .cond = &cond, - .lock = &lock, - .residencies = &stateResidencies, - .providers = &mProviders - }; - - pthread_create(&tid, NULL, &getStateResidenciesAsync, (void*)&async); - - clock_gettime(CLOCK_REALTIME, &start); - - uint64_t expirationMillis = mTimeoutMillis * mStateCount; - timeout.tv_sec = start.tv_sec + expirationMillis / 1000; - uint64_t nsec = start.tv_nsec + (expirationMillis % 1000) * 1000000; - if (nsec < 1000000000) { - timeout.tv_nsec = nsec; - } else { - timeout.tv_sec += 1; - timeout.tv_nsec = nsec - 1000000000; - } - - pthread_mutex_lock(&lock); - condResult = pthread_cond_timedwait(&cond, &lock, &timeout); - pthread_mutex_unlock(&lock); - - if (condResult != 0) { - if (condResult == ETIMEDOUT) { - LOG(WARNING) << __func__ << " latency for AoC timeout: " << expirationMillis << " ms"; - } else { - LOG(ERROR) << "Failed to wait for the condition variable: " << condResult; - } - ret = false; - } else { - for (const auto &residency : stateResidencies) { - residencies->emplace(residency.first, residency.second); - } - } - return ret; } diff --git a/powerstats/include/AocStateResidencyDataProvider.h b/powerstats/include/AocStateResidencyDataProvider.h index 150f335..5008912 100644 --- a/powerstats/include/AocStateResidencyDataProvider.h +++ b/powerstats/include/AocStateResidencyDataProvider.h @@ -27,8 +27,7 @@ namespace stats { class AocStateResidencyDataProvider : public PowerStats::IStateResidencyDataProvider { public: AocStateResidencyDataProvider(std::vector> ids, - std::vector> states, - const uint64_t timeoutMillis); + std::vector> states); ~AocStateResidencyDataProvider() = default; bool getStateResidencies( std::unordered_map> *residencies) override; @@ -37,12 +36,10 @@ class AocStateResidencyDataProvider : public PowerStats::IStateResidencyDataProv private: std::unordered_map> /* providers */> mProviders; - int32_t mStateCount; - uint64_t mTimeoutMillis; }; } // namespace stats } // namespace power } // namespace hardware } // namespace android -} // namespace aidl +} // namespace aidl \ No newline at end of file