Improve cpusensor performance
The cpusensor service is still making some redundant accesses for
reading hwmon attributes on every scanning. Actually, threshold
update is needed only when CPU package's Tcontrol target is
changed so this commit make it do that only when Tcontrol value
is changed.
Tested:
Compare cpu resource consumption of cpusensor service using top.
It should eat less cpu resources than before.
Change-Id: Ieb582996fc5c9c07abbfc7ac0d1b37f593269d00
Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
diff --git a/src/CPUSensor.cpp b/src/CPUSensor.cpp
index d8d0ecb..a6dc275 100644
--- a/src/CPUSensor.cpp
+++ b/src/CPUSensor.cpp
@@ -27,39 +27,42 @@
#include <sdbusplus/asio/object_server.hpp>
#include <string>
-static constexpr size_t warnAfterErrorCount = 10;
-static constexpr double maxReading = 127;
-static constexpr double minReading = -128;
-
CPUSensor::CPUSensor(const std::string& path, const std::string& objectType,
sdbusplus::asio::object_server& objectServer,
std::shared_ptr<sdbusplus::asio::connection>& conn,
boost::asio::io_service& io, const std::string& sensorName,
std::vector<thresholds::Threshold>&& _thresholds,
- const std::string& sensorConfiguration) :
+ const std::string& sensorConfiguration, int cpuId,
+ bool show) :
Sensor(boost::replace_all_copy(sensorName, " ", "_"), path,
std::move(_thresholds), sensorConfiguration, objectType, maxReading,
minReading),
objServer(objectServer), inputDev(io, open(path.c_str(), O_RDONLY)),
- waitTimer(io), errCount(0)
-
+ waitTimer(io), show(show),
+ privTcontrol(std::numeric_limits<double>::quiet_NaN()), errCount(0)
{
- sensorInterface = objectServer.add_interface(
- "/xyz/openbmc_project/sensors/temperature/" + name,
- "xyz.openbmc_project.Sensor.Value");
- if (thresholds::hasWarningInterface(thresholds))
+ nameTcontrol = labelTcontrol;
+ nameTcontrol += " CPU" + std::to_string(cpuId);
+
+ if (show)
{
- thresholdInterfaceWarning = objectServer.add_interface(
+ sensorInterface = objectServer.add_interface(
"/xyz/openbmc_project/sensors/temperature/" + name,
- "xyz.openbmc_project.Sensor.Threshold.Warning");
+ "xyz.openbmc_project.Sensor.Value");
+ if (thresholds::hasWarningInterface(thresholds))
+ {
+ thresholdInterfaceWarning = objectServer.add_interface(
+ "/xyz/openbmc_project/sensors/temperature/" + name,
+ "xyz.openbmc_project.Sensor.Threshold.Warning");
+ }
+ if (thresholds::hasCriticalInterface(thresholds))
+ {
+ thresholdInterfaceCritical = objectServer.add_interface(
+ "/xyz/openbmc_project/sensors/temperature/" + name,
+ "xyz.openbmc_project.Sensor.Threshold.Critical");
+ }
+ setInitialProperties(conn);
}
- if (thresholds::hasCriticalInterface(thresholds))
- {
- thresholdInterfaceCritical = objectServer.add_interface(
- "/xyz/openbmc_project/sensors/temperature/" + name,
- "xyz.openbmc_project.Sensor.Threshold.Critical");
- }
- setInitialProperties(conn);
setupPowerMatch(conn);
setupRead();
}
@@ -69,9 +72,12 @@
// close the input dev to cancel async operations
inputDev.close();
waitTimer.cancel();
- objServer.remove_interface(thresholdInterfaceWarning);
- objServer.remove_interface(thresholdInterfaceCritical);
- objServer.remove_interface(sensorInterface);
+ if (show)
+ {
+ objServer.remove_interface(thresholdInterfaceWarning);
+ objServer.remove_interface(thresholdInterfaceCritical);
+ objServer.remove_interface(sensorInterface);
+ }
}
void CPUSensor::setupRead(void)
@@ -96,7 +102,7 @@
try
{
std::getline(responseStream, response);
- float nvalue = std::stof(response);
+ double nvalue = std::stof(response);
responseStream.clear();
nvalue /= CPUSensor::sensorScaleFactor;
if (overridenState)
@@ -105,25 +111,44 @@
}
if (nvalue != value)
{
- updateValue(nvalue);
- }
- if (!thresholds.empty())
- {
- std::vector<thresholds::Threshold> newThresholds;
- if (parseThresholdsFromAttr(newThresholds, path,
- CPUSensor::sensorScaleFactor))
+ if (show)
{
- if (!std::equal(thresholds.begin(), thresholds.end(),
- newThresholds.begin(), newThresholds.end()))
- {
- thresholds = newThresholds;
- thresholds::updateThresholds(this);
- }
+ updateValue(nvalue);
}
else
{
- std::cerr << "Failure to update thresholds for " << name
- << "\n";
+ value = nvalue;
+ }
+ }
+ double gTcontrol = gCpuSensors[nameTcontrol]
+ ? gCpuSensors[nameTcontrol]->value
+ : std::numeric_limits<double>::quiet_NaN();
+ if (gTcontrol != privTcontrol)
+ {
+ privTcontrol = gTcontrol;
+
+ if (!thresholds.empty())
+ {
+ std::vector<thresholds::Threshold> newThresholds;
+ if (parseThresholdsFromAttr(newThresholds, path,
+ CPUSensor::sensorScaleFactor))
+ {
+ if (!std::equal(thresholds.begin(), thresholds.end(),
+ newThresholds.begin(),
+ newThresholds.end()))
+ {
+ thresholds = newThresholds;
+ if (show)
+ {
+ thresholds::updateThresholds(this);
+ }
+ }
+ }
+ else
+ {
+ std::cerr << "Failure to update thresholds for " << name
+ << "\n";
+ }
}
}
errCount = 0;
@@ -150,13 +175,27 @@
std::cerr << "Failure to read sensor " << name << " at " << path
<< "\n";
}
- updateValue(0);
+ if (show)
+ {
+ updateValue(0);
+ }
+ else
+ {
+ value = 0;
+ }
errCount++;
}
else
{
errCount = 0; // check power again in 10 cycles
- updateValue(std::numeric_limits<double>::quiet_NaN());
+ if (show)
+ {
+ updateValue(std::numeric_limits<double>::quiet_NaN());
+ }
+ else
+ {
+ value = std::numeric_limits<double>::quiet_NaN();
+ }
}
}
@@ -180,5 +219,8 @@
void CPUSensor::checkThresholds(void)
{
- thresholds::checkThresholds(this);
+ if (show)
+ {
+ thresholds::checkThresholds(this);
+ }
}
diff --git a/src/CPUSensorMain.cpp b/src/CPUSensorMain.cpp
index 6c4e2e5..858ab24 100644
--- a/src/CPUSensorMain.cpp
+++ b/src/CPUSensorMain.cpp
@@ -41,6 +41,8 @@
static constexpr bool DEBUG = false;
+boost::container::flat_map<std::string, std::unique_ptr<CPUSensor>> gCpuSensors;
+
enum State
{
OFF, // host powered down
@@ -76,26 +78,22 @@
"xyz.openbmc_project.Configuration.";
static constexpr std::array<const char*, 3> sensorTypes = {
"SkylakeCPU", "BroadwellCPU", "HaswellCPU"};
-static constexpr std::array<const char*, 3> skipProps = {"Tcontrol",
- "Tthrottle", "Tjmax"};
+static constexpr std::array<const char*, 3> hiddenProps = {
+ CPUSensor::labelTcontrol, "Tthrottle", "Tjmax"};
void detectCpuAsync(
boost::asio::deadline_timer& pingTimer,
boost::asio::deadline_timer& creationTimer, boost::asio::io_service& io,
sdbusplus::asio::object_server& objectServer,
std::shared_ptr<sdbusplus::asio::connection>& dbusConnection,
- boost::container::flat_map<std::string, std::unique_ptr<CPUSensor>>&
- cpuSensors,
boost::container::flat_set<CPUConfig>& cpuConfigs,
ManagedObjectType& sensorConfigs);
-bool createSensors(
- boost::asio::io_service& io, sdbusplus::asio::object_server& objectServer,
- std::shared_ptr<sdbusplus::asio::connection>& dbusConnection,
- boost::container::flat_map<std::string, std::unique_ptr<CPUSensor>>&
- cpuSensors,
- boost::container::flat_set<CPUConfig>& cpuConfigs,
- ManagedObjectType& sensorConfigs)
+bool createSensors(boost::asio::io_service& io,
+ sdbusplus::asio::object_server& objectServer,
+ std::shared_ptr<sdbusplus::asio::connection>& dbusConnection,
+ boost::container::flat_set<CPUConfig>& cpuConfigs,
+ ManagedObjectType& sensorConfigs)
{
bool available = false;
for (const CPUConfig& cpu : cpuConfigs)
@@ -269,25 +267,10 @@
std::getline(labelFile, label);
labelFile.close();
- // skip non-sensor properties
- bool skipIt = false;
- for (const char* prop : skipProps)
- {
- if (label == prop)
- {
- skipIt = true;
- break;
- }
- }
- if (skipIt)
- {
- continue;
- }
-
std::string sensorName = label + " CPU" + std::to_string(cpuId);
- auto findSensor = cpuSensors.find(sensorName);
- if (findSensor != cpuSensors.end())
+ auto findSensor = gCpuSensors.find(sensorName);
+ if (findSensor != gCpuSensors.end())
{
if (DEBUG)
{
@@ -297,6 +280,17 @@
continue;
}
+ // check hidden properties
+ bool show = true;
+ for (const char* prop : hiddenProps)
+ {
+ if (label == prop)
+ {
+ show = false;
+ break;
+ }
+ }
+
std::vector<thresholds::Threshold> sensorThresholds;
std::string labelHead = label.substr(0, label.find(" "));
parseThresholdsFromConfig(*sensorData, sensorThresholds,
@@ -310,9 +304,10 @@
<< sensorName << "\n";
}
}
- cpuSensors[sensorName] = std::make_unique<CPUSensor>(
+ gCpuSensors[sensorName] = std::make_unique<CPUSensor>(
inputPathStr, sensorType, objectServer, dbusConnection, io,
- sensorName, std::move(sensorThresholds), *interfacePath);
+ sensorName, std::move(sensorThresholds), *interfacePath, cpuId,
+ show);
createdSensors.insert(sensorName);
if (DEBUG)
{
@@ -375,15 +370,13 @@
std::cout << parameters << " on bus " << busStr << " is exported\n";
}
-void detectCpu(
- boost::asio::deadline_timer& pingTimer,
- boost::asio::deadline_timer& creationTimer, boost::asio::io_service& io,
- sdbusplus::asio::object_server& objectServer,
- std::shared_ptr<sdbusplus::asio::connection>& dbusConnection,
- boost::container::flat_map<std::string, std::unique_ptr<CPUSensor>>&
- cpuSensors,
- boost::container::flat_set<CPUConfig>& cpuConfigs,
- ManagedObjectType& sensorConfigs)
+void detectCpu(boost::asio::deadline_timer& pingTimer,
+ boost::asio::deadline_timer& creationTimer,
+ boost::asio::io_service& io,
+ sdbusplus::asio::object_server& objectServer,
+ std::shared_ptr<sdbusplus::asio::connection>& dbusConnection,
+ boost::container::flat_set<CPUConfig>& cpuConfigs,
+ ManagedObjectType& sensorConfigs)
{
size_t rescanDelaySeconds = 0;
bool keepPinging = false;
@@ -490,12 +483,11 @@
return; // we're being canceled
}
- if (!createSensors(io, objectServer, dbusConnection, cpuSensors,
- cpuConfigs, sensorConfigs))
+ if (!createSensors(io, objectServer, dbusConnection, cpuConfigs,
+ sensorConfigs))
{
detectCpuAsync(pingTimer, creationTimer, io, objectServer,
- dbusConnection, cpuSensors, cpuConfigs,
- sensorConfigs);
+ dbusConnection, cpuConfigs, sensorConfigs);
}
});
}
@@ -503,7 +495,7 @@
if (keepPinging)
{
detectCpuAsync(pingTimer, creationTimer, io, objectServer,
- dbusConnection, cpuSensors, cpuConfigs, sensorConfigs);
+ dbusConnection, cpuConfigs, sensorConfigs);
}
}
@@ -512,8 +504,6 @@
boost::asio::deadline_timer& creationTimer, boost::asio::io_service& io,
sdbusplus::asio::object_server& objectServer,
std::shared_ptr<sdbusplus::asio::connection>& dbusConnection,
- boost::container::flat_map<std::string, std::unique_ptr<CPUSensor>>&
- cpuSensors,
boost::container::flat_set<CPUConfig>& cpuConfigs,
ManagedObjectType& sensorConfigs)
{
@@ -525,7 +515,7 @@
}
detectCpu(pingTimer, creationTimer, io, objectServer, dbusConnection,
- cpuSensors, cpuConfigs, sensorConfigs);
+ cpuConfigs, sensorConfigs);
});
}
@@ -623,8 +613,6 @@
systemBus->request_name("xyz.openbmc_project.CPUSensor");
sdbusplus::asio::object_server objectServer(systemBus);
- boost::container::flat_map<std::string, std::unique_ptr<CPUSensor>>
- cpuSensors;
std::vector<std::unique_ptr<sdbusplus::bus::match::match>> matches;
boost::asio::deadline_timer pingTimer(io);
boost::asio::deadline_timer creationTimer(io);
@@ -641,7 +629,7 @@
if (getCpuConfig(systemBus, cpuConfigs, sensorConfigs))
{
detectCpuAsync(pingTimer, creationTimer, io, objectServer,
- systemBus, cpuSensors, cpuConfigs, sensorConfigs);
+ systemBus, cpuConfigs, sensorConfigs);
}
});
@@ -669,8 +657,7 @@
if (getCpuConfig(systemBus, cpuConfigs, sensorConfigs))
{
detectCpuAsync(pingTimer, creationTimer, io, objectServer,
- systemBus, cpuSensors, cpuConfigs,
- sensorConfigs);
+ systemBus, cpuConfigs, sensorConfigs);
}
});
};