mm/alloc: multiple simplifications and cleanups #149
Merged
Conversation
This file contains 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
|
I'm planning to use this patchset as a base for a future PR which will introduce mm-specific errors, but I'll wait until this is merged to keep it simple and make reviews easier. |
|
This has conflicts now, can you please rebase to latest main branch? |
|
Test failure is unrelated, but merge #155 first just in case. |
Add trivial derives to types related to page information. Signed-off-by: Carlos López <[email protected]>
Move the page info being written outside loops in cases where it did not need to be inside. This makes code clearer. Signed-off-by: Carlos López <[email protected]>
Most page allocation paths perform a series of common steps, with the only difference being the page order and page information stored. Factor out the common steps to a single function. Signed-off-by: Carlos López <[email protected]>
Use let-else syntax instead of pattern matching in a few places, making code easier to read. Signed-off-by: Carlos López <[email protected]>
Currently, there are two different paths to get the information for a specific page: 1. Passing a pfn to `MemoryRegion::read_page_info()`, which will panic on an invalid pfn (via `MemoryRegion::check_pfn()`). 2. Passing a `VirtAddr` to `MemoryRegion::get_page_info()`, which will return an error on an invalid address, and then will call `MemoryRegion::read_page_info()` by computing the correct pfn for the address. This is a bit confusing and is redundant in some cases, as some of the callers of `MemoryRegion::get_page_info()` also need the pfn afterwards, which means the pfn is computed twice. To simplify this, decouple the `VirtAddr` to pfn conversion from the information reading by introducing `MemoryRegion::get_pfn()`. The new method will perform all the necessary checks on the `VirtAddr`, and will return a valid pfn that can be used to retrieve the page information. This allows the removal of `MemoryRegion::get_page_info()`. In theory, this could also allow us to remmove `MemoryRegion::check_pfn()`, as all the pfns are coming from either the new function or the page freelist. However, we leave it in to catch corruption issues in the freelist. Signed-off-by: Carlos López <[email protected]>
PageStorageType is a transparent type, so reading a u64 from the pointer was fine, but it is not the actual type we store. This makes the function more symmetric with `MemoryRegion::write_page_info()`. Signed-off-by: Carlos López <[email protected]>
The code checking whether a VirtAddr is correctly within the heap's range is duplicated in a few places. Centralize all the checks to a single function (`MemoryRegion::get_virt_offset()`), reducing code complexity. Signed-off-by: Carlos López <[email protected]>
`MemoryRegion::page_info_virt_addr()` is used to read or write page information for a provided pfn. The returned `VirtAddr` is always casted to a pointer to a `PageStorageType` by the callers, so return that type directly. The arithmetic is also simplified, since the primitive pointer type has an `add()` method that takes into account the size of the generic type `T` of the pointer. To make things more clear, rename `page_info_virt_addr()` to `page_info_ptr()`. Mark it unsafe as well, as the addition of the pfn offset with overflow will cause undefined behavior, according to the documentation of the `add()` method. Signed-off-by: Carlos López <[email protected]>
Do not allow external callers to directly manipulate a file page's reference count, as that should only happen via the `PageRef` struct. Signed-off-by: Carlos López <[email protected]>
Replace an if-else chain to a return or continue early loop to improve code clarity. Signed-off-by: Carlos López <[email protected]>
Replace an if-else chain to a return or continue early loop to improve code clarity. Signed-off-by: Carlos López <[email protected]>
Signed-off-by: Carlos López <[email protected]>
SlabPageSlab::allocate() returns a pointer, so there is no need to transform the pointer to a mutable reference that will be coerced back to a pointer. Signed-off-by: Carlos López <[email protected]>
Having a &mut SlabPage is enough to call SlabPage::get_next_page(). Signed-off-by: Carlos López <[email protected]>
`slab_page` is already a `&mut SlabPage`. Signed-off-by: Carlos López <[email protected]>
…fill_page_list() Get the next_page pfn by using a safe array getter, which is a bit simpler. Fix leftover whitespace while we are at it. Signed-off-by: Carlos López <[email protected]>
Use Self instead of the type name in constructors. This makes code slightly clearer and makes future type renaming easier. Signed-off-by: Carlos López <[email protected]>
Replace a `map()` + `unwrap_or_else()` chain by the more concise `map_or_else()`. Signed-off-by: Carlos López <[email protected]>
Signed-off-by: Carlos López <[email protected]>
00xc
force-pushed
the
mm/alloc
branch
2 times, most recently
from
72b4c50 to
63599fc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Simplify certain code paths, mostly related to heap page management, and simplify syntax where possible.