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

Redox: Fix Condvar.wait(); do not lock mutex twice #8

Merged
merged 1 commit into from Jul 5, 2017

Conversation

Projects
None yet
2 participants
@ids1024
Copy link
Member

ids1024 commented Jul 5, 2017

I think this is the correct solution, but I'm not entirely sure. It does seem fairly clear though that it was wrong previously, since this loop locks the mutex, then mutex_lock is called to lock it again, blocking.

@jackpot51

This comment has been minimized.

Copy link
Member

jackpot51 commented Jul 5, 2017

This looks okay, but I am wondering why the example of futexes has the loop to exchange with 2

This is where I originally got the code: https://locklessinc.com/articles/mutex_cv_futex/

@jackpot51 jackpot51 merged commit 3d587da into redox-os:master Jul 5, 2017

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@ids1024

This comment has been minimized.

Copy link
Member Author

ids1024 commented Jul 5, 2017

That code uses the loop with an exchange of 2 instead of calling mutex_lock, whereas before this PR the Redox code was doing both; I don't know if there's an important reason it uses that loop instead of calling the lock function.

@ids1024

This comment has been minimized.

Copy link
Member Author

ids1024 commented Jul 5, 2017

Actually, looking at that page, I think the call to mutex_lock should be removed, and this loop should be brought back.

Based on this comment there, before the implementation of cond_wait:

The cond_wait operation is slightly tricky. We need to make sure that after we wake up that we change the mutex into the contended state. Thus we cannot use the normal mutex lock function, and have to use an inline version with this constraint.

@jackpot51

This comment has been minimized.

Copy link
Member

jackpot51 commented Jul 5, 2017

Thanks, I must have mistakenly added mutex_lock to the cond_wait function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.