sol: proper deallocation of struct sol_config#448
Merged
AltraMayor merged 4 commits intomasterfrom Dec 22, 2020
Merged
Conversation
Collaborator
|
Patch ggu: fix cleanup stack of run_ggu() is ready for merge. |
Collaborator
|
Patch sol: review included headers in SOL's header is ready for merge. |
mengxiang0811
requested changes
Dec 20, 2020
Collaborator
mengxiang0811
left a comment
There was a problem hiding this comment.
sol: proper deallocation of struct sol_config
The cleanup stack of run_ggu() was missing a put on the gk_config. This bug has likely never been triggered.
The header of the SOL block was including headers that are no longer needed. This patch moves the headers still needed to sol/main.c.
d9442e1 to
acb94d4
Compare
This patch improves comments on the reference counting of struct gk_config.
acb94d4 to
3f32c70
Compare
The SOL block was originally designed to have a single instance, so the code was not accounting for multiple deallocations. This patch adds the reference count that is employed in the GK and GT blocks to deal with the issue. On top of the multiple deallocations, there is a race condition during the deallocations, so the errors that are logged vary. The following is only an example of what can be found: RING: Cannot free ring, not created with rte_ring_create() The problem becomes more pronounced with the increase of NUMA nodes since a SOL instance is allocated to each NUMA node. While Gatekeeper was not finishing cleanly, this issue was ONLY creating stranger log entries.
3f32c70 to
df5eb2b
Compare
Owner
Author
|
It's ready for another review. All the patches moved because the branch was rebased in relation to the master branch. |
Collaborator
|
sol: proper deallocation of struct sol_config is ready for merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The SOL block was originally designed to have a single instance, so the code was not accounting for multiple deallocations. This patch adds the reference count that is employed in the GK and GT blocks to deal with the issue.
While Gatekeeper was not finishing cleanly, this issue was ONLY creating stranger log entries.