Skip to content

Commit

Permalink
Remove redundant async call in FRU collection threads (ibm-openbmc#588)
Browse files Browse the repository at this point in the history
This commit removes the redundant std:async call in the detached thread
launched for parsing and publishing the VPD for an individual FRU.
Since we have a dedicated detached thread for each FRU, we can do VPD
parse and publish in a synchronous manner from the detached thread
itself, instead of launching a asynchronous task which adds unnecessary
performance cost. This commit also handles any exception thrown while
launching the detached thread for a FRU. Incase launching detached
thread for a FRU fails, we log a PEL and continue launching threads for
other FRUs.

This commit addresses issue ibm-openbmc#558.

Test:

```
1. Install bitbaked image on Everest (ever6bmc)
2. After initial boot, check:
	- BMC State Ready
	- vpd-manager's CollectionStatus property should be "Completed"
	busctl get-property com.ibm.VPD.Manager
	/com/ibm/VPD/Manager com.ibm.VPD.Manager
	CollectionStatus
	s "Completed"
	- vpd-manager status should be running with no restarts
	- vpd-manager should have only 1 thread running:
	  check "ls -la /proc/<vpd-manager PID>/task"
3. Reboot the BMC several times and repeat Step 2.
```

Change-Id: I603c64dc9b5057429a2288f0edfde6086755b851
Signed-off-by: Souvik Roy <[email protected]>
  • Loading branch information
Souvik Roy committed Jan 17, 2025
1 parent 8d35fae commit 76c6b03
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 14 deletions.
1 change: 1 addition & 0 deletions vpd-manager/include/worker.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class Worker
* Note: Config JSON file path should be passed to worker class constructor
* to make use of this API.
*
* @throw std::runtime_error
*/
void collectFrusFromJson();

Expand Down
38 changes: 24 additions & 14 deletions vpd-manager/src/worker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "backup_restore.hpp"
#include "configuration.hpp"
#include "constants.hpp"
#include "event_logger.hpp"
#include "exceptions.hpp"
#include "logger.hpp"
#include "parser.hpp"
Expand Down Expand Up @@ -1512,22 +1513,31 @@ void Worker::collectFrusFromJson()
continue;
}

std::thread{[vpdFilePath, this]() {
auto l_futureObject = std::async(&Worker::parseAndPublishVPD, this,
vpdFilePath);

std::tuple<bool, std::string> l_threadInfo = l_futureObject.get();
try
{
std::thread{[vpdFilePath, this]() {
const auto& l_parseResult = parseAndPublishVPD(vpdFilePath);

// thread returned.
m_mutex.lock();
m_activeCollectionThreadCount--;
m_mutex.unlock();
m_mutex.lock();
m_activeCollectionThreadCount--;
m_mutex.unlock();

if (!m_activeCollectionThreadCount)
{
m_isAllFruCollected = true;
}
}}.detach();
if (!m_activeCollectionThreadCount)
{
m_isAllFruCollected = true;
}
}}.detach();
}
catch (const std::exception& l_ex)
{
// TODO: Should we re-try launching thread for this FRU?
EventLogger::createSyncPel(
types::ErrorType::InvalidVpdMessage, types::SeverityType::Alert,
__FILE__, __FUNCTION__, 0,
std::string("Failed to start collection thread for FRU :[" +
vpdFilePath + "]. Error: " + l_ex.what()),
std::nullopt, std::nullopt, std::nullopt, std::nullopt);
}
}
}

Expand Down

0 comments on commit 76c6b03

Please sign in to comment.