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/include/PSUEvent.hpp b/include/PSUEvent.hpp
index 93c06c1..2ee7a33 100644
--- a/include/PSUEvent.hpp
+++ b/include/PSUEvent.hpp
@@ -19,10 +19,11 @@
#include <Utils.hpp>
#include <boost/asio/deadline_timer.hpp>
#include <boost/asio/io_service.hpp>
-#include <boost/asio/streambuf.hpp>
+#include <boost/asio/random_access_file.hpp>
#include <boost/container/flat_map.hpp>
#include <sdbusplus/asio/object_server.hpp>
+#include <array>
#include <memory>
#include <set>
#include <string>
@@ -57,11 +58,12 @@
PowerState readState;
boost::asio::deadline_timer waitTimer;
- std::shared_ptr<boost::asio::streambuf> readBuf;
+ std::shared_ptr<std::array<char, 128>> buffer;
void restartRead();
- void handleResponse(const boost::system::error_code& err);
+ void handleResponse(const boost::system::error_code& err,
+ size_t bytesTransferred);
void updateValue(const int& newValue);
- boost::asio::posix::stream_descriptor inputDev;
+ boost::asio::random_access_file inputDev;
std::string psuName;
std::string groupEventName;
std::string fanName;
diff --git a/include/PSUSensor.hpp b/include/PSUSensor.hpp
index 9e32138..4d7aaba 100644
--- a/include/PSUSensor.hpp
+++ b/include/PSUSensor.hpp
@@ -2,10 +2,11 @@
#include <PwmSensor.hpp>
#include <Thresholds.hpp>
-#include <boost/asio/streambuf.hpp>
+#include <boost/asio/random_access_file.hpp>
#include <sdbusplus/asio/object_server.hpp>
#include <sensor.hpp>
+#include <array>
#include <memory>
#include <string>
#include <utility>
@@ -26,15 +27,19 @@
void setupRead(void);
private:
+ // Note, this buffer is a shared_ptr because during a read, its lifetime
+ // might have to outlive the PSUSensor class if the object gets destroyed
+ // while in the middle of a read operation
+ std::shared_ptr<std::array<char, 128>> buffer;
sdbusplus::asio::object_server& objServer;
- boost::asio::posix::stream_descriptor inputDev;
+ boost::asio::random_access_file inputDev;
boost::asio::deadline_timer waitTimer;
std::string path;
unsigned int sensorFactor;
double sensorOffset;
thresholds::ThresholdTimer thresholdTimer;
void restartRead();
- void handleResponse(const boost::system::error_code& err);
+ void handleResponse(const boost::system::error_code& err, size_t bytesRead);
void checkThresholds(void) override;
unsigned int sensorPollMs = defaultSensorPollMs;
diff --git a/meson.build b/meson.build
index 48c7c27..8b29e64 100644
--- a/meson.build
+++ b/meson.build
@@ -11,6 +11,16 @@
meson_version: '>=0.57.0',
)
+# Note, there is currently an issue with CPUSensor when used in conjunction
+# with io_uring. For the moment, we enable uring for all other daemons, but
+# we'd like to enable it for all daemons.
+# https://github.com/openbmc/dbus-sensors/issues/19
+uring_args = [
+ '-DBOOST_ASIO_HAS_IO_URING',
+ '-DBOOST_ASIO_DISABLE_EPOLL',
+ '-DBOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT',
+]
+
add_project_arguments(
'-Wno-psabi',
'-Wuninitialized',
@@ -34,7 +44,13 @@
# i2c-tools doesn't ship a pkg-config file for libi2c
i2c = meson.get_compiler('cpp').find_library('i2c')
-sdbusplus = dependency('sdbusplus')
+sdbusplus = dependency('sdbusplus', required : false, include_type: 'system')
+if not sdbusplus.found()
+ sdbusplus_proj = subproject('sdbusplus', required: true)
+ sdbusplus = sdbusplus_proj.get_variable('sdbusplus_dep')
+ sdbusplus = sdbusplus.as_system('system')
+endif
+
phosphor_logging_dep = dependency('phosphor-logging')
if cpp.has_header('nlohmann/json.hpp')
@@ -49,10 +65,27 @@
pkgconfig_define: ['prefix', get_option('prefix')])
threads = dependency('threads')
+boost = dependency('boost',version : '>=1.79.0', required : false, include_type: 'system')
+if not boost.found()
+ subproject('boost', required: false)
+ boost_inc = include_directories('subprojects/boost_1_79_0/', is_system:true)
+ boost = declare_dependency(include_directories : boost_inc)
+ boost = boost.as_system('system')
+endif
+
+uring = dependency('liburing', required : false, include_type: 'system')
+if not uring.found()
+ uring_proj = subproject('liburing', required: true)
+ uring = uring_proj.get_variable('uring')
+ uring = uring.as_system('system')
+endif
+
default_deps = [
+ boost,
nlohmann_json,
phosphor_logging_dep,
sdbusplus,
+ uring,
]
thresholds_a = static_library(
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,
diff --git a/subprojects/boost.wrap b/subprojects/boost.wrap
new file mode 100644
index 0000000..008548f
--- /dev/null
+++ b/subprojects/boost.wrap
@@ -0,0 +1,4 @@
+[wrap-file]
+source_url = https://downloads.yoctoproject.org/mirror/sources/boost_1_79_0.tar.bz2
+source_hash = 475d589d51a7f8b3ba2ba4eda022b170e562ca3b760ee922c146b6c65856ef39
+source_filename = 1_79_0.tar.bz2
diff --git a/subprojects/liburing.wrap b/subprojects/liburing.wrap
new file mode 100644
index 0000000..d9225d5
--- /dev/null
+++ b/subprojects/liburing.wrap
@@ -0,0 +1,13 @@
+[wrap-file]
+directory = liburing-liburing-2.2
+source_url = https://github.com/axboe/liburing/archive/refs/tags/liburing-2.2.tar.gz
+source_filename = liburing-2.2.tar.gz
+source_hash = e092624af6aa244ade2d52181cc07751ac5caba2f3d63e9240790db9ed130bbc
+patch_filename = liburing_2.2-1_patch.zip
+patch_url = https://wrapdb.mesonbuild.com/v2/liburing_2.2-1/get_patch
+patch_hash = 43fed02db27c38647500e859e3a42bf91585e7fbea37999bf50337cc46c7de26
+wrapdb_version = 2.2-1
+
+[provide]
+uring = uring
+
diff --git a/tests/meson.build b/tests/meson.build
index e20eccf..51d5d60 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -19,6 +19,12 @@
endif
endif
+ut_deps_list = [
+ gtest_dep,
+]
+
+ut_deps_list += default_deps
+
have_boost_dbus = meson.get_compiler('cpp').has_header('dbus/connection.hpp')
if have_boost_dbus
test(
@@ -26,10 +32,7 @@
executable(
'test_hwmon_temp_sensor',
'test_HwmonTempSensor.cpp',
- dependencies: [
- gtest,
- boost_dbus,
- ],
+ dependencies: ut_deps_list,
implicit_include_directories: false,
include_directories: '../include',
)
@@ -40,10 +43,7 @@
executable(
'test_tach_sensor',
'test_TachSensor.cpp',
- dependencies: [
- gtest,
- boost_dbus,
- ],
+ dependencies: ut_deps_list,
implicit_include_directories: false,
include_directories: '../include',
)
@@ -56,10 +56,7 @@
'test_utils',
'test_Utils.cpp',
'../src/Utils.cpp',
- dependencies: [
- sdbusplus,
- gtest_dep,
- ],
+ dependencies: ut_deps_list,
implicit_include_directories: false,
include_directories: '../include',
)