-
Notifications
You must be signed in to change notification settings - Fork 139
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
move malloc from Solo5 to libOS layer #223
Comments
Thanks for the change @djwillia . It makes things easier on the IncludeOS side. We are currently using the IncludeOS malloc() which uses all of the solo5 heap. |
@ricarkol Can you point me to the relevant code in the IncludeOS repository that initialises the heap when running on Solo5? |
@mato here it is: https://github.com/hioa-cs/IncludeOS/blob/master/src/platform/x86_solo5/kernel_start.cpp#L72
|
As you can see, using |
I've finally found some time to think about the implications of this change in depth, both from a design and implementation (WIP in #224, mirage/ocaml-solo5#29) point of view. To set the stage: I agree that a high-level memory allocator (as defined by the malloc() family of functions) need not be part of the Solo5 API, and we gain the following benefits by removing it:
However: I think that Solo5 should retain some control over the low-level memory layout and allocation semantics (i.e. how malloc() gets its memory at runtime). Rationale:
To summarize: By all means, let's remove malloc(), but let's keep the "cut point" at the low-level runtime interface for dynamic memory allocation, which should stay in Solo5. If we don't do that then we will shoot ourselves in the foot and be sorry later. Now, the question is, what should this low-level interface look like? I can think of two options:
In the case of (1), given that a) we don't directly expose the notion of "virtual memory" to the application and don't plan to do so in the future, b) we are presenting the abstract concept of a single heap to the application, an mmap()-style interface seems far too over-engineered and would include a bunch of complex housekeeping to deal with multiple memory regions in Solo5. That leaves the 1970s-style sbrk(), or a more modern version of the same concept, i.e. "extend the heap" and "shrink the heap". The main limitation of a sbrk() style interface is that it requires the heap to be contiguous in memory. I don't think this is an issue for us since the current direction with Solo5 is to perform all external resource allocation (which may include mapping "other" memory regions) before passing control to the guest, leaving it with an entirely static system. Unless anyone has strong objections to this, I'm going to re-work #224 and mirage/ocaml-solo5#29 along these lines and see what I get. /cc @djwillia @ricarkol |
I think we are in agreement that Solo5 should retain some control over The only of your points that seems to need an allocation routine seem Up to this point, we've been doing most resource allocation tasks Do you have any unikernels in mind that use a malloc that tries to |
There's also my first point, which is enforcing a stack/heap split. Granted, this is mainly a convenience feature, but if we want to implement proper stack guards (to detect stack overflow correctly at runtime) then we can't just hand off "all the memory" to the libos. It might be sufficient to resolve this by returning the heap boundaries in a solo5_mem_info() call instead of "all the memory".
I'm more concerned about the real-world operational aspects in the long run. While static memory allocation works, we've been glossing over the fact that (excepting platforms which themselves use static allocation such as virtio or muen) under the hood the guest memory pages have to come from somewhere. Memory is still a scarce resource, especially if you look at scenarios where you want to run lots of unikernels at high density, potentially overcommitting your host resources. With the "hands off" approach of "here's a chunk of memory, go use any/all/none of it", we are effectively committing ourselves to pretending that memory is cheap, and ignoring any memory pressure or hidden costs (page tables, kernel memory) on the host. With sbrk() we have at least some control point where we keep the option of changing our memory allocation strategy under the hood in the future without breaking applications.
Most (all?) modern memory allocators will try to give back memory to the host (or "next level below") at some level of granularity. Even dlmalloc (and thus Mirage/Solo5) does so. We're just ignoring it and not communicating that back to the host. Granted, in the vt (ukvm) case we probably don't want to do that as it has other implications for the implementation, but again, I'm not sure, and why rob ourselves of the option to do so in the future? |
Could we have "dynamic memory allocation" implemented in an optional module? What about the following. A unikernel could start with a static heap size, and if the dynamic memory allocation module is present, it could ask for some pages (not necessarily sequential, and not necessarily after the original static heap). And specifically for this PR, we could start by having a |
I think stack/heap splits is a decision that the (upper layer) libOS
I think that overcommitting host memory resources is often a bad idea
I'm not sure I understand; if we leave out Is there a way to do what you need in an optional way, e.g., as a |
An mprotect-like interface might work for this, though it'd still be up to the libOS to ensure that it lays out whatever regions it's trying to protect with enough granularity to support any underlying architectural limits (e.g. huge pages). Also, if such an interface existed I'd be keen on it only ever being able to increase protection, eg.
This is not specifically about
You misunderstand me -- I don't need anything. The current arrangement with malloc works just fine for Mirage/Solo5, so from that point of view this discussion is largely academic as it doesn't change "what can you do with today's code after this change that you couldn't do before". However, given that we are changing things I'm trying to understand the implications for the wider "contract" (formalised or not) between Solo5 and the libOS, and whether or not any of them get us into a situation where "we can't do something with tomorrow's code after this change that we could do before". Now, I think we've covered most of the points and you've managed to convince me that the "hand's off" approach is the best we have at the moment. Let me try and document the new contract:
With that, I think we should dispense with the proposed
...since any non-trivial guest will need this information at start time. Note that unlike the proposed For the Mirage/Solo5 and ocaml-freestanding split I can adapt to this scheme by adding a What do you think? Does this make sense? |
Clean up the API post malloc removal (see discussion in Solo5#223). - Introduce a struct solo5_boot_info, pass a const pointer to a statically allocated copy of it to solo5_app_main(). - Move cmdline to this struct, make it const. - Use arch-independent type uintptr_t for heap_start. - mem_lock_heap() now returns the values to be used for heap_start, heap_size when setting up solo5_boot_info in _start(). - Remove remains of stack_guard_size from mem.c, this is to be handled by the application. - Remove solo5_get_info() / solo5_mem_info() entirely as it's now redundant. Additionally, while cleaning up these code paths make it clear that the semantics of returning from solo5_app_main() are equivalent to calling solo5_exit().
Clean up the API post malloc removal (see discussion in Solo5#223). - Introduce a struct solo5_boot_info, pass a const pointer to a statically allocated copy of it to solo5_app_main(). - Move cmdline to this struct, make it const. - Use arch-independent type uintptr_t for heap_start. - mem_lock_heap() now returns the values to be used for heap_start, heap_size when setting up solo5_boot_info in _start(). - Remove remains of stack_guard_size from mem.c, this is to be handled by the application. - Remove solo5_get_info() / solo5_mem_info() entirely as it's now redundant. Additionally, while cleaning up these code paths make it clear that the semantics of returning from solo5_app_main() are equivalent to calling solo5_exit().
@djwillia I now have the Solo5 side of the above ready in #244. This is based on your work with the additional changes discussed here and rebased to current master. I've also done one more thing, which is changing |
Corresponding And the minimal I'll open PRs for these in the right order (for CI purposes) once this change lands in Solo5. |
Done in mirage/ocaml-solo5#32, mirage/mirage-solo5#26. This is now completed, yay! |
Solo5 currently contains a malloc implementation. In the MirageOS case, it is used by the libOS via ocaml-freestanding. In the IncludeOS case, they have their own malloc, which gets memory from the Solo5 malloc.
I think malloc should be the responsibility of the libOS layer. This has the advantage of removing duplicate functionality (in the IncludeOS case). Also, it removes the malloc family of calls from the solo5 interface (solo5.h) and bringing it closer to alignment with the underlying isolation interface (e.g., the set of ukvm hypercalls).
I've prepared a PR for Solo5 and for ocaml-freestanding, which applies to the MirageOS case. @ricarkol I would like your opinion on whether this will be suitable for IncludeOS as well.
The text was updated successfully, but these errors were encountered: