-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add condition variables requirements #77
base: main
Are you sure you want to change the base?
Conversation
docs/system_requirements/index.sdoc
Outdated
COMPONENT: Condition Variables | ||
TITLE: Condition Variables | ||
STATEMENT: >>> | ||
The Zephyr RTOS shall provide a framework to synchronize threads based on a 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.
praise:
The Zephyr RTOS shall provide a framework to synchronize threads based on a condition. | |
The Zephyr RTOS shall provide a framework to synchronize threads based on a condition variable. |
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.
Added
COMPONENT: Condition Variables | ||
TITLE: Signal one waiting thread | ||
STATEMENT: >>> | ||
The Zephyr RTOS shall provide a mechanism to signal one waiting thread when a condition is met. |
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.
question: Do we need to specify which of the potentially many threads that might be waiting this is? Like the first or last or highest prio thread that was added to the waiting list? As per the implementation it is the thread that was added first.
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 we don't need to specify it here because it is a design/implementation decision like you said it could have also been the last one. Or do you think I should change it?
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 did go back to the implementation and as per the implementation it would always be the highest prio thread waiting for the cond var. IMHO we could make that statement explicit here.
COMPONENT: Condition Variables | ||
TITLE: Unblock a waiting thread | ||
STATEMENT: >>> | ||
A waiting thread shall only be unblocked if a signal is sent to the condition variable. |
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.
question: What about timeouts? The API allows to specify a timeout after which the thread will also unblocked again.
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.
Will add that requirements
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.
Added a two reqs for the timeout
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 saw the timeout requirements thanks for that, now in order to avoid inconcistencies I think it makes sense to remove the qualifier 'only' as there is no another mechanism, namely the timeout that does the same.
COMPONENT: Condition Variables | ||
TITLE: Release mutex on wait | ||
STATEMENT: >>> | ||
While waiting on a condition variable the thread shall release the current owned 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.
note: this is not strictly needed for the condition variable but to protect the resource from which the condition is calculated, ie. that mutex is actually shared between the threads waiting for the cond var and the threads signaling the the cond var.
As it is written it might be misleading for some to think that it is the thread's responsibility to release the mutex while it actually happens inside the implementation of the wait function.
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.
Changed the description to be less missleading
COMPONENT: Condition Variables | ||
TITLE: Wait timeout on a condition variable | ||
STATEMENT: >>> | ||
When waiting on a condition variable, a timeout value shall be specified. |
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 requirement duplicates the previous requirement. It should be removed.
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.
Removed duplicate.
Hey the document moved to the official documentation under the following link https://docs.zephyrproject.org/latest/safety/safety_requirements.html |
Thank you, that's what I was looking for. |
COMPONENT: Condition Variables | ||
TITLE: Wait timeout on a condition variable | ||
STATEMENT: >>> | ||
When waiting on a condition variable, a timeout value shall be specified. |
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.
It is not clear "who" is specifying the timeout.
When waiting on a condition variable, a timeout value shall be specified. | |
When waiting on a condition variable, the thread shall specify a timeout value. |
COMPONENT: Condition Variables | ||
TITLE: Unblock a waiting thread | ||
STATEMENT: >>> | ||
A waiting thread shall only be unblocked if a signal is sent to the condition variable. |
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 requirement should be explicit about what is unblocking the thread.
Also, how important is the word "only"? Can it be removed?
A waiting thread shall only be unblocked if a signal is sent to the condition variable. | |
The Zephyr RTOS shall unblock a waiting thread if a signal is sent to the condition variable. |
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.
Not sure if this is a duplicate of 21-3? And if so, it would be not some arbitrary thread that gets unblocked but the highest prio thread currently waiting for the cond var. Another point is about 'sending signals'. IMHO there is one thread signaling a cond variable that leads to unblocking the highest prio waiting threads by means of the Zephyr RTOS. So perhaps something akin to: "Whenever some thread signals a condition variable the Zephyr RTOS shall unblock the highest priority thread currently waiting for this condition variable."
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.
Not sure if this is a duplicate of 21-3? And if so, it would be not some arbitrary thread that gets unblocked but the highest prio thread currently waiting for the cond var. Another point is about 'sending signals'. IMHO there is one thread signaling a cond variable that leads to unblocking the highest prio waiting threads by means of the Zephyr RTOS. So perhaps something akin to: "Whenever some thread signals a condition variable the Zephyr RTOS shall unblock the highest priority thread currently waiting for this condition variable."
adapted your proposals. It seems like a duplicate at first but the 21-3 specify the signal interface and this one specify the unblocking reaction if a signal appears.
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.
no additional comments from my side, ok for me when the other comments are resolved.
Add new section condition variables to the system requirements. Add new system requirement for condition variables. Signed-off-by: Simon Hein <[email protected]>
Add new software requirements for condition variables and update the index.sdoc file to include the new requirements file for the software requirements. Signed-off-by: Simon Hein <[email protected]>
@tobiaskaestner The force push is only a rebase to the main branch |
Add condition variables requirements on system and software level.
Closes #72