Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

For each FRU, vpd-manager launches an extra unnecessary asynchronous task(and potentially nested thread) #558

Open
souvik1914581 opened this issue Jan 2, 2025 · 0 comments
Open

For each FRU, vpd-manager launches an extra unnecessary asynchronous task(and potentially nested thread) #558

souvik1914581 opened this issue Jan 2, 2025 · 0 comments

Comments

@souvik1914581
Copy link

souvik1914581 commented Jan 2, 2025 •
edited
Loading

Context:

  1. vpd-manager calls Worker::collectFrusFromJson() to parse and publish VPD of all FRUs.
  2. Worker::collectFrusFromJson() iterates through the list of FRUs in System Config JSON and launches a detached thread for VPD collection of each FRU.
  3. Within the body of this detached thread, we are calling std::async with default launch policy, in order to parse and publish the VPD of a FRU in an asynchronous manner. See Worker::collectFrusFromJson()

Issue:

  1. Calling std::async inside the detached thread for a FRU and then collecting the result in a std::future is an unnecessary performance overhead.
  2. Calling std::async without specifying launch policy may potentially launch another thread. See std::async
  3. std::async may throw some exceptions. Worker::collectFrusFromJson() doxygen does not capture this.

As we already have a dedicated detached thread for collecting VPD of a FRU, we should just parse and publish the VPD for this FRU in a synchronous manner from the thread.
souvik1914581 pushed a commit to souvik1914581/openpower-vpd-parser that referenced this issue Jan 15, 2025
Remove redundant async call in FRU collection threads (ibm-openbmc#588)
6a5fc3f 
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 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]>
@souvik1914581 souvik1914581 mentioned this issue Jan 15, 2025
Open
souvik1914581 pushed a commit to souvik1914581/openpower-vpd-parser that referenced this issue Jan 17, 2025
Remove redundant async call in FRU collection threads (ibm-openbmc#588)
bf9dbbf 
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 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]>
souvik1914581 pushed a commit to souvik1914581/openpower-vpd-parser that referenced this issue Jan 17, 2025
Remove redundant async call in FRU collection threads (ibm-openbmc#588)
76c6b03 
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]>
souvik1914581 pushed a commit to souvik1914581/openpower-vpd-parser that referenced this issue Jan 21, 2025
Remove redundant async call in FRU collection threads (ibm-openbmc#588)
a5e4cbf 
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]>
souvik1914581 pushed a commit to souvik1914581/openpower-vpd-parser that referenced this issue Jan 23, 2025
Remove redundant async call in FRU collection threads (ibm-openbmc#588)
b74977c 
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@souvik1914581