Skip to content
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

Frame allocator will allocate two copies of a frame #451

Closed
Ramla-I opened this issue Oct 19, 2021 · 4 comments · Fixed by #452 or #456
Closed

Frame allocator will allocate two copies of a frame #451

Ramla-I opened this issue Oct 19, 2021 · 4 comments · Fixed by #452 or #456
Assignees

Comments

@Ramla-I
Copy link
Member

Ramla-I commented Oct 19, 2021

// Allocate pages for RQ and SQ
let (rq_mp, rq_pa) = create_contiguous_mapping(rq_size_in_bytes, NIC_MAPPING_FLAGS)?;
let sq_pa = PhysicalAddress::new(rq_pa.value() + rq_size_in_bytes).ok_or("Could not create starting address for SQ")?; 
let sq_mp = allocate_memory(sq_pa, sq_size_in_bytes)?;
        
// Allocate page for SQ/RQ doorbell
let (db_page, db_pa) = create_contiguous_mapping(4096, NIC_MAPPING_FLAGS)?;

This is the portion of code where the problem occurs. The resulting physical address values are:
rq_pa = 0x586b000
sq_pa = 0x587b000
db_pa = 0x587b000

I've traced the issue to the allocate_frames_deferred() function in the frame_allocator crate. It seems that when a physical address is provided to the function, it'll add the required frames to the FREE_RESERVED_FRAMES_LIST without checking if that frame is present in the FREE_FRAMES_LIST.

I'm not sure if the solution is to return an Error if a user requests a physical address that is not in the reserved portions, or to return AllocatedFrames from the FREE_FRAMES_LIST.

@kevinaboos
Copy link
Member

Thanks for filing the issue, I'll look into this immediately.

kevinaboos added a commit that referenced this issue Oct 20, 2021
…ved frame ranges (#452)

* Ensure that frames allocated from a requested address are removed from the list of free general-purpose frames. Closes #451
@kevinaboos
Copy link
Member

For clarity, the solution was to take the range of requested frames from the general-purpose list of free frames and then treat those frames as reserved from then on.

github-actions bot pushed a commit that referenced this issue Oct 20, 2021
…ved frame ranges (#452)

* Ensure that frames allocated from a requested address are removed from the list of free general-purpose frames. Closes #451 abb5b83
@Ramla-I Ramla-I reopened this Oct 26, 2021
@Ramla-I
Copy link
Member Author

Ramla-I commented Oct 26, 2021

You can still allocate duplicate frames at any given physical address that have been allocated before from the FREE_FRAMES_LIST. That's because the check inserted just ensures that the frames are currently not in the free list.

let (mp, pa) = create_contiguous_mapping(100, NIC_MAPPING_FLAGS).unwrap();
let mp2 = allocate_memory(pa, 100).unwrap();
let mp3 = allocate_memory(pa, 100).unwrap();

mp and mp2 will be allocated with duplicate frames. mp3 will fail because by then we've added that frame to the list of reserved regions.

@kevinaboos
Copy link
Member

Thanks for correctly identifying this problem! I'll fix it hopefully by tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment