Skip to content
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 pthread_barrier event #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bjackman
Copy link
Contributor

@bjackman bjackman commented Aug 7, 2017

This is just like the barrier event except it actually uses
pthread_barrier_wait instead of pthread_cond_wait and
pthread_cond_broadcast. This alternative method may cause issues with
killing rt-app, but is still desirable because it exercises different
syscall usage patterns, which are used in real workloads. I have
tried to explain the details of this in tutorial.txt

Implementing this requires adding a finalize step to resource
setup, because we need to know the number of threads that refer to
the resource before we can call pthread_barrier_init.

@bjackman bjackman force-pushed the pthread-barrier branch 7 times, most recently from 11d2f7c to fc23d7e Compare August 7, 2017 13:58
@bjackman
Copy link
Contributor Author

bjackman commented Aug 7, 2017

Internally, @derkling suggested that the name for the existing event be changed, I guess 'cond_barrier' would be a natural name, but @deggeman pointed out that 'barrier' is in v1.0 so it can't be removed, but we could deprecate the name 'barrier', what do you think @vingu-linaro and @jlelli ?

@bjackman
Copy link
Contributor Author

bjackman commented Aug 8, 2017

Oh also cc: @credp who did the 'barrier' event

This is just like the barrier event except it actually uses
pthread_barrier_wait instead of pthread_cond_wait and
pthread_cond_broadcast. This alternative method may cause issues with
killing rt-app, but is still desirable because it exercises different
syscall usage patterns, which are used in real workloads. I have
tried to explain the details of this in tutorial.txt

Implementing this requires adding a `finalize` step to resource
setup, because we need to know the number of threads that refer to
the resource before we can call pthread_barrier_init.
@bjackman
Copy link
Contributor Author

bjackman commented Nov 2, 2017

Ping?

@bjackman
Copy link
Contributor Author

bjackman commented Nov 9, 2017

Just spoke with @vingu-linaro, he said that this would be better implemented using a single event rather than having two separate events - at least in the JSON grammar (even if it is implemented internally as a separate event).

This is basically all about testing this kernel patch, which has not generated any interest from maintainers and since I am leaving Arm soon, has no immediate future.

Therefore it looks like this PR will hang around for a while longer.

@vingu-linaro
Copy link
Member

Let if someone else can take over your patchset to do the changes

@credp
Copy link
Contributor

credp commented May 22, 2018

I am in favour of keeping this PR open but I don't have time to pick it up myself right now.
We might want to come back to the kernel wakeup changes at some point and even if we don't I think having a way to use a specific variant of the kernel wake events is useful for tools like rt-app.

@jlelli
Copy link
Contributor

jlelli commented May 22, 2018

I am in favour of keeping this PR open

+1

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

Successfully merging this pull request may close these issues.

4 participants