Basic OpenBSD backend support. #74
Conversation
Cannot enforce alignment so we just check its correctness.
|
|
davidchisnall
left a comment
davidchisnall
left a comment
There was a problem hiding this comment.
There's a lot of code copied and pasted from the FreeBSD PAL here. Please can you pull that out into a common generic BSD PAL? I think that the OpenBSD one will likely be the generic one and FreeBSD is that plus the addition of support for aligned allocation support.
src/pal/pal_openbsd.h
Outdated
| * Bitmap of PalFeatures flags indicating the optional features that this | ||
| * PAL supports. | ||
| */ | ||
| static constexpr uint64_t pal_features = AlignedAllocation | LazyCommit; |
There was a problem hiding this comment.
If you can't guarantee alignment, then don't advertise that you support aligned allocation - the code using the PAL will fix this for you.
There was a problem hiding this comment.
It was
There's a lot of code copied and pasted from the FreeBSD PAL here. Please can you pull that out into a common generic BSD PAL? I think that the OpenBSD one will likely be the generic one and FreeBSD is that plus the addition of support for aligned allocation support.
It was to avoid too many ifdef
There was a problem hiding this comment.
If you can't guarantee alignment, then don't advertise that you support aligned allocation - the code using the PAL will fix this for you.
Yes useless flag indeed.
There was a problem hiding this comment.
I meant pull out the common functions into a shared superclass, not add a mess of ifdefs...
mjp41
left a comment
mjp41
left a comment
There was a problem hiding this comment.
This looks good to me. Thanks for your contribution. @davidchisnall can you double check
| class PALOBSD : public PALBSD | ||
| { | ||
| public: | ||
| static constexpr uint64_t pal_features = LazyCommit; |
There was a problem hiding this comment.
If you include this line in the PALBSD, then we can remove this file, and just use PALBSD for OpenBSD and extend it for FreeBSD.
There was a problem hiding this comment.
Yes I considered this but was not sure if one day OpenBSD would need a particular treatment (ie having special mmap flags nothing else has).
There was a problem hiding this comment.
I think the right thing to do here (from the perspective of making the code readable) is for the BSD PAL to set LazyCommit and for the OpenBSD PAL to declare pal_features but set it to PALBSD::pal_features, so if anyone needs to extend this class with non-generic behaviour later then they have a reminder to do so. I'll make that change when I do some cleanup.
There was a problem hiding this comment.
No problem at all with that, result is the same just make the "inheritance" reminder clearer indeed.
|
Now that the BSD back ends are merged, the Linux one looks quite untidy. I think I will try to refactor some of it into a POSIX PAL today. |
4 participants
Cannot enforce alignment so we just check its correctness.