propagate additional process loading errors to main.rs #1770
Conversation
kernel/src/process.rs
Outdated
| app_flash.as_ptr() as usize, | ||
| app_flash.as_ptr() as usize + app_flash.len(), | ||
| process_name | ||
| ); | ||
| } | ||
| return Ok((None, 0)); | ||
| return Err(ProcessLoadError::InternalError); |
There was a problem hiding this comment.
I'm not sure making this error InternalError is right. I think we want to save the internal error for only issues where the kernel is wrong. In this case, however, I think an app loaded with a length in flash that the MPU doesn't expect will cause an error, which is maybe arguably a kernel error, but the fix is to correct the app size, not patch the kernel.
Likely a new error would be better, one that indicates it could be the app or a bad MPU implementation.
|
Added a new error type for when the MPU does not support the flash length of an app |
|
These are good changes, but since we're stalled on merging for a bit for 1.5 anyway, maybe this should keep bubbling up these errors and address #1225? |
I think that this does address #1225, at least for the process loading case? These errors are returned from |
|
bors r+ |
|
This PR was included in a batch that successfully built, but then failed to merge into master (it was a non-fast-forward update). It will be automatically retried. |
Pull Request Overview
This pull request modifies
process::create()to return an error in 3 cases where previously it just returned an empty process slot -- if there is an error initializing the process, if there is an error allocating the MPU region for that process's flash, or if there is not enough memory to meet the amount requested by that process (or the minimum amount currently required by the kernel).It adds one new
ProcessLoadErrorvariant:ProcessLoadError::NotEnoughMemory. Failures with allocating an MPU region or initializing a process are treated asInternalError's.This PR fixes #1764 by making it so that failing to load a valid process is no longer a silent failure if the board chooses to handle the error.
Testing Strategy
This pull request was tested by flashing 4 apps, each requesting 8kB of RAM, and observing they work as expected, then flashing 5 apps, each requesting 8kB of RAM, and observing that
load_processes()fails, causing a debug message to be printed to the screen.TODO or Help Wanted
This modifies the existing behavior of
load_processes()so that it returns as soon as any process fails to load, whereas currently it will continue trying to load processes even if one fails for one of the above 3 reasons. I think this behavior is better, especially given that boards can redefineload_processesif they want to, but am open to pushback on this.Documentation Updated
Formatting
make formatall.