George Hung | 712a27e | 2020-05-19 15:52:33 +0800 | [diff] [blame^] | 1 | From 61b91122b72d7a3a5c93b80b6fb6d3ac892619dc Mon Sep 17 00:00:00 2001 |
| 2 | From: Eddielu <Eddie.Lu@quantatw.com> |
| 3 | Date: Mon, 1 Jun 2020 14:18:33 +0800 |
| 4 | Subject: [PATCH] add sensors slow readings |
| 5 | |
| 6 | --- |
| 7 | mainloop.cpp | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++---- |
| 8 | mainloop.hpp | 3 +++ |
| 9 | sensor.cpp | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++------- |
| 10 | sensor.hpp | 21 +++++++++++++++-- |
| 11 | 4 files changed, 158 insertions(+), 15 deletions(-) |
| 12 | |
| 13 | diff --git a/mainloop.cpp b/mainloop.cpp |
| 14 | index 1769e94..5094343 100644 |
| 15 | --- a/mainloop.cpp |
| 16 | +++ b/mainloop.cpp |
| 17 | @@ -32,6 +32,7 @@ |
| 18 | #include <cassert> |
| 19 | #include <cstdlib> |
| 20 | #include <functional> |
| 21 | +#include <future> |
| 22 | #include <iostream> |
| 23 | #include <memory> |
| 24 | #include <phosphor-logging/elog-errors.hpp> |
| 25 | @@ -296,7 +297,7 @@ std::optional<ObjectStateData> |
| 26 | { |
| 27 | // Add status interface based on _fault file being present |
| 28 | sensorObj->addStatus(info); |
| 29 | - valueInterface = sensorObj->addValue(retryIO, info); |
| 30 | + valueInterface = sensorObj->addValue(retryIO, info, _timedoutMap); |
| 31 | } |
| 32 | catch (const std::system_error& e) |
| 33 | { |
| 34 | @@ -542,10 +543,74 @@ void MainLoop::read() |
| 35 | // RAII object for GPIO unlock / lock |
| 36 | auto locker = sensor::gpioUnlock(sensor->getGpio()); |
| 37 | |
| 38 | - // Retry for up to a second if device is busy |
| 39 | - // or has a transient error. |
| 40 | - value = _ioAccess->read(sensorSysfsType, sensorSysfsNum, input, |
| 41 | + // For sensors with attribute ASYNC_READ_TIMEOUT, |
| 42 | + // spawn a thread with timeout |
| 43 | + auto asyncRead = |
| 44 | + env::getEnv("ASYNC_READ_TIMEOUT", sensorSetKey); |
| 45 | + if (!asyncRead.empty()) |
| 46 | + { |
| 47 | + // Default async read timeout |
| 48 | + std::chrono::milliseconds asyncReadTimeout{ |
| 49 | + std::stoi(asyncRead)}; |
| 50 | + bool valueIsValid = false; |
| 51 | + std::future<SensorValueType> asyncThread; |
| 52 | + |
| 53 | + auto asyncIter = _timedoutMap.find(sensorSetKey); |
| 54 | + if (asyncIter == _timedoutMap.end()) |
| 55 | + { |
| 56 | + // If sesnor not found in timedoutMap, spawn an async |
| 57 | + // thread |
| 58 | + asyncThread = std::async( |
| 59 | + std::launch::async, |
| 60 | + &hwmonio::HwmonIOInterface::read, _ioAccess, |
| 61 | + sensorSysfsType, sensorSysfsNum, input, |
| 62 | + hwmonio::retries, hwmonio::delay); |
| 63 | + valueIsValid = true; |
| 64 | + } |
| 65 | + else |
| 66 | + { |
| 67 | + // If we already have the async thread in the |
| 68 | + // timedoutMap, it means this sensor has already timed |
| 69 | + // out in the previous reads. No need to wait on |
| 70 | + // subsequent reads |
| 71 | + asyncReadTimeout = std::chrono::seconds(0); |
| 72 | + asyncThread = std::move(asyncIter->second); |
| 73 | + } |
| 74 | + |
| 75 | + std::future_status status = |
| 76 | + asyncThread.wait_for(asyncReadTimeout); |
| 77 | + switch (status) |
| 78 | + { |
| 79 | + // Read has finished |
| 80 | + case std::future_status::ready: |
| 81 | + // Read has finished |
| 82 | + if (valueIsValid) |
| 83 | + { |
| 84 | + value = asyncThread.get(); |
| 85 | + break; |
| 86 | + // Good sensor reads should skip the code below |
| 87 | + } |
| 88 | + // Async read thread has completed, erase from |
| 89 | + // timedoutMap to allow retry then throw |
| 90 | + _timedoutMap.erase(sensorSetKey); |
| 91 | + throw sensor::AsyncSensorReadTimeOut(); |
| 92 | + default: |
| 93 | + // Read timed out so add the thread to the |
| 94 | + // timedoutMap (if the entry already exists, |
| 95 | + // operator[] updates it) |
| 96 | + _timedoutMap[sensorSetKey] = std::move(asyncThread); |
| 97 | + throw sensor::AsyncSensorReadTimeOut(); |
| 98 | + } |
| 99 | + } |
| 100 | + else |
| 101 | + { |
| 102 | + // Retry for up to a second if device is busy |
| 103 | + // or has a transient error. |
| 104 | + value = |
| 105 | + _ioAccess->read(sensorSysfsType, sensorSysfsNum, input, |
| 106 | hwmonio::retries, hwmonio::delay); |
| 107 | + } |
| 108 | + |
| 109 | // Set functional property to true if we could read sensor |
| 110 | statusIface->functional(true); |
| 111 | |
| 112 | diff --git a/mainloop.hpp b/mainloop.hpp |
| 113 | index 6c5b8e0..cfe34d5 100644 |
| 114 | --- a/mainloop.hpp |
| 115 | +++ b/mainloop.hpp |
| 116 | @@ -9,6 +9,7 @@ |
| 117 | #include "types.hpp" |
| 118 | |
| 119 | #include <any> |
| 120 | +#include <future> |
| 121 | #include <memory> |
| 122 | #include <optional> |
| 123 | #include <sdbusplus/server.hpp> |
| 124 | @@ -116,6 +117,8 @@ class MainLoop |
| 125 | /** @brief Store the specifications of sensor objects */ |
| 126 | std::map<SensorSet::key_type, std::unique_ptr<sensor::Sensor>> |
| 127 | _sensorObjects; |
| 128 | + /** @brief Store the async futures of timed out sensor objects */ |
| 129 | + std::map<SensorSet::key_type, std::future<SensorValueType>> _timedoutMap; |
| 130 | |
| 131 | /** |
| 132 | * @brief Map of removed sensors |
| 133 | diff --git a/sensor.cpp b/sensor.cpp |
| 134 | index 93bbb03..7b681e1 100644 |
| 135 | --- a/sensor.cpp |
| 136 | +++ b/sensor.cpp |
| 137 | @@ -13,6 +13,7 @@ |
| 138 | #include <cmath> |
| 139 | #include <cstring> |
| 140 | #include <filesystem> |
| 141 | +#include <future> |
| 142 | #include <phosphor-logging/elog-errors.hpp> |
| 143 | #include <thread> |
| 144 | #include <xyz/openbmc_project/Common/error.hpp> |
| 145 | @@ -135,8 +136,9 @@ SensorValueType Sensor::adjustValue(SensorValueType value) |
| 146 | return value; |
| 147 | } |
| 148 | |
| 149 | -std::shared_ptr<ValueObject> Sensor::addValue(const RetryIO& retryIO, |
| 150 | - ObjectInfo& info) |
| 151 | +std::shared_ptr<ValueObject> Sensor::addValue( |
| 152 | + const RetryIO& retryIO, ObjectInfo& info, |
| 153 | + std::map<SensorSet::key_type, std::future<SensorValueType>>& timedoutMap) |
| 154 | { |
| 155 | static constexpr bool deferSignals = true; |
| 156 | |
| 157 | @@ -163,13 +165,69 @@ std::shared_ptr<ValueObject> Sensor::addValue(const RetryIO& retryIO, |
| 158 | // RAII object for GPIO unlock / lock |
| 159 | auto locker = gpioUnlock(getGpio()); |
| 160 | |
| 161 | - // Retry for up to a second if device is busy |
| 162 | - // or has a transient error. |
| 163 | - val = |
| 164 | - _ioAccess->read(_sensor.first, _sensor.second, |
| 165 | - hwmon::entry::cinput, std::get<size_t>(retryIO), |
| 166 | - std::get<std::chrono::milliseconds>(retryIO)); |
| 167 | - |
| 168 | + // For sensors with attribute ASYNC_READ_TIMEOUT, |
| 169 | + // spawn a thread with timeout |
| 170 | + auto asyncRead = env::getEnv("ASYNC_READ_TIMEOUT", _sensor); |
| 171 | + if (!asyncRead.empty()) |
| 172 | + { |
| 173 | + // Default async read timeout |
| 174 | + std::chrono::milliseconds asyncReadTimeout{ |
| 175 | + std::stoi(asyncRead)}; |
| 176 | + bool valueIsValid = false; |
| 177 | + std::future<SensorValueType> asyncThread; |
| 178 | + |
| 179 | + auto asyncIter = timedoutMap.find(_sensor); |
| 180 | + if (asyncIter == timedoutMap.end()) |
| 181 | + { |
| 182 | + // If sesnor not found in timedoutMap, spawn an async thread |
| 183 | + asyncThread = std::async( |
| 184 | + std::launch::async, &hwmonio::HwmonIOInterface::read, |
| 185 | + _ioAccess, _sensor.first, _sensor.second, |
| 186 | + hwmon::entry::cinput, std::get<size_t>(retryIO), |
| 187 | + std::get<std::chrono::milliseconds>(retryIO)); |
| 188 | + valueIsValid = true; |
| 189 | + } |
| 190 | + else |
| 191 | + { |
| 192 | + // If we already have the async thread in the timedoutMap, |
| 193 | + // it means this sensor has already timed out in the |
| 194 | + // previous reads. No need to wait on subsequent reads |
| 195 | + asyncReadTimeout = std::chrono::seconds(0); |
| 196 | + asyncThread = std::move(asyncIter->second); |
| 197 | + } |
| 198 | + |
| 199 | + std::future_status status = |
| 200 | + asyncThread.wait_for(asyncReadTimeout); |
| 201 | + switch (status) |
| 202 | + { |
| 203 | + case std::future_status::ready: |
| 204 | + // Read has finished |
| 205 | + if (valueIsValid) |
| 206 | + { |
| 207 | + val = asyncThread.get(); |
| 208 | + break; |
| 209 | + // Good sensor reads should skip the code below |
| 210 | + } |
| 211 | + // Async read thread has completed, erase from |
| 212 | + // timedoutMap to allow retry then throw |
| 213 | + timedoutMap.erase(_sensor); |
| 214 | + throw AsyncSensorReadTimeOut(); |
| 215 | + default: |
| 216 | + // Read timed out so add the thread to the timedoutMap |
| 217 | + // (if the entry already exists, operator[] updates it) |
| 218 | + timedoutMap[_sensor] = std::move(asyncThread); |
| 219 | + throw AsyncSensorReadTimeOut(); |
| 220 | + } |
| 221 | + } |
| 222 | + else |
| 223 | + { |
| 224 | + // Retry for up to a second if device is busy |
| 225 | + // or has a transient error. |
| 226 | + val = _ioAccess->read( |
| 227 | + _sensor.first, _sensor.second, hwmon::entry::cinput, |
| 228 | + std::get<size_t>(retryIO), |
| 229 | + std::get<std::chrono::milliseconds>(retryIO)); |
| 230 | + } |
| 231 | val = adjustValue(val); |
| 232 | } |
| 233 | #ifdef UPDATE_FUNCTIONAL_ON_FAIL |
| 234 | diff --git a/sensor.hpp b/sensor.hpp |
| 235 | index 369a252..d1d6730 100644 |
| 236 | --- a/sensor.hpp |
| 237 | +++ b/sensor.hpp |
| 238 | @@ -4,6 +4,8 @@ |
| 239 | #include "sensorset.hpp" |
| 240 | #include "types.hpp" |
| 241 | |
| 242 | +#include <cerrno> |
| 243 | +#include <future> |
| 244 | #include <gpioplus/handle.hpp> |
| 245 | #include <memory> |
| 246 | #include <optional> |
| 247 | @@ -20,6 +22,17 @@ struct valueAdjust |
| 248 | std::unordered_set<int> rmRCs; |
| 249 | }; |
| 250 | |
| 251 | +/** @brief Custom exception for async sensor reading timeout |
| 252 | + */ |
| 253 | +struct AsyncSensorReadTimeOut : public std::system_error |
| 254 | +{ |
| 255 | + AsyncSensorReadTimeOut() : |
| 256 | + system_error(std::error_code(ETIMEDOUT, std::system_category()), |
| 257 | + "Async sensor read timed out") |
| 258 | + { |
| 259 | + } |
| 260 | +}; |
| 261 | + |
| 262 | /** @class Sensor |
| 263 | * @brief Sensor object based on a SensorSet container's key type |
| 264 | * @details Sensor object to create and modify an associated device's sensor |
| 265 | @@ -87,10 +100,14 @@ class Sensor |
| 266 | * (number of and delay between) |
| 267 | * @param[in] info - Sensor object information |
| 268 | * |
| 269 | + * @param[in] timedoutMap - Map to track timed out threads |
| 270 | + * |
| 271 | * @return - Shared pointer to the value object |
| 272 | */ |
| 273 | - std::shared_ptr<ValueObject> addValue(const RetryIO& retryIO, |
| 274 | - ObjectInfo& info); |
| 275 | + std::shared_ptr<ValueObject> |
| 276 | + addValue(const RetryIO& retryIO, ObjectInfo& info, |
| 277 | + std::map<SensorSet::key_type, std::future<SensorValueType>>& |
| 278 | + timedoutMap); |
| 279 | |
| 280 | /** |
| 281 | * @brief Add status interface and functional property for sensor |
| 282 | -- |
| 283 | 2.7.4 |
| 284 | |