platform-mc: Defer adding sensor objects from PDRs
After all PDRs retrieved from the other terminus are parsed into pdr
structs. They will be processed further to create sensor objects (e.g
via `addNumericSensor` function for Numeric Sensors).
During this phase for one sensor, sensor name is achieved (may enlist
Sensor Aux Name PDRs), and NumericSensor object is constructed.
Sensor object construction involves parsing pdr struct elements into
sensor class variables, D-Bus interface initialization and many calls
to set D-Bus object properties.
They are actually not blocking actions, but as it continuously loops
through sensor PDRs in `parseTerminusPDRs` to add sensors, they take
too much time and prevent BMC from processing events coming from other
termini. Not to mention the adding of new sensor types in the future
and the increase in sensor numbers, the total time will be large, while
this is not a primary task in the initialization phase of a terminus.
This commit defers `addNumericSensor` and `addCompactNumericSensor`
calls using sdeventplus::source::Defer event source, instead of
continuously calling them while looping through the sensor PDRs, to let
events coming from other termini break in and be processed by BMC.
Tested:
While BMC is getting, parsing sensor PDRs and creating sensor objects
from those PDRs, events can still be received from the other termini.
Change-Id: I4a3bacd4139b51c302e36614757fa1b5e5105f21
Signed-off-by: Chau Ly <chaul@amperecomputing.com>
diff --git a/platform-mc/test/event_manager_test.cpp b/platform-mc/test/event_manager_test.cpp
index f27f94e..087dff3 100644
--- a/platform-mc/test/event_manager_test.cpp
+++ b/platform-mc/test/event_manager_test.cpp
@@ -9,6 +9,7 @@
#include "platform-mc/platform_manager.hpp"
#include "platform-mc/terminus_manager.hpp"
#include "test/test_instance_id.hpp"
+#include "utils_test.hpp"
#include <gtest/gtest.h>
@@ -45,7 +46,7 @@
#define WARNING_HIGH 45
pldm_tid_t tid = 1;
termini[tid] = std::make_shared<pldm::platform_mc::Terminus>(
- tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
std::vector<uint8_t> pdr1{
0x1,
0x0,
@@ -146,6 +147,9 @@
termini[tid]->pdrs.emplace_back(pdr1);
termini[tid]->pdrs.emplace_back(pdr2);
termini[tid]->parseTerminusPDRs();
+ // Run event loop for a few seconds to let sensor creation
+ // defer tasks be run. May increase time when sensor num is large
+ utils::runEventLoopForSeconds(event, 1);
EXPECT_EQ(1, termini[tid]->numericSensors.size());
uint8_t platformEventStatus = 0;
@@ -170,7 +174,7 @@
auto mappedTid = terminusManager.mapTid(pldm::MctpInfo(10, "", "", 1));
auto tid = mappedTid.value();
termini[tid] = std::make_shared<pldm::platform_mc::Terminus>(
- tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
auto terminus = termini[tid];
/* Set supported command by terminus */
@@ -374,6 +378,9 @@
// should finish immediately
stdexec::sync_wait(platformManager.initTerminus());
+ // Run event loop for a few seconds to let sensor creation
+ // defer tasks be run. May increase time when sensor num is large
+ utils::runEventLoopForSeconds(event, 1);
EXPECT_EQ(true, terminus->initialized);
EXPECT_EQ(32, terminus->maxBufferSize);
EXPECT_EQ(0x06, terminus->synchronyConfigurationSupported.byte);
@@ -398,7 +405,7 @@
auto mappedTid = terminusManager.mapTid(pldm::MctpInfo(10, "", "", 1));
auto tid = mappedTid.value();
termini[tid] = std::make_shared<pldm::platform_mc::Terminus>(
- tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
auto terminus = termini[tid];
// queue pollForPlatformEventMessage first part response
diff --git a/platform-mc/test/platform_manager_test.cpp b/platform-mc/test/platform_manager_test.cpp
index ef226f8..ab4921b 100644
--- a/platform-mc/test/platform_manager_test.cpp
+++ b/platform-mc/test/platform_manager_test.cpp
@@ -2,6 +2,7 @@
#include "mock_terminus_manager.hpp"
#include "platform-mc/platform_manager.hpp"
#include "test/test_instance_id.hpp"
+#include "utils_test.hpp"
#include <sdeventplus/event.hpp>
@@ -37,7 +38,7 @@
auto mappedTid = mockTerminusManager.mapTid(pldm::MctpInfo(10, "", "", 1));
auto tid = mappedTid.value();
termini[tid] = std::make_shared<pldm::platform_mc::Terminus>(
- tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
auto terminus = termini[tid];
/* Set supported command by terminus */
@@ -190,6 +191,9 @@
EXPECT_EQ(true, terminus->initialized);
EXPECT_EQ(true, terminus->doesSupportCommand(PLDM_PLATFORM, PLDM_GET_PDR));
EXPECT_EQ(2, terminus->pdrs.size());
+ // Run event loop for a few seconds to let sensor creation
+ // defer tasks be run. May increase time when sensor num is large
+ utils::runEventLoopForSeconds(event, 1);
EXPECT_EQ(1, terminus->numericSensors.size());
EXPECT_EQ("S0", terminus->getTerminusName().value());
}
@@ -200,7 +204,7 @@
auto mappedTid = mockTerminusManager.mapTid(pldm::MctpInfo(10, "", "", 1));
auto tid = mappedTid.value();
termini[tid] = std::make_shared<pldm::platform_mc::Terminus>(
- tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
auto terminus = termini[tid];
/* Set supported command by terminus */
@@ -361,7 +365,7 @@
auto mappedTid = mockTerminusManager.mapTid(pldm::MctpInfo(10, "", "", 1));
auto tid = mappedTid.value();
termini[tid] = std::make_shared<pldm::platform_mc::Terminus>(
- tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
auto terminus = termini[tid];
/* Set supported command by terminus */
@@ -478,11 +482,14 @@
// terminus doesn't Type2 support
auto mappedTid = mockTerminusManager.mapTid(pldm::MctpInfo(10, "", "", 1));
auto tid = mappedTid.value();
- termini[tid] =
- std::make_shared<pldm::platform_mc::Terminus>(tid, 1 << PLDM_BASE);
+ termini[tid] = std::make_shared<pldm::platform_mc::Terminus>(
+ tid, 1 << PLDM_BASE, event);
auto terminus = termini[tid];
stdexec::sync_wait(platformManager.initTerminus());
+ // Run event loop for a few seconds to let sensor creation
+ // defer tasks be run. May increase time when sensor num is large
+ utils::runEventLoopForSeconds(event, 1);
EXPECT_EQ(true, terminus->initialized);
EXPECT_EQ(0, terminus->pdrs.size());
EXPECT_EQ(0, terminus->numericSensors.size());
@@ -494,7 +501,7 @@
auto mappedTid = mockTerminusManager.mapTid(pldm::MctpInfo(10, "", "", 1));
auto tid = mappedTid.value();
termini[tid] = std::make_shared<pldm::platform_mc::Terminus>(
- tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
auto terminus = termini[tid];
// queue getPDRRepositoryInfo response cc=PLDM_ERROR
@@ -515,6 +522,9 @@
EXPECT_EQ(rc, PLDM_SUCCESS);
stdexec::sync_wait(platformManager.initTerminus());
+ // Run event loop for a few seconds to let sensor creation
+ // defer tasks be run. May increase time when sensor num is large
+ utils::runEventLoopForSeconds(event, 1);
EXPECT_EQ(true, terminus->initialized);
EXPECT_EQ(0, terminus->pdrs.size());
EXPECT_EQ(0, terminus->numericSensors.size());
diff --git a/platform-mc/test/sensor_manager_test.cpp b/platform-mc/test/sensor_manager_test.cpp
index 4ec9f5b..3bf185e 100644
--- a/platform-mc/test/sensor_manager_test.cpp
+++ b/platform-mc/test/sensor_manager_test.cpp
@@ -3,6 +3,7 @@
#include "mock_sensor_manager.hpp"
#include "platform-mc/terminus_manager.hpp"
#include "test/test_instance_id.hpp"
+#include "utils_test.hpp"
#include <sdeventplus/event.hpp>
@@ -22,24 +23,6 @@
sensorManager(event, terminusManager, termini, nullptr)
{}
- void runEventLoopForSeconds(uint64_t sec)
- {
- uint64_t t0 = 0;
- uint64_t t1 = 0;
- uint64_t usec = sec * 1000000;
- uint64_t elapsed = 0;
- sd_event_now(event.get(), CLOCK_MONOTONIC, &t0);
- do
- {
- if (!sd_event_run(event.get(), usec - elapsed))
- {
- break;
- }
- sd_event_now(event.get(), CLOCK_MONOTONIC, &t1);
- elapsed = t1 - t0;
- } while (elapsed < usec);
- }
-
PldmTransport* pldmTransport = nullptr;
sdbusplus::bus_t& bus;
sdeventplus::Event event;
@@ -151,9 +134,8 @@
TEST_F(SensorManagerTest, sensorPollingTest)
{
uint64_t seconds = 10;
-
pldm_tid_t tid = 1;
- termini[tid] = std::make_shared<pldm::platform_mc::Terminus>(tid, 0);
+ termini[tid] = std::make_shared<pldm::platform_mc::Terminus>(tid, 0, event);
termini[tid]->pdrs.push_back(pdr1);
termini[tid]->pdrs.push_back(pdr2);
termini[tid]->parseTerminusPDRs();
@@ -172,7 +154,7 @@
sensorManager.startPolling(tid);
- runEventLoopForSeconds(seconds);
+ utils::runEventLoopForSeconds(event, seconds);
sensorManager.stopPolling(tid);
}
diff --git a/platform-mc/test/terminus_manager_test.cpp b/platform-mc/test/terminus_manager_test.cpp
index 80d2699..f4f0ec1 100644
--- a/platform-mc/test/terminus_manager_test.cpp
+++ b/platform-mc/test/terminus_manager_test.cpp
@@ -553,7 +553,7 @@
auto mappedTid = mockTerminusManager.mapTid(mctpInfo);
auto tid = mappedTid.value();
termini[tid] = std::make_shared<pldm::platform_mc::Terminus>(
- tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ tid, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
auto terminus = termini[tid];
auto mappedTid1 = terminusManager.mapTid(mctpInfo);
diff --git a/platform-mc/test/terminus_test.cpp b/platform-mc/test/terminus_test.cpp
index a8dcbc1..77ae931 100644
--- a/platform-mc/test/terminus_test.cpp
+++ b/platform-mc/test/terminus_test.cpp
@@ -7,9 +7,10 @@
TEST(TerminusTest, supportedTypeTest)
{
- auto t1 = pldm::platform_mc::Terminus(1, 1 << PLDM_BASE);
- auto t2 =
- pldm::platform_mc::Terminus(2, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ auto event = sdeventplus::Event::get_default();
+ auto t1 = pldm::platform_mc::Terminus(1, 1 << PLDM_BASE, event);
+ auto t2 = pldm::platform_mc::Terminus(
+ 2, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
EXPECT_EQ(true, t1.doesSupportType(PLDM_BASE));
EXPECT_EQ(false, t1.doesSupportType(PLDM_PLATFORM));
@@ -19,16 +20,18 @@
TEST(TerminusTest, getTidTest)
{
+ auto event = sdeventplus::Event::get_default();
const pldm_tid_t tid = 1;
- auto t1 = pldm::platform_mc::Terminus(tid, 1 << PLDM_BASE);
+ auto t1 = pldm::platform_mc::Terminus(tid, 1 << PLDM_BASE, event);
EXPECT_EQ(tid, t1.getTid());
}
TEST(TerminusTest, parseSensorAuxiliaryNamesPDRTest)
{
- auto t1 =
- pldm::platform_mc::Terminus(1, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ auto event = sdeventplus::Event::get_default();
+ auto t1 = pldm::platform_mc::Terminus(
+ 1, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
std::vector<uint8_t> pdr1{
0x0,
0x0,
@@ -110,8 +113,9 @@
TEST(TerminusTest, parseSensorAuxiliaryMultiNamesPDRTest)
{
- auto t1 =
- pldm::platform_mc::Terminus(1, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ auto event = sdeventplus::Event::get_default();
+ auto t1 = pldm::platform_mc::Terminus(
+ 1, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
std::vector<uint8_t> pdr1{
0x0,
0x0,
@@ -229,8 +233,9 @@
TEST(TerminusTest, parseSensorAuxiliaryNamesMultiSensorsPDRTest)
{
- auto t1 =
- pldm::platform_mc::Terminus(1, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ auto event = sdeventplus::Event::get_default();
+ auto t1 = pldm::platform_mc::Terminus(
+ 1, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
std::vector<uint8_t> pdr1{
0x0,
0x0,
@@ -350,8 +355,9 @@
TEST(TerminusTest, parsePDRTestNoSensorPDR)
{
- auto t1 =
- pldm::platform_mc::Terminus(1, 1 << PLDM_BASE | 1 << PLDM_PLATFORM);
+ auto event = sdeventplus::Event::get_default();
+ auto t1 = pldm::platform_mc::Terminus(
+ 1, 1 << PLDM_BASE | 1 << PLDM_PLATFORM, event);
std::vector<uint8_t> pdr1{
0x1, 0x0, 0x0,
0x0, // record handle
diff --git a/platform-mc/test/utils_test.hpp b/platform-mc/test/utils_test.hpp
new file mode 100644
index 0000000..ca78082
--- /dev/null
+++ b/platform-mc/test/utils_test.hpp
@@ -0,0 +1,24 @@
+#include <systemd/sd-event.h>
+
+#include <sdeventplus/event.hpp>
+
+namespace utils
+{
+void runEventLoopForSeconds(sdeventplus::Event& event, uint64_t sec)
+{
+ uint64_t t0 = 0;
+ uint64_t t1 = 0;
+ uint64_t usec = sec * 1000000;
+ uint64_t elapsed = 0;
+ sd_event_now(event.get(), CLOCK_MONOTONIC, &t0);
+ do
+ {
+ if (!sd_event_run(event.get(), usec - elapsed))
+ {
+ break;
+ }
+ sd_event_now(event.get(), CLOCK_MONOTONIC, &t1);
+ elapsed = t1 - t0;
+ } while (elapsed < usec);
+}
+} // namespace utils