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

Allocator test harness #52

Open
1 task
hawkw opened this issue Feb 18, 2018 · 9 comments
Open
1 task

Allocator test harness #52

hawkw opened this issue Feb 18, 2018 · 9 comments

Comments

@hawkw
Copy link
Member

hawkw commented Feb 18, 2018

Would be nice to have some code to make unit testing allocators easier:

At a minimum:

  • test FrameAllocator implementation that uses system alloc libraries to allocate "frames"(mmap?)

We might also want generic functions that take a generic allocator and make a bunch of assertions, since a lot of the things we'll want to test for correctness should be invariant across allocator implementations.

@amanjeev
Copy link
Member

I have no idea about this but I will try to take a shot at it.

@amanjeev amanjeev self-assigned this Feb 28, 2018
@hawkw
Copy link
Member Author

hawkw commented Feb 28, 2018

@amanjeev it's a pretty big issue, so we probably ought talk about a plan for this, and maybe break it down into multiple issues or something. But, I'm super happy you're willing to tackle it!

@amanjeev
Copy link
Member

But, I'm super happy you're willing to tackle it!

Thanks!

and maybe break it down into multiple issues or something

👍

@amanjeev
Copy link
Member

Adding notes here so I do not miss them -

Basics:

  • create a new alarm-test crate
  • it should depend on base only
  • create an implementation of this trait in there:
    pub unsafe trait Allocator {
    /// Architecture-dependent size of a physical page.
    const FRAME_SIZE: usize;
    /// Type representing frames provided by this allocator.
    ///
    /// A `Frame` must either be a pointer to a contiguous block of `FRAME_SIZE`
    /// bytes, or be a handle that may be converted into such a pointer.
    type Frame;
    /// Returns a new `Frame`.
    unsafe fn alloc(&mut self) -> Result<Self::Frame, AllocErr>;
    /// Deallocate a `Frame`.
    ///
    /// # Unsafety
    /// This function is unsafe because undefined behaviour may result if the
    /// given `frame` was not originally allocated by this `Allocator`.
    unsafe fn dealloc(&mut self, frame: Self::Frame) -> Result<(), AllocErr>;
    // TODO: alloc_range/dealloc_range; requires an architecture-independent
    // way of representing frame ranges.
    }

Notes:

  • we can probably implement it with a struct that has a vec of Box<[u8; 4096]> and tracking their numbers somehow.
  • We could probably make the page size smaller for test purposes
  • basically we want to be able to allocate “frames” to give to our allocator but instead of actually tracking physical frames in Real Life Memory we can just make some appropriately big arrays and have rust handle the allocation

@amanjeev
Copy link
Member

amanjeev commented Jul 1, 2018

More insight from @hawkw, adding here for documentation so its not lost:

I think what you'll want to do (after rebasing onto master) is write a new struct implementing Page. It should probably be something like this:

struct MockFrame {
    number: usize, // count the frame numbers for tracking allocations etc.
    frame: [u8; FRAME_SIZE], // the frame's actual memory area.
}

the frame's base address would be

&self.frame[0] as *const _ as usize

(the first byte in the frame) and the end address would be

&self.frame[PAGE_SIZE] as *const _ as usize

We'd probably want to leave the from_addr_up/from_addr_down methods unimplemented!() as we'd only expect to get MockFrames from our test allocator and not through address math.

@hawkw
Copy link
Member Author

hawkw commented Jul 8, 2018

We'd probably want to leave the from_addr_up/from_addr_down methods unimplemented!() as we'd only expect to get MockFrames from our test allocator and not through address math.

Having thought about it a bit more, I think perhaps those methods could be defined on a separate trait that some implementers of Page can also implement, though on the other hand, that could just be introducing needless complexity in most cases... I'm still figuring out how the HAL APIs ought to be designed.

@amanjeev
Copy link
Member

though on the other hand, that could just be introducing needless complexity in most cases.

Seems like it will be. Most usage won't be like this test suite that I am writing, right?

Regarding the trait Page, it has type declaration type Address: Address; which is bound by trait Address https://github.com/sos-os/hal9000/blob/master/hal9000/src/mem.rs#L48.

The compiler tells me -

error[E0277]: the trait bound `usize: hal9000::mem::Address` is not satisfied
  --> alarm-test/src/lib.rs:11:6
   |
11 | impl Page for MockFrame {
   |      ^^^^ the trait `hal9000::mem::Address` is not implemented for `usize`

I do not think I can impl Address for usize given neither is local to me. Am I missing something Rusty here or something OSy.

@hawkw
Copy link
Member Author

hawkw commented Jul 23, 2018

@amanjeev You're going to want to define a newtype. Try something like this:

#[macro_use] extern crate hal9000_derive;

#[derive(Address)]
#[address_repr(usize)]
pub struct MockAddress(usize);

And then you should be able to use MockAddress as the Addr type for your Page type.

Note that the derive crate for hal9000 is still a very rough work in progress, so please let me know if anything goes wrong --- it's probably my fault, not yours!

@amanjeev
Copy link
Member

I was getting an error of non-primitive cast

error[E0605]: non-primitive cast: `*const u8` as `MockAddress`
  --> alarm-test/src/lib.rs:41:9
   |
41 |         &self.frame[4096] as *const _ as MockAddress
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: an `as` expression can only be used to convert between primitive types. Consider using the `From` trait

My solution was to do this MockAddress(self.frame[0]).

So far, a skeleton is here -
https://github.com/sos-os/alarm/blob/amanjeev/allocator-test-harness-part-2/alarm-test/src/lib.rs#L36

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

No branches or pull requests

2 participants