allocator: make aligned allocations precise #3607
Closed
Conversation
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
lyakh
requested review from
dbaluta,
jajanusz,
lbetlej,
lgirdwood,
libinyang,
mmaka1 and
plbossart
as code owners
lgirdwood
reviewed
Nov 16, 2020
Member
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
Can you give some background here, what is this fixing ?
Collaborator
Author
|
lgirdwood
approved these changes
Nov 16, 2020
Collaborator
|
@lyakh after your explanation i feel this PR should be split into 3 patches each with proper commit message explaining why it is needed. |
Member
|
@lyakh please also check CI |
No need for two divisions for an alignment macro, one is enough. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Simplify the align_ptr() function by re-using an existing ALIGN() macro. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
alloc_block() and alloc_cont_blocks() don't use their "caps" parameter, remote it. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
The condition "size + alignment <= block_size" for allocating memory from a signle buffer is sufficient but not precise enough. For example if we want to allocate 20 bytes with 64-byte alignment, a 32-byte buffer *might* be sufficient if it's suitably aligned. Fix the algorithm to account for such cases. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
alloc_heap_buffer() is trying to allocate memory from a single heap, no need to check its size on each iteration of an internal loop over maps. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
When allocating multiple buffers the first free buffer might not contain a suitably aligned start address. Skip it instead of allocating it with the others. Also don't add alignment to the allocation size, but find a suitably aligned start address instead. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Collaborator
Author
|
SOFCI TEST |
Member
|
@dbaluta it looks like you got 6 patches now ! |
Merged
Member
|
@lyakh can you check CI. |
Collaborator
Author
|
@lgirdwood splitting this into #3642, #3646 and #3647, we can close this one then. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Currently when the allocator tries to allocate aligned buffers, if it detects, that the current alignment isn't suitable, it adds alignment to the requested size and tries to allocate that. This is too imprecise and counterintuitive. In the very worst theoretical case alignment - 1 bytes might be needed additionally. Also no need to begin grabbing buffers for continuous allocations, which don't contain an address with required alignment. Simplify the alignment macro too to only perform division once.