pseq: Add presence caching to Services
Add caching of hardware presence data to the Services class in the
phosphor-power-sequencer application. Obtaining hardware presence from
D-Bus is a slow operation. During pgood fault isolation, multiple rails
may need the same hardware presence information. Cache this information
to improve performance.
Provide a method to clear the cached data since it is only valid for a
short period of time. For example, power supplies are hot-pluggable, so
their presence may change while a system is powered on.
Tested:
* BMCServices
* createPMBus()
* Verify correct sysfs path passed to PMBus constructor
* isPresent()
* Test where hardware is present
* Test where value is not cached
* Verify value is cached after data obtained from D-Bus
* Test where value is cached
* Test where hardware is not present
* Test where value is not cached
* Verify value is cached after data obtained from D-Bus
* Test where value is cached
* Test where D-Bus method call fails with an expected exception
* Verify false is stored in cache
* Verify false is returned
* Test where D-Bus method call fails with an unexpected exception
* Verify nothing is cached
* Verify exception is re-thrown
* clearCache()
* Verify clears all cached data
* MockServices
* Verify all methods can be called from an automated test
* Run existing automated tests
Change-Id: I3e92be3ded1ed333acbedc970409176cabe98c09
Signed-off-by: Shawn McCarney <shawnmm@us.ibm.com>
diff --git a/phosphor-power-sequencer/src/meson.build b/phosphor-power-sequencer/src/meson.build
index db3ac4a..c1fa67c 100644
--- a/phosphor-power-sequencer/src/meson.build
+++ b/phosphor-power-sequencer/src/meson.build
@@ -6,6 +6,7 @@
phosphor_power_sequencer_library = static_library(
'phosphor-power-sequencer',
'config_file_parser.cpp',
+ 'services.cpp',
implicit_include_directories: false,
dependencies: [
nlohmann_json_dep
@@ -21,7 +22,6 @@
'power_control.cpp',
'power_interface.cpp',
'power_sequencer_monitor.cpp',
- 'services.cpp',
'ucd90x_monitor.cpp',
'ucd90160_monitor.cpp',
'ucd90320_monitor.cpp',
diff --git a/phosphor-power-sequencer/src/services.cpp b/phosphor-power-sequencer/src/services.cpp
index 80ad464..9c9ace7 100644
--- a/phosphor-power-sequencer/src/services.cpp
+++ b/phosphor-power-sequencer/src/services.cpp
@@ -61,24 +61,36 @@
// Initially assume hardware is not present
bool present{false};
- // Get presence from D-Bus interface/property
- try
+ // Try to find cached presence value
+ auto it = presenceCache.find(inventoryPath);
+ if (it != presenceCache.end())
{
- util::getProperty(INVENTORY_IFACE, PRESENT_PROP, inventoryPath,
- INVENTORY_MGR_IFACE, bus, present);
+ present = it->second;
}
- catch (const sdbusplus::exception_t& e)
+ else
{
- // If exception type is expected and indicates hardware not present
- if (isExpectedException(e))
+ // Get presence from D-Bus interface/property
+ try
{
- present = false;
+ util::getProperty(INVENTORY_IFACE, PRESENT_PROP, inventoryPath,
+ INVENTORY_MGR_IFACE, bus, present);
}
- else
+ catch (const sdbusplus::exception_t& e)
{
- // Re-throw unexpected exception
- throw;
+ // If exception type is expected and indicates hardware not present
+ if (isExpectedException(e))
+ {
+ present = false;
+ }
+ else
+ {
+ // Re-throw unexpected exception
+ throw;
+ }
}
+
+ // Cache presence value
+ presenceCache[inventoryPath] = present;
}
return present;
diff --git a/phosphor-power-sequencer/src/services.hpp b/phosphor-power-sequencer/src/services.hpp
index 743c034..255e2c5 100644
--- a/phosphor-power-sequencer/src/services.hpp
+++ b/phosphor-power-sequencer/src/services.hpp
@@ -18,13 +18,12 @@
#include "pmbus.hpp"
#include "xyz/openbmc_project/Logging/Entry/server.hpp"
-#include <fmt/format.h>
-
#include <phosphor-logging/lg2.hpp>
#include <sdbusplus/bus.hpp>
#include <sdbusplus/exception.hpp>
#include <cstdint>
+#include <format>
#include <map>
#include <memory>
#include <string>
@@ -127,6 +126,15 @@
createPMBus(uint8_t bus, uint16_t address,
const std::string& driverName = "",
size_t instance = 0) = 0;
+
+ /**
+ * Clear any cached data.
+ *
+ * Some data may be cached for performance reasons, such as hardware
+ * presence. Clearing the cache results in the latest data being obtained
+ * by a subsequent method calls.
+ */
+ virtual void clearCache() = 0;
};
/**
@@ -188,11 +196,17 @@
const std::string& driverName = "",
size_t instance = 0) override
{
- std::string path = fmt::format("/sys/bus/i2c/devices/{}-{:04x}", bus,
+ std::string path = std::format("/sys/bus/i2c/devices/{}-{:04x}", bus,
address);
return std::make_unique<PMBus>(path, driverName, instance);
}
+ /** @copydoc Services::clearCache() */
+ virtual void clearCache() override
+ {
+ presenceCache.clear();
+ }
+
private:
/**
* Returns whether the specified D-Bus exception is one of the expected
@@ -206,6 +220,13 @@
* D-Bus bus object.
*/
sdbusplus::bus_t& bus;
+
+ /**
+ * Cached presence data.
+ *
+ * Map from inventory paths to presence values.
+ */
+ std::map<std::string, bool> presenceCache{};
};
} // namespace phosphor::power::sequencer
diff --git a/phosphor-power-sequencer/test/meson.build b/phosphor-power-sequencer/test/meson.build
index 4f6b167..6bce451 100644
--- a/phosphor-power-sequencer/test/meson.build
+++ b/phosphor-power-sequencer/test/meson.build
@@ -4,6 +4,7 @@
'config_file_parser_tests.cpp',
'rail_tests.cpp',
dependencies: [
+ gmock,
gtest,
nlohmann_json_dep
],
diff --git a/phosphor-power-sequencer/test/mock_services.hpp b/phosphor-power-sequencer/test/mock_services.hpp
index 78ccbc2..c0544f0 100644
--- a/phosphor-power-sequencer/test/mock_services.hpp
+++ b/phosphor-power-sequencer/test/mock_services.hpp
@@ -44,7 +44,7 @@
MOCK_METHOD(void, logInfoMsg, (const std::string& message), (override));
MOCK_METHOD(void, logError,
(const std::string& message, Entry::Level severity,
- std::map<std::string, std::string>& additionalData),
+ (std::map<std::string, std::string> & additionalData)),
(override));
MOCK_METHOD(bool, isPresent, (const std::string& inventoryPath),
(override));
@@ -52,12 +52,12 @@
(override));
virtual std::unique_ptr<PMBusBase>
- createPMBus(uint8_t bus, uint16_t address,
- const std::string& driverName = "",
- size_t instance = 0) override
+ createPMBus(uint8_t, uint16_t, const std::string&, size_t) override
{
return std::make_unique<MockPMBus>();
}
+
+ MOCK_METHOD(void, clearCache, (), (override));
};
} // namespace phosphor::power::sequencer