Skip to content

allocator: Use appropriate memory types #804

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.

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

Merged
merged 2 commits into from
May 14, 2023
Merged

allocator: Use appropriate memory types #804

merged 2 commits into from
May 14, 2023

Conversation

medhefgo
Copy link
Contributor

No description provided.

@phip1611
Copy link
Member

phip1611 commented May 14, 2023

Thanks for the contribution! I see why this is beneficial and probably this is the right thing to do. Could you please briefly explain the high-level use case you are working on? I'm curious in what scenario this was a problem.

All I can think of at the moment is that you were installing a custom boot service driver and you want that memory to be freed when boot services are exited?

PS: Plz fix the CI warnings.

@@ -22,6 +23,9 @@ use crate::table::boot::{BootServices, MemoryType};
/// exited by the host application yet.
static mut BOOT_SERVICES: Option<NonNull<BootServices>> = None;

/// The memory type used for pool memory allocations.
static mut MEMORY_TYPE: MemoryType = MemoryType::LOADER_DATA;
Copy link
Member

@phip1611 phip1611 May 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I'd like to have the Once type here but it is a nightly-only API so far (https://doc.rust-lang.org/core/cell/struct.OnceCell.html).

Could you at least add a comment that this value is supposed to be set once during initialization and never altered again?

medhefgo added 2 commits May 14, 2023 15:30
Boot/runtime driveres generally only want to allocate with their
respective memory types. Using the wrong allocation types or mixing them
can result in hard to debug issues (when loading linux).
@medhefgo
Copy link
Contributor Author

I'm actually not doing anything with this. I've been hit by mixed allocations in sd-boot and just think that uefi-rs should try to avoid this. (It's also what gnu-efi does for its AllocatePool.)

Copy link
Member

@nicholasbishop nicholasbishop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for fixing that

@nicholasbishop nicholasbishop merged commit e6803d7 into rust-osdev:main May 14, 2023
@medhefgo medhefgo deleted the mem-type branch May 14, 2023 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants