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

RISC-V: Use v2 syscalls when appropriate #2

Open
wants to merge 1 commit into
base: riscv-musl-1.1.20
from

Conversation

Projects
None yet
3 participants
@ddevault
Copy link
Collaborator

ddevault commented Dec 9, 2018

The RISC-V port does not support any legacy syscalls.

@ddevault ddevault force-pushed the ddevault:riscv-musl-1.1.20 branch from bdb75f6 to 6d247cb Dec 15, 2018

RISC-V: Use v2 syscalls when appropriate
The RISC-V port does not support any legacy syscalls.

@ddevault ddevault force-pushed the ddevault:riscv-musl-1.1.20 branch from 6d247cb to cd95681 Dec 15, 2018

@@ -6,7 +6,11 @@ int rename(const char *old, const char *new)
{
#ifdef SYS_rename
return syscall(SYS_rename, old, new);
#else
#ifdef SYS_renameat2

This comment has been minimized.

@Alcaro

Alcaro Dec 20, 2018

Possible typo: This one uses the new syscall if both old and new exist, but the mmap/renameat changes prefer the old one.

(I'm not a musl or riscv maintainer, I'm just a random nobody who found this PR through your blog https://drewdevault.com/2018/12/20/Porting-Alpine-Linux-to-RISC-V.html. Hi.)

This comment has been minimized.

@ddevault

ddevault Dec 20, 2018

Collaborator

Good eye. I took the more conservative approach with this one based on the ifdefs which were already here - they prefer rename over renameat even though the latter is newer/more flexible.

This comment has been minimized.

@pstoll

pstoll Dec 22, 2018

worth leaving a comment in the code here to that effect? Anytime something is “surprising” (as the commenter was) usually qualifies IMO.

michaeljclark pushed a commit to anarch128/riscv-musl that referenced this pull request Dec 21, 2018

fix race condition in file locking
The condition occurs when
- thread riscv#1 is holding the lock
- thread riscv#2 is waiting for it on __futexwait
- thread riscv#1 is about to release the lock and performs a_swap
- thread riscv#3 enters the __lockfile function and manages to grab the lock
  before thread riscv#1 calls __wake, resetting the MAYBE_WAITERS flag
- thread riscv#1 calls __wake
- thread riscv#2 wakes up but goes again to __futexwait as the lock is
  held by thread riscv#3
- thread riscv#3 releases the lock but does not call __wake as the
  MAYBE_WAITERS flag is not set

This condition results in thread riscv#2 not being woken up. This patch fixes
the problem by making the woken up thread ensure that the flag is
properly set before going to sleep again.

Mainainer's note: This fixes a regression introduced in commit
c21f750.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment