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

Jump to bottom

syscall, x/sys/unix: wrong order of arguments for SyncFileRange on linux/ppc64{,le} #27485

Closed
tklauser opened this issue Sep 4, 2018 · 4 comments
Closed

syscall, x/sys/unix: wrong order of arguments for SyncFileRange on linux/ppc64{,le} #27485

tklauser opened this issue Sep 4, 2018 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Linux

Comments

@tklauser
Copy link
Member

tklauser commented Sep 4, 2018 •
edited

While investigating what was wrong with https://golang.org/cl/133015 (see #27475), I noticed that the order of the syscall arguments in the SyncFileRange implementation is wrong for ppc64 and ppc64le. They use the SYS_SYNC_FILE_RANGE2 syscall which is defined as:

https://github.com/torvalds/linux/blob/60c1f89241d49bacf71035470684a8d7b4bb46ea/fs/sync.c#L371-L375

while SYS_SYNC_FILE_RANGE used by all other GOARCHes is defined as:

https://github.com/torvalds/linux/blob/60c1f89241d49bacf71035470684a8d7b4bb46ea/fs/sync.c#L363-L367

Note the different position of the flags argument. This should be fixed by using appropriate wrappers on linux/ppc64{,le}.

/cc @bradfitz @ianlancetaylor
@tklauser tklauser added OS-Linux NeedsFix The path to resolution is known, but the work has not been done. labels Sep 4, 2018
@tklauser tklauser self-assigned this Sep 4, 2018
@tklauser
Copy link
Member Author

tklauser commented Sep 4, 2018

Also see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=edd5cd4a9424f22b0fa08bef5e299d41befd5622 for the reasoning for there being two syscalls.

@tklauser
Copy link
Member Author

tklauser commented Sep 4, 2018

The implementation on linux/arm64 is correct, SYS_SYNC_FILE_RANGE and SYS_SYNC_FILE_RANGE2 have the same value and call the sync_file_range syscall.

@tklauser tklauser changed the title syscall, x/sys/unix: wrong order of arguments for SyncFileRange on linux/ppc64{,le} and arm64 syscall, x/sys/unix: wrong order of arguments for SyncFileRange on linux/ppc64{,le} Sep 4, 2018
@gopherbot
Copy link

Change https://golang.org/cl/133195 mentions this issue: syscall: correct argument order for SyncFileRange syscall on linux/ppc64{,le

@gopherbot
Copy link

Change https://golang.org/cl/133215 mentions this issue: unix: correct argument order for SyncFileRange syscall on linux/ppc64{,le}

gopherbot pushed a commit that referenced this issue Sep 5, 2018
@tklauser
syscall: correct argument order for SyncFileRange syscall on linux/pp…
eee1cfb 
…c64{,le}

On linux/ppc64{,le} the SYS_SYNC_FILE_RANGE2 syscall is used to
implement SyncFileRange. This syscall has a different argument order
than SYS_SYNC_FILE_RANGE. Apart from that the implementations of both
syscalls are the same, so use a simple wrapper to invoke the syscall
with the correct argument order.

For context see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=edd5cd4a9424f22b0fa08bef5e299d41befd5622

Updates #27485

Change-Id: Ib94fb98376bf6c879df6f1b68c3bdd11ebcb5a44
Reviewed-on: https://go-review.googlesource.com/133195
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Linux
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@tklauser @gopherbot