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

kernel: move restart logic to function #1841

Merged
merged 1 commit into from May 14, 2020

Conversation

bradjc
Copy link
Contributor

@bradjc bradjc commented May 8, 2020

Pull Request Overview

This pull request moves the process restart logic to its own function. This is a bit cleaner, but really it structures things more effectively for adding an exit syscall that can also be used to ask the kernel to restart the process.

This is just a small refactor and doesn't change any current behavior.

I also updated some comments to make them more clear.

Testing Strategy

This pull request was tested by running the MPU fault test app and verifying that the app restarts several times on hail before the kernel panics.

TODO or Help Wanted

n/a

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make formatall.

@bradjc bradjc added the kernel label May 8, 2020
///
/// After `restart()` runs the process will either be queued to run its
/// `_start` function, or it will be left in `failure_state`.
fn restart(&self, failure_state: State) {
Copy link
Member

Choose a reason for hiding this comment

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

Since restarting can fail, should the function return some indication of success or failure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. I have no idea what the caller could do with that information, but it might be useful to know what happened and potentially report it back for informative purposes.

But, I think it's better if this PR doesn't change any behavior, just re-organizes code. Actual changes can be part of a different patch.

Copy link
Member

Choose a reason for hiding this comment

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

I think if these was a pub I'd push back, but given that it's contained, this is fine for now.

@ppannuto
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented May 13, 2020

Timed out.

@hudson-ayers
Copy link
Contributor

bors retry

@bors
Copy link
Contributor

bors bot commented May 14, 2020

Timed out.

@ppannuto ppannuto merged commit 84e2417 into master May 14, 2020
@bors bors bot deleted the kernel-process-restart-app-function branch May 14, 2020 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants