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: remove the need for &mut Process #1109

Merged
merged 2 commits into from Jul 11, 2018

Conversation

Projects
None yet
4 participants
@bradjc
Copy link
Contributor

bradjc commented Jul 10, 2018

This switches the Process struct to use Cells rather than mutate itself directly. This has a bit of a size penalty, but makes it possible to remove static buffers in the kernel.

This is pushing towards #1043. To remove the PROCS static array, there is no way (that I know of) to do it without removing the use of mutable &Process in the kernel. This change brings Process inline with other structures in Tock.

Testing Strategy

This pull request was tested by running hail on hail.

TODO or Help Wanted

n/a

Documentation Updated

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

Formatting

  • Ran make formatall.
kernel: remove the need for &mut Process
This switches the `Process` struct to use `Cell`s rather than mutate
itself directly. This has a bit of a size penalty, but makes it possible
to remove static buffers in the kernel.

bradjc added a commit that referenced this pull request Jul 10, 2018

@bradjc bradjc referenced this pull request Jul 10, 2018

Merged

Kernel: Move `PROCS` array to `Kernel` struct #1111

2 of 4 tasks complete

/// Name of the app. Public so that IPC can use it.
pub package_name: &'static str,

/// Values kept so that we can print useful debug messages when apps fault.
debug: ProcessDebug,
debug: MapCell<ProcessDebug>,

This comment has been minimized.

@alevy

alevy Jul 11, 2018

Member

Why make this a MapCell as opposed to leaving the fields as Cells? This seems OK to, but was there any particular logic behind the decision?

This comment has been minimized.

@bradjc

bradjc Jul 11, 2018

Contributor

I thought that was more consistent with Tock, for example in capsules using a cell around an App rather than having every field in an App use cells.

This comment has been minimized.

@alevy

alevy Jul 11, 2018

Member

I think Grants are pretty different as they basically need to be reified into mutable references in order to make them general enough, whereas as this can easily be just a bunch of Cells (which is more consistent with how we write non-Grant capsule code generally).

However, this is a small and contained enough design decision that i don't think it's worth kvetching over. I think using a MapCell here is just fine, was just wondering if there was a strict need for it I was missing.

@ppannuto
Copy link
Member

ppannuto left a comment

Looks good to me minus the one trivial fix. 👍 to delete this review and not wait for me again after that.

self.kernel.increment_work();
let ret = self.tasks.enqueue(Task::IPC((from, cb_type)));
// let ret = self.tasks.enqueue(Task::IPC((from, cb_type)));

This comment has been minimized.

@ppannuto

ppannuto Jul 11, 2018

Member

delete

@bradjc bradjc dismissed stale reviews from alevy and phil-levis via 246be6b Jul 11, 2018

@alevy alevy merged commit 9d910c0 into master Jul 11, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@alevy alevy deleted the process-no-mut branch Jul 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment