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
mutex requirements #35
base: main
Are you sure you want to change the base?
mutex requirements #35
Changes from all commits
a5ed580
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.
Remove the While clause. The mechanism needs to always be provided.
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.
"The Zephyr RTOS shall provide a mechanism for the owning thread to unlock a mutex."
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 second with Tims suggestion for the RQT.
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.
How do you verify "ensure"? Can the verification be automated?
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.
yes, we test for this already.
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.
Restate without superfluous infinitives per INCOSE GtWR R10.
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 think this requirement is intended to cover the atomic mutex operation of unlocking and then locking for the next waiting thread.
If so, I suggest:
"When a mutex is unlocked and another thread waiting to lock that mutex, the unlock and lock operations shall be atomic"
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.
You just changed abstraction levels. "the Zephyr RTOS shall" is a different level than "Mutexes shall".
Rephrase to remove "where". Reserve "where" for preconditions.
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.
Seems like the Semaphore requirements covering timeouts should be duplicated for mutexes.
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.
For consistency of language this RQT should also start with "The Zephyr RTOS shall support ..." the more consistent RQTs are written the easier they become to parse (by humans and non-humans alike)
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.
", allowing ..." is descriptive text. Please remove it from the requirement statement.
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.
"The Zephyr RTOS shall support reentrant mutex locking."
Do we depend on common software engineering knowledge (and Google) or do we need more requirements describing the behavior of a entrant mutex.
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 seems like an implementation detail. Is the current lock count exposed in an API?
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 is part of the mutex design in Zephyr and how it works, it is not an
implementation detail
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 is the key behavior of a reentrant mutex. So it is good.
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.
Remove the While clause. The mechanism needs to always be provided.
"shall implement" is not an observable behavior. Are you intentionally trying to constrain the implementation? If so, how do you verify this? Can verification be automated?
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.
please provide a suggestion on the wording. I do not see any other way this can
be expressed. You keep asking about automation of verification, not sure what
you mean by now, this is something that can be tested, yes, why are we talking
about automation? why is this relevant?
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.
Mutexes are only used for resource synchronization, so remove the conditional.
"to prevent priority inversion scenarios" is justification. Remove it (INCOSE GtWR R20).
Priority inheritance is implied by mutex in an RTOS. I think this is a better description for the highest abstraction layers: "The Zephyr shall expose mutex services."
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.
{sigh}
Automated verification helps fulfill a critical User Need: to be able to verify the OSS software with the User-provided qualified toolchain in a timely manner. There are 4 identified methods for verification: Test, Demonstration, Inspection, Analysis. Doing full verification manually takes FAR too long, so product integrators need automated verification suites to come with the OSS. The most natural verification type to automate is Test.
AFAICT the biggest enabler to deploying and updating secure systems is the rapid integration, revalidation, and deployment of security fixes. Product manufacturers soon will need to publicly commit to a specific response time to security issues (e.g., 30d, 60d, 90d). Component providers (e.g., Nordic, Zephyr Project) that commit to rolling out security fixes faster than the product manufacturer commitment remain as viable suppliers. Those that do not lose out on the design win.
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.
Requirements need to be testable, fully agree with that and there is no argument. Automation is key and we need to get to almost 100% automation, sure thing.
Asking if the some requirement can be tested or verified is fine, however asking if "verification can be automated" is off-topic and I did not know what you mean because of that.
This is going off-topic.
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.
"Mutexes shall implement priority inheritance protocols."
Implement does not tell me how priority inheritance behaves. But the next two requirements address priority inheritance specifically.
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.
How is this distinct from the "Priority Inheritance" requirement above? If these are at different levels of detail then they need to reference each other.
Use "While" instead of "When". This condition is a state not an event.
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.
"While a higher-priority thread is waiting on a mutex, the Zephyr RTOS shall elevate the priority of the owning thread."
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.
Remove "to ensure ...". It is justification not observable behavior or condition.
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.
"Mutexes shall track the owning thread when locked"
I think I made this a second part to ZEP-MUTEX-4 Exclusive Locking of a Mutex
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.
Avoid using the word "only" in requirements. It is usually(?) very hard or impossible to verify.
In many cI think this is better phrased as:
While the mutex is not in a recursive lock,
When the thread owning a mutex requests to unlock it,
the ... shall ...
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 think this is covered by my suggested wording for ZEP-MUTEX-5 Unlocking of a Mutex:
"The Zephyr RTOS shall provide a mechanism for the owning thread to unlock a mutex."
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.
Remove the justification phrase "to limit ...".
"shall implement" is not an observable behavior. Are you intentionally trying to constrain the implementation? If so, how do you verify this? Can verification be automated?