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

Scheduling enclaves #242

Merged
merged 63 commits into from
Nov 13, 2023
Merged

Scheduling enclaves #242

merged 63 commits into from
Nov 13, 2023

Conversation

erlingrj
Copy link
Collaborator

@erlingrj erlingrj commented Jun 26, 2023

This PR implements the first version of scheduling enclaves in the C target. This PR is paired with lf-lang/lingua-franca#1872.


Things not supported yet:

  1. Enclaved federations.
  2. Physical connections between enclaves
  3. Ability to specify a maximum lead between enclaves
  4. Smart mutexes wich can catch violations of specified order of acquisition

@erlingrj erlingrj marked this pull request as draft June 26, 2023 13:08
@erlingrj erlingrj changed the title Enclaves2 Scheduling enclaves Jun 28, 2023
@erlingrj
Copy link
Collaborator Author

@edwardalee and @lhstrh. I am tagging you for a review now. There is still a test failure on macOS on a test with coordinated shutdown of enclaves. I will look at this when I get my hands on a MacBook next week

@erlingrj
Copy link
Collaborator Author

The compiler errors now are due the fact that in reactor-c the preprocessor flags for picking the scheduler has changed, from NP -> SCHED_NP and so on. I think that my first step in November should be to craft a minimal lingua-franca PR which can me merged together with this PR. It should not introduce the AST transformation and enable enclaves. But just let us merge these refactorings of the RTI, which I think are good, into master.

Copy link
Contributor

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a review of rti_local.h/c. I don't understand the locking strategy here.

core/federated/RTI/rti_local.h Outdated Show resolved Hide resolved
core/federated/RTI/rti_local.h Outdated Show resolved Hide resolved
core/federated/RTI/rti_local.h Show resolved Hide resolved
core/federated/RTI/rti_local.h Show resolved Hide resolved
core/federated/RTI/rti_local.h Outdated Show resolved Hide resolved
core/federated/RTI/rti_local.h Outdated Show resolved Hide resolved
core/federated/RTI/rti_local.h Outdated Show resolved Hide resolved
core/federated/RTI/rti_local.c Outdated Show resolved Hide resolved
core/federated/RTI/rti_local.c Outdated Show resolved Hide resolved
core/federated/RTI/rti_local.c Outdated Show resolved Hide resolved
@edwardalee
Copy link
Contributor

The compiler errors now are due the fact that in reactor-c the preprocessor flags for picking the scheduler has changed, from NP -> SCHED_NP and so on. I think that my first step in November should be to craft a minimal lingua-franca PR which can me merged together with this PR. It should not introduce the AST transformation and enable enclaves. But just let us merge these refactorings of the RTI, which I think are good, into master.

I have started doing this in lingua-franca/enclaves3, which is pointed to by reactor-c/enclaves3, which includes some small extensions of your reactor-c/enclaves2 branch. Feel free to push directly to either enclave3 branch or to your reactor-c/enclaves2 branch, which we can then merge into reactor-c/enclaves3.

Copy link
Collaborator Author

@erlingrj erlingrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answering questions

core/federated/RTI/rti_local.h Show resolved Hide resolved
core/federated/RTI/rti_local.h Show resolved Hide resolved
core/federated/RTI/rti_local.h Show resolved Hide resolved
core/federated/RTI/rti_local.c Outdated Show resolved Hide resolved
core/federated/RTI/rti_local.c Outdated Show resolved Hide resolved
core/federated/RTI/rti_local.c Outdated Show resolved Hide resolved
Copy link
Contributor

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've finally gone through this whole PR and it looks good to me! Modulo small comments, I think we can merge this as soon as we can get tests to pass in lingua-franca/master with minimal changes to lingua-franca. This will not yet provide support for enclaves, but will get us much closer.

I'm proposing further changes in the enclaves3 branch, but we can review and merge those after merging this into main.

core/federated/RTI/rti_common.h Outdated Show resolved Hide resolved
core/tag.c Outdated Show resolved Hide resolved
include/core/reactor.h Outdated Show resolved Hide resolved
include/core/reactor.h Outdated Show resolved Hide resolved
include/core/reactor.h Outdated Show resolved Hide resolved
@erlingrj erlingrj marked this pull request as ready for review November 12, 2023 17:36
@erlingrj
Copy link
Collaborator Author

@edwardalee. I have addressed all comments. I removed the PROLOGUE/EPILOGUE macros which locked/unlocked mutex. I have made the documentation clearer with respect to which mutex are assumed held etc. Tests are passing here and in LF? Shall we merge?

@erlingrj erlingrj merged commit f68e56a into main Nov 13, 2023
28 checks passed
@lhstrh lhstrh added the feature New feature label Jan 23, 2024
@erlingrj erlingrj deleted the enclaves2 branch February 3, 2024 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants