Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[WIP] OCaml 5.0 support #122
[WIP] OCaml 5.0 support #122
Changes from 28 commits
de0299f
2b31cbb
cad5c3b
3f473c7
563fd92
665e955
fa430fb
82205a7
f147b12
92fc584
d6b9bcb
6b06b3f
1d5f77c
97671f2
aa1ac8f
ad01905
2b65ad2
5b378ed
f91352a
8017dfd
c235799
401371d
0c2ec55
59621fd
f154280
5dc13d7
840db53
6440f4c
4f795f2
c3e4833
9a1f248
02c2010
2f3f627
1b69cbb
02407fb
33da6cf
22cacf0
a3fd322
6e799a9
2b26368
8f5bd96
738e62b
72eb013
8a23cdf
d3e6e33
f118992
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be easier to just call into the compiler atomics (
__atomic_bla
) no ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmhmm, from a discussion with @Engil and @TheLortex, I suspected a call of
mmap
which should be replaced bymalloc
. Can you compile and run a program with your version of OCaml 5?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have to be careful, mmap -> malloc on MAP_ANONYMOUS probabably is ok, on !MAP_ANONYMOUS we should fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why you
STUB_ABORT
these functions when you defined them intoatomic.h
(as macros) and when, in anyway, the compiler should be able to provide to us implementations of them. A first solution is to define these symbols as functions and use your macros inside. But, as @haesbaert, pointed out, the compiler should give to you the implementation of them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could probably be STUB_IGNORE, we'll never have signals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed it can be
STUB_IGNORE
, during the small experiences I've done so far it doesn't seems to be an issue as I never saw any abort exception raised. Could it worth to keep it asSTUB_ABORT
and wait for the time an unikernel tells us when we need to add signal to solo5? :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be something like calling
solo5_yield(deadline, emptyset)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, we currently can
sleep
with Solo5 since we have the monotonic clock (it's just matter that we will block the only CPU we have).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is one:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh forgot to store
errno
on the error cases, fixed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I agree it should be added, but as for
sigfillset
, if no exception raised, maybe it's not needed?