Revert "powerstats: timeout mechanism for AoC data provider"

Revert submission 17800080-power-stats-aoc-timeout

Reason for revert: Causing crashes due to improper thread sync
Reverted Changes:
I71a9e6817:powerstats: timeout mechanism for AoC data provide...
I725c00745:powerstats: set time limit as timeout for AoC
I5b290a53d:powerstats: set time limit as timeout for AoC

Change-Id: I6844a141739f93154d10debc29c7565f92349d25
This commit is contained in:
Kelly Rossmoyer 2022-04-24 23:22:49 +00:00
parent df08a31b76
commit 20b039192d
2 changed files with 9 additions and 96 deletions

View File

@ -17,8 +17,6 @@
#include "AocStateResidencyDataProvider.h"
#include <android-base/logging.h>
#include <chrono>
#include <pthread.h>
namespace aidl {
namespace android {
@ -26,23 +24,12 @@ namespace hardware {
namespace power {
namespace stats {
struct async_data_t {
pthread_cond_t *cond;
pthread_mutex_t *lock;
uint64_t timeoutMillis;
std::unordered_map<std::string, std::vector<StateResidency>> *residencies;
std::unordered_map<std::string,
std::vector<std::unique_ptr<GenericStateResidencyDataProvider>>> *providers;
};
AocStateResidencyDataProvider::AocStateResidencyDataProvider(std::vector<std::pair<std::string,
std::string>> ids, std::vector<std::pair<std::string, std::string>> states,
const uint64_t timeoutMillis) {
std::string>> ids, std::vector<std::pair<std::string, std::string>> 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<uint64_t(uint64_t)> aocTickToMs = [](uint64_t a) { return a / AOC_CLK; };
GenericStateResidencyDataProvider::StateResidencyConfig config = {
.entryCountSupported = true,
@ -67,17 +54,13 @@ AocStateResidencyDataProvider::AocStateResidencyDataProvider(std::vector<std::pa
mProviders[id.first].push_back(std::move(sdp));
}
}
mStateCount = states.size();
mTimeoutMillis = timeoutMillis == 0 ? TIMEOUT_MILLIS : timeoutMillis;
}
void *getStateResidenciesAsync(void *arg) {
struct timespec start, end;
struct async_data_t *async = (struct async_data_t*)arg;
uint64_t timeoutUs = async->timeoutMillis * 1000;
bool AocStateResidencyDataProvider::getStateResidencies(
std::unordered_map<std::string, std::vector<StateResidency>> *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<StateResidency> stateResidencies;
@ -85,18 +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<std::string, std::vector<StateResidency>> residency;
clock_gettime(CLOCK_REALTIME, &start);
provider->getStateResidencies(&residency);
clock_gettime(CLOCK_REALTIME, &end);
uint64_t elapsedUs =
((end.tv_sec - start.tv_sec) * 1000000) + ((end.tv_nsec - start.tv_nsec) / 1000);
if (elapsedUs >= timeoutUs) {
LOG(WARNING) << "getStateResidencies latency for " << curEntity
<< " exceeds time allowed: " << elapsedUs << " us";
return 0;
}
ret &= provider->getStateResidencies(&residency);
// Each provider should only return data for curEntity but checking anyway
if (residency.find(curEntity) != residency.end()) {
@ -112,64 +84,8 @@ void *getStateResidenciesAsync(void *arg) {
}
}
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<std::string, std::vector<StateResidency>> *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<std::string, std::vector<StateResidency>> stateResidencies;
struct timespec start, timeout;
struct async_data_t async = {
.cond = &cond,
.lock = &lock,
.timeoutMillis = mTimeoutMillis,
.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;
}

View File

@ -27,8 +27,7 @@ namespace stats {
class AocStateResidencyDataProvider : public PowerStats::IStateResidencyDataProvider {
public:
AocStateResidencyDataProvider(std::vector<std::pair<std::string, std::string>> ids,
std::vector<std::pair<std::string, std::string>> states,
const uint64_t timeoutMillis);
std::vector<std::pair<std::string, std::string>> states);
~AocStateResidencyDataProvider() = default;
bool getStateResidencies(
std::unordered_map<std::string, std::vector<StateResidency>> *residencies) override;
@ -37,12 +36,10 @@ class AocStateResidencyDataProvider : public PowerStats::IStateResidencyDataProv
private:
std::unordered_map<std::string /* entity name */,
std::vector<std::unique_ptr<GenericStateResidencyDataProvider>> /* providers */> mProviders;
int32_t mStateCount;
uint64_t mTimeoutMillis;
};
} // namespace stats
} // namespace power
} // namespace hardware
} // namespace android
} // namespace aidl
} // namespace aidl