-
Notifications
You must be signed in to change notification settings - Fork 139
[PATCH v1] api: barrier: clarify thread barrier reuse #2262
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
base: master
Are you sure you want to change the base?
[PATCH v1] api: barrier: clarify thread barrier reuse #2262
Conversation
Explicitly state that a memory barrier can be reused without initializing it again. Make it clear that no more than N threads may be inside odp_barrier_wait() for a barrier of N threads. Without this requirement, an application could e.g. wait a barrier in thread 1 and thread 2, and, when thread 2 returns from the barrier (at which point thread 1 is supposed to have been unblocked too), tell threads 3 and 4 to wait for the same barrier. If threads 3 and 4 manage to start waiting for the barrier before thread 1 has returned from odp_barrier_wait(), thread 1 may continue to be blocked in the counter based implementation used in multiple ODPs. Clarify that a barrier must be initialized before first use and can be reinitialized if not in use. Make also a remark about the thread-unsafety of the initialization function. Signed-off-by: Janne Peltonen <[email protected]>
/** | ||
* Initialize barrier with thread count. | ||
* | ||
* This function must not be called by multiple threads simultaenously for |
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.
Typo: simultaenously
* If a barrier is used by more threads than the thread count the barrier was | ||
* initialized with, the number of threads that have called odp_barrier_wait() | ||
* but not yet returned from it must never exceed the thread count the barrier | ||
* was initiazed with. |
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.
Typo: initiazed
/** | ||
* Synchronize thread execution on barrier. | ||
* Wait for all threads to arrive at the barrier until they are let loose again. | ||
* Wait for a number of threads to arrive at the barrier until they are |
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.
Could add also a newline to separate the one-line description.
* A barrier can be reused without reinitializing it again. It is ok to call | ||
* odp_barrier_wait() for the same barrier again immediately after returning | ||
* from the previous odp_barrier_wait() even if other threads that were waiting | ||
* for the barrier have not yet returned from odp_barrier_wait(). |
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.
Do we have a validation test already for this? If not, one should be added.
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.
We have multiple tests that rely on this, so in a way we have, but maybe it would be good add an explicit test.
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.
Yep, explicit test seems worth adding.
Explicitly state that a memory barrier can be reused without initializing it again.
Make it clear that no more than N threads may be inside odp_barrier_wait() for a barrier of N threads. Without this requirement, an application could e.g. wait a barrier in thread 1 and thread 2, and, when thread 2 returns from the barrier (at which point thread 1 is supposed to have been unblocked too), tell threads 3 and 4 to wait for the same barrier. If threads 3 and 4 manage to start waiting for the barrier before thread 1 has returned from odp_barrier_wait(), thread 1 may continue to be blocked in the counter based implementation used in multiple ODPs.
Clarify that a barrier must be initialized before first use and can be reinitialized if not in use. Make also a remark about the thread-unsafety of the initialization function.