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