Change sync_file_range to be non-cancellable
Commit Message
This patch changes the linux specific sync_file_range syscalls to be
non-cancellable. The rationale is:
1. This is a Linux specific syscall that is not mentioned in POSIX
cancellable entrypoints [1] and the standard states and implementation
shall not introduce cancellation points into any other functions
specified.
2. For mips it requires 7 arguments, which will make the new
cancellation code require very specific handling for the platform.
3. It aligns with other implementations (musl) which does not set
sync_file_range as cancellable.
Also since GLIBC requires a minimum 2.6.32 kernel, the patch also
cleanups the mips code to assume __NR_sync_file_range and the
powerpc one to either assume __NR_sync_file_range2 or __NR_sync_file_range.
Checked on x86_64, i386, powerpc64le and build for mips (ABIO32, ABIN32,
and ABI64).
[1] http://pubs.opengroup.org/onlinepubs/9699919799/
* sysdeps/unix/sysv/linux/powerpc/powerpc64/sync_file_range.c
(__NR_sync_file_range2): Assume it is always defined.
(sync_file_range): Make syscall non-cancellable.
* sysdeps/unix/sysv/linux/mips/mips32/sync_file_range.c
(__NR_sync_file_range): Assume it is always defined.
(sync_file_range): Make syscall non-cancellable.
* sysdeps/unix/sysv/linux/mips/mips64/n32/syscalls.list
(sync_file_range): Likewise.
* sysdeps/unix/sysv/linux/mips/mips64/n64/syscalls.list
(sync_file_range): Likewise.
* sysdeps/unix/sysv/linux/sync_file_range.c (sync_file_range):
Likewise.
* sysdeps/unix/sysv/linux/wordsize-64/syscalls.list (sync_file_range):
Likewise.
---
ChangeLog | 15 +++++++++++++++
sysdeps/unix/sysv/linux/mips/mips32/sync_file_range.c | 13 +------------
sysdeps/unix/sysv/linux/mips/mips64/n32/syscalls.list | 2 +-
sysdeps/unix/sysv/linux/mips/mips64/n64/syscalls.list | 2 +-
.../unix/sysv/linux/powerpc/powerpc64/sync_file_range.c | 13 +------------
sysdeps/unix/sysv/linux/sync_file_range.c | 4 ++--
sysdeps/unix/sysv/linux/wordsize-64/syscalls.list | 2 +-
7 files changed, 22 insertions(+), 29 deletions(-)
Comments
On 10/21/2015 02:34 PM, Adhemerval Zanella wrote:
> This patch changes the linux specific sync_file_range syscalls to be
> non-cancellable. The rationale is:
>
> 1. This is a Linux specific syscall that is not mentioned in POSIX
> cancellable entrypoints [1] and the standard states and implementation
> shall not introduce cancellation points into any other functions
> specified.
It says that we must not make any function mentioned in POSIX
cancellable. We can certainly introduce our own functions which are
cancellable and not port of POSIX.
I think sync_file_range should be cancellable for consistency with fsync
and fdatasync.
Florian
On 21/10/15 14:24, Florian Weimer wrote:
> On 10/21/2015 02:34 PM, Adhemerval Zanella wrote:
>> This patch changes the linux specific sync_file_range syscalls to be
>> non-cancellable. The rationale is:
>>
>> 1. This is a Linux specific syscall that is not mentioned in POSIX
>> cancellable entrypoints [1] and the standard states and implementation
>> shall not introduce cancellation points into any other functions
>> specified.
>
> It says that we must not make any function mentioned in POSIX
> cancellable. We can certainly introduce our own functions which are
> cancellable and not port of POSIX.
>
> I think sync_file_range should be cancellable for consistency with fsync
> and fdatasync.
>
this is something musl might want too.
one ugliness is that cancellation is either guaranteed
to be acted upon or guaranteed not to, which is bad
for all cases when there is a fast path in libc that
avoids calling the blocking syscall.
e.g. sync_file_range is not a cancellation point now
if it returns ENOSYS in glibc.
> Florian
>
On 21 Oct 2015 10:34, Adhemerval Zanella wrote:
> Also since GLIBC requires a minimum 2.6.32 kernel, the patch also
> cleanups the mips code to assume __NR_sync_file_range and the
> powerpc one to either assume __NR_sync_file_range2 or __NR_sync_file_range.
unless i'm missing something, this is orthogonal to cancellation, so it
should be a sep patch/commit. it would also allow for quick merging as
it wouldn't get caught up in the cancellation debate.
-mike
> Em 21 de out de 2015, às 12:46, Mike Frysinger <vapier@gentoo.org> escreveu:
>
>> On 21 Oct 2015 10:34, Adhemerval Zanella wrote:
>> Also since GLIBC requires a minimum 2.6.32 kernel, the patch also
>> cleanups the mips code to assume __NR_sync_file_range and the
>> powerpc one to either assume __NR_sync_file_range2 or __NR_sync_file_range.
>
> unless i'm missing something, this is orthogonal to cancellation, so it
> should be a sep patch/commit. it would also allow for quick merging as
> it wouldn't get caught up in the cancellation debate.
> -mike
Yes and also I withdrawn the cancellation changes. I will resend a patch with cleanup for syscall definitions.
@@ -23,22 +23,11 @@
#include <sysdep-cancel.h>
#include <sys/syscall.h>
-
-#ifdef __NR_sync_file_range
int
sync_file_range (int fd, __off64_t from, __off64_t to, unsigned int flags)
{
- return SYSCALL_CANCEL (sync_file_range, fd, 0,
+ return INLINE_SYSCALL (sync_file_range, 7, fd, 0,
__LONG_LONG_PAIR ((long) (from >> 32), (long) from),
__LONG_LONG_PAIR ((long) (to >> 32), (long) to),
flags);
}
-#else
-int
-sync_file_range (int fd, __off64_t from, __off64_t to, unsigned int flags)
-{
- __set_errno (ENOSYS);
- return -1;
-}
-stub_warning (sync_file_range)
-#endif
@@ -1,7 +1,7 @@
# File name Caller Syscall name # args Strong name Weak names
readahead - readahead i:iii __readahead readahead
-sync_file_range - sync_file_range Ci:iiii sync_file_range
+sync_file_range - sync_file_range i:iiii sync_file_range
prlimit64 EXTRA prlimit64 i:iipp prlimit64
@@ -1,6 +1,6 @@
# File name Caller Syscall name # args Strong name Weak names
-sync_file_range - sync_file_range Ci:iiii sync_file_range
+sync_file_range - sync_file_range i:iiii sync_file_range
prlimit EXTRA prlimit64 i:iipp prlimit prlimit64
@@ -23,19 +23,8 @@
#include <sysdep-cancel.h>
#include <sys/syscall.h>
-
-#if defined __NR_sync_file_range2
-int
-sync_file_range (int fd, __off64_t from, __off64_t to, unsigned int flags)
-{
- return SYSCALL_CANCEL (sync_file_range2, fd, flags, from, to);
-}
-#else
int
sync_file_range (int fd, __off64_t from, __off64_t to, unsigned int flags)
{
- __set_errno (ENOSYS);
- return -1;
+ return INLINE_SYSCALL (sync_file_range2, 4, fd, flags, from, to);
}
-stub_warning (sync_file_range)
-#endif
@@ -28,7 +28,7 @@
int
sync_file_range (int fd, __off64_t from, __off64_t to, unsigned int flags)
{
- return SYSCALL_CANCEL (sync_file_range, fd,
+ return INLINE_SYSCALL (sync_file_range, 6, fd,
__LONG_LONG_PAIR ((long) (from >> 32), (long) from),
__LONG_LONG_PAIR ((long) (to >> 32), (long) to),
flags);
@@ -37,7 +37,7 @@ sync_file_range (int fd, __off64_t from, __off64_t to, unsigned int flags)
int
sync_file_range (int fd, __off64_t from, __off64_t to, unsigned int flags)
{
- return SYSCALL_CANCEL (sync_file_range2, fd, flags,
+ return INLINE_SYSCALL (sync_file_range2, 6, fd, flags,
__LONG_LONG_PAIR ((long) (from >> 32), (long) from),
__LONG_LONG_PAIR ((long) (to >> 32), (long) to));
}
@@ -14,7 +14,7 @@ getrlimit - getrlimit i:ip __getrlimit getrlimit getrlimit64 __getrlimit64
setrlimit - setrlimit i:ip __setrlimit setrlimit setrlimit64
readahead - readahead i:iii __readahead readahead
sendfile - sendfile i:iipi sendfile sendfile64
-sync_file_range - sync_file_range Ci:iiii sync_file_range
+sync_file_range - sync_file_range i:iiii sync_file_range
creat - creat Ci:si creat creat64
open - open Ci:siv __libc_open __open open __open64 open64
prlimit EXTRA prlimit64 i:iipp prlimit prlimit64