Refactor CxPlat Pool Abstraction #4929
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4929 +/- ##
==========================================
- Coverage 87.42% 86.93% -0.50%
==========================================
Files 56 56
Lines 17726 17723 -3
==========================================
- Hits 15497 15407 -90
- Misses 2229 2316 +87 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…squic into cxplat-pool-refactor
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| CXPLAT_FRE_ASSERT(Pool->ListDepth > 0); | ||
| CXPLAT_POOL_HEADER* Header = | ||
| #if DEBUG | ||
| CxPlatGetAllocFailDenominator() ? NULL : |
There was a problem hiding this comment.
Is this fixing a bug in the previous behavior? It looks like previously it would allocate new on true, and allocate from the pool on false. Now it's return NULL on true, and allocating from the pool or new on false.
There was a problem hiding this comment.
ok yeah, it's coming back to me now. The point of this was to skip the pool's lookaside list and always call CxPlatAlloc when random memory failures are enabled so that the logic in CxPlatAlloc can decide to return NULL.
The problem here is there's now a timing change where it'll always take and then drop the lock before allocating directly, when it would skip the lock before. Maybe that'll exercise new code paths.
There was a problem hiding this comment.
The point of this was to skip the pool's lookaside list and always call CxPlatAlloc when random memory failures are enabled so that the logic in CxPlatAlloc can decide to return NULL.
It'll be nice to have this in a comment, I was wondering what this could be for.
There was a problem hiding this comment.
This code isn't changing any behavior. It's still skipping the lookaside list when we have either random allocations or custom allocation index dropping enabled. It sets the value to NULL here, which forces the code to call alloc below. And I'd rather not change the code once more if it's only for a comment. 😄
| CXPLAT_FRE_ASSERT(Pool->ListDepth > 0); | ||
| CXPLAT_POOL_HEADER* Header = | ||
| #if DEBUG | ||
| CxPlatGetAllocFailDenominator() ? NULL : |
There was a problem hiding this comment.
The point of this was to skip the pool's lookaside list and always call CxPlatAlloc when random memory failures are enabled so that the logic in CxPlatAlloc can decide to return NULL.
It'll be nice to have this in a comment, I was wondering what this could be for.
Description
Updates the CxPlat Pool abstraction to maintain the owning pool pointer so that free always goes back to the allocation source.
Testing
CI/CD
Documentation
N/A