Enable an io-uring build
There exists a bug where calls that we previously thought were
non-blocking, actually do block when used with hwmon or filesystem fds.
This causes high latencies on the dbus interfaces when lots of sensors
are used in a single daemon, as is the case in PSUSensor.
This patchset moves the PSUSensor code over to using io-uring, through
boost asio, using the random_access_file class. This helps with
performance in a number of ways, the largest of which being that sensor
reads are no longer blocking.
Tested:
Booted the sensor system on Tyan S7106; dbus-monitor shows sensors
scanning normally.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I654eafcfab5a24b65b89c204ab43d81c7ae064cf
diff --git a/src/PSUEvent.cpp b/src/PSUEvent.cpp
index d1cb40e..883c871 100644
--- a/src/PSUEvent.cpp
+++ b/src/PSUEvent.cpp
@@ -149,23 +149,17 @@
eventInterface(std::move(eventInterface)),
asserts(std::move(asserts)), combineEvent(std::move(combineEvent)),
assertState(std::move(state)), path(path), eventName(eventName),
- readState(powerState), waitTimer(io), inputDev(io), psuName(psuName),
- groupEventName(groupEventName), systemBus(conn)
+ readState(powerState), waitTimer(io),
+
+ inputDev(io, path, boost::asio::random_access_file::read_only),
+ psuName(psuName), groupEventName(groupEventName), systemBus(conn)
{
+ buffer = std::make_shared<std::array<char, 128>>();
if (pollRate > 0.0)
{
eventPollMs = static_cast<unsigned int>(pollRate * 1000);
}
- // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg)
- fd = open(path.c_str(), O_RDONLY);
- if (fd < 0)
- {
- std::cerr << "PSU sub event failed to open file\n";
- return;
- }
- inputDev.assign(fd);
-
auto found = logID.find(eventName);
if (found == logID.end())
{
@@ -205,19 +199,21 @@
restartRead();
return;
}
+ if (!buffer)
+ {
+ std::cerr << "Buffer was invalid?";
+ return;
+ }
- std::shared_ptr<boost::asio::streambuf> buffer =
- std::make_shared<boost::asio::streambuf>();
std::weak_ptr<PSUSubEvent> weakRef = weak_from_this();
- boost::asio::async_read_until(
- inputDev, *buffer, '\n',
- [weakRef, buffer](const boost::system::error_code& ec,
- std::size_t /*bytes_transfered*/) {
+ inputDev.async_read_some_at(
+ 0, boost::asio::buffer(buffer->data(), buffer->size() - 1),
+ [weakRef, buffer{buffer}](const boost::system::error_code& ec,
+ std::size_t bytesTransferred) {
std::shared_ptr<PSUSubEvent> self = weakRef.lock();
if (self)
{
- self->readBuf = buffer;
- self->handleResponse(ec);
+ self->handleResponse(ec, bytesTransferred);
}
});
}
@@ -239,23 +235,34 @@
});
}
-void PSUSubEvent::handleResponse(const boost::system::error_code& err)
+void PSUSubEvent::handleResponse(const boost::system::error_code& err,
+ size_t bytesTransferred)
{
+ if (err == boost::asio::error::operation_aborted)
+ {
+ return;
+ }
+
if ((err == boost::system::errc::bad_file_descriptor) ||
(err == boost::asio::error::misc_errors::not_found))
{
return;
}
- std::istream responseStream(readBuf.get());
+ if (!buffer)
+ {
+ std::cerr << "Buffer was invalid?";
+ return;
+ }
+ // null terminate the string so we don't walk off the end
+ std::array<char, 128>& bufferRef = *buffer;
+ bufferRef[bytesTransferred] = '\0';
+
if (!err)
{
std::string response;
try
{
- std::getline(responseStream, response);
- int nvalue = std::stoi(response);
- responseStream.clear();
-
+ int nvalue = std::stoi(bufferRef.data());
updateValue(nvalue);
errCount = 0;
}
diff --git a/src/PSUSensor.cpp b/src/PSUSensor.cpp
index bf75e61..af63805 100644
--- a/src/PSUSensor.cpp
+++ b/src/PSUSensor.cpp
@@ -18,6 +18,7 @@
#include <PSUSensor.hpp>
#include <boost/algorithm/string/predicate.hpp>
+#include <boost/asio/random_access_file.hpp>
#include <boost/asio/read_until.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <sdbusplus/asio/connection.hpp>
@@ -47,9 +48,12 @@
const std::string& label, size_t tSize, double pollRate) :
Sensor(escapeName(sensorName), std::move(thresholdsIn), sensorConfiguration,
objectType, false, false, max, min, conn, powerState),
- objServer(objectServer), inputDev(io), waitTimer(io), path(path),
- sensorFactor(factor), sensorOffset(offset), thresholdTimer(io)
+ objServer(objectServer),
+ inputDev(io, path, boost::asio::random_access_file::read_only),
+ waitTimer(io), path(path), sensorFactor(factor), sensorOffset(offset),
+ thresholdTimer(io)
{
+ buffer = std::make_shared<std::array<char, 128>>();
std::string unitPath = sensor_paths::getPathForUnits(sensorUnits);
if constexpr (debug)
{
@@ -64,15 +68,6 @@
sensorPollMs = static_cast<unsigned int>(pollRate * 1000);
}
- // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg)
- fd = open(path.c_str(), O_RDONLY | O_NONBLOCK);
- if (fd < 0)
- {
- std::cerr << "PSU sensor failed to open file\n";
- return;
- }
- inputDev.assign(fd);
-
std::string dbusPath = sensorPathPrefix + unitPath + "/" + name;
sensorInterface = objectServer.add_interface(
@@ -124,15 +119,29 @@
return;
}
- std::weak_ptr<PSUSensor> weakRef = weak_from_this();
- inputDev.async_wait(boost::asio::posix::descriptor_base::wait_read,
- [weakRef](const boost::system::error_code& ec) {
- std::shared_ptr<PSUSensor> self = weakRef.lock();
- if (self)
+ if (buffer == nullptr)
+ {
+ std::cerr << "Buffer was invalid?";
+ return;
+ }
+
+ std::weak_ptr<PSUSensor> weak = weak_from_this();
+ // Note, we are building a asio buffer that is one char smaller than
+ // the actual data structure, so that we can always append the null
+ // terminator. This can go away once std::from_chars<double> is available
+ // in the standard
+ inputDev.async_read_some_at(
+ 0, boost::asio::buffer(buffer->data(), buffer->size() - 1),
+ [weak, buffer{buffer}](const boost::system::error_code& ec,
+ size_t bytesRead) {
+ std::shared_ptr<PSUSensor> self = weak.lock();
+ if (!self)
{
- self->handleResponse(ec);
+ return;
}
- });
+
+ self->handleResponse(ec, bytesRead);
+ });
}
void PSUSensor::restartRead(void)
@@ -155,40 +164,32 @@
// Create a buffer expected to be able to hold more characters than will be
// present in the input file.
-static constexpr uint32_t psuBufLen = 128;
-void PSUSensor::handleResponse(const boost::system::error_code& err)
+void PSUSensor::handleResponse(const boost::system::error_code& err,
+ size_t bytesRead)
{
+ if (err == boost::asio::error::operation_aborted)
+ {
+ std::cerr << "Read aborted\n";
+ return;
+ }
if ((err == boost::system::errc::bad_file_descriptor) ||
(err == boost::asio::error::misc_errors::not_found))
{
std::cerr << "Bad file descriptor for " << path << "\n";
return;
}
+ // null terminate the string so we don't walk off the end
+ std::array<char, 128>& bufferRef = *buffer;
+ bufferRef[bytesRead] = '\0';
- std::string buffer;
- buffer.resize(psuBufLen);
- lseek(fd, 0, SEEK_SET);
- int rdLen = read(fd, buffer.data(), psuBufLen);
-
- if (rdLen > 0)
+ try
{
- try
- {
- rawValue = std::stod(buffer);
- updateValue((rawValue / sensorFactor) + sensorOffset);
- }
- catch (const std::invalid_argument&)
- {
- std::cerr << "Could not parse input from " << path << "\n";
- incrementError();
- }
+ rawValue = std::stod(bufferRef.data());
+ updateValue((rawValue / sensorFactor) + sensorOffset);
}
- else
+ catch (const std::invalid_argument&)
{
- std::cerr
- << "System error " << errno << " ("
- << std::generic_category().default_error_condition(errno).message()
- << ") reading from " << path << ", line: " << __LINE__ << "\n";
+ std::cerr << "Could not parse input from " << path << "\n";
incrementError();
}
diff --git a/src/meson.build b/src/meson.build
index 0fe5f56..4c3d61c 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -14,6 +14,7 @@
thresholds_dep,
utils_dep,
],
+ cpp_args: uring_args,
implicit_include_directories: false,
include_directories: '../include',
install: true,
@@ -48,6 +49,7 @@
thresholds_dep,
utils_dep,
],
+ cpp_args: uring_args,
implicit_include_directories: false,
include_directories: '../include',
install: true,
@@ -66,6 +68,7 @@
thresholds_dep,
utils_dep,
],
+ cpp_args: uring_args,
implicit_include_directories: false,
include_directories: '../include',
install: true,
@@ -82,6 +85,7 @@
thresholds_dep,
utils_dep,
],
+ cpp_args: uring_args,
implicit_include_directories: false,
include_directories: '../include',
install: true,
@@ -99,6 +103,7 @@
i2c,
utils_dep,
],
+ cpp_args: uring_args,
implicit_include_directories: false,
include_directories: '../include',
install: true,
@@ -114,6 +119,7 @@
thresholds_dep,
utils_dep,
],
+ cpp_args: uring_args,
implicit_include_directories: false,
include_directories: '../include',
install: true,
@@ -130,6 +136,7 @@
thresholds_dep,
utils_dep,
],
+ cpp_args: uring_args,
implicit_include_directories: false,
include_directories: '../include',
install: true,
@@ -146,6 +153,7 @@
'nvmesensor',
sources: nvme_srcs,
dependencies: nvme_deps,
+ cpp_args: uring_args,
implicit_include_directories: false,
include_directories: '../include',
install: true,
@@ -164,6 +172,7 @@
thresholds_dep,
utils_dep,
],
+ cpp_args: uring_args,
implicit_include_directories: false,
include_directories: '../include',
install: true,
@@ -180,6 +189,7 @@
thresholds_dep,
utils_dep,
],
+ cpp_args: uring_args,
implicit_include_directories: false,
include_directories: '../include',
install: true,