[v3,2/2] posix: Implement preadv2 and pwritev2
Commit Message
On Fri, Jun 2, 2017 at 12:46 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
> On 02/06/2017 16:02, Florian Weimer wrote:
>> On 06/02/2017 08:19 PM, Adhemerval Zanella wrote:
>>> I am even more confident this is indeed a miscompilation from GCC7 branch.
>>> Using a previous built x86_64-linux-gnu with GCC6 branch I saw no issue, however
>>> using GCC7 there is indeed the test failure. And it seems to show only for kernels
>>> with support for p{read,write}v2 syscall, which means it stress the default
>>> SYSCALL_CANCEL path.
>>>
>>> However, running a GCC6 built testcase with a GCC7 build glibc (with testrun.sh)
>>> I saw no issue. The GCC7 built test also fails with a GCC6 built glibc. I will
>>> check with GCC master to see if this is something only on GCC7 branch or if it
>>> is still on master.
>>>
>>> Also, I seems that for GCC7 only x86_64-linux-gnu is affect on x86.
>>
>> I see it with GCC 6.3 on Fedora 24, x86-64 as well. It could be a
>> broken GCC 7 patch that was backported, or perhaps our constraints for
>> the syscall instruction are off.
>
> My GCC 6.3 was built with build-many-glibcs.py, so I assume it contains
> no backports. I just checked with GCC trunk (gcc version 8.0.0 20170602)
> built also with build-many-glibcs.py and it does not trigger the issue.
The kernel interface for p{readv,writev}{64}v is
(unsigned long fd, {const }struct iovec *iov, unsigned long vlen,
unsigned long pos_l, unsigned long pos_h)
The LO_HI_LONG macro is used to pass offset to the pos_l and pos_h pair.
Since pos_h is ignored when size of offset == sizeof of pos_l, x86-64
has
#define LO_HI_LONG(val) (val)
But the kernel interface for p{readv,writev}{64}v2 is
(unsigned long fd, {const }struct iovec *iov, unsigned long vlen,
unsigned long pos_l, unsigned long pos_h, int flags)
Except for targets which define __ARCH_WANT_COMPAT_SYS_PREADV64V2 and
__ARCH_WANT_COMPAT_SYS_PWRITEV64V2,
(unsigned long fd, {const }struct iovec *iov, unsigned long vlen,
unsigned long pos, int flags)
will be used for p{readv,writev}{64}v2. X32 is the only such target.
The x86-64 LO_HI_LONG can't be used for p{readv,writev}{64}v2. Add a
new macro, LO_HI_LONG_FLAGS, to pass the off{64}_t and flags arguments.
Please test it on other aches.
Thanks.
Comments
On Jun 02 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> The x86-64 LO_HI_LONG can't be used for p{readv,writev}{64}v2. Add a
> new macro, LO_HI_LONG_FLAGS, to pass the off{64}_t and flags arguments.
Why can't LO_HI_LONG just pass the padding unconditionally on x86_64?
Andreas.
On Sat, Jun 3, 2017 at 1:22 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Jun 02 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>
>> The x86-64 LO_HI_LONG can't be used for p{readv,writev}{64}v2. Add a
>> new macro, LO_HI_LONG_FLAGS, to pass the off{64}_t and flags arguments.
>
> Why can't LO_HI_LONG just pass the padding unconditionally on x86_64?
>
To avoid the unnecessary (long) (((uint64_t) (val)) >> 32).
On 06/03/2017 01:04 PM, H.J. Lu wrote:
> On Sat, Jun 3, 2017 at 1:22 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> On Jun 02 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>>
>>> The x86-64 LO_HI_LONG can't be used for p{readv,writev}{64}v2. Add a
>>> new macro, LO_HI_LONG_FLAGS, to pass the off{64}_t and flags arguments.
>>
>> Why can't LO_HI_LONG just pass the padding unconditionally on x86_64?
>>
>
> To avoid the unnecessary (long) (((uint64_t) (val)) >> 32).
I think the question is why you can't define it like this:
(val), 0
? Are you concerned about the additional overhead of passing that
unnecessary zero at the end of the parameter list for other system
calls? Or would this result in an observable kernel interface
difference and break stuff?
Thanks,
Florian
From 5750b7e3f971e02e1d5c676484eb10c779c6ac13 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 2 Jun 2017 18:31:16 -0700
Subject: [PATCH] Use LO_HI_LONG_FLAGS for p{readv,writev}{64}v2
The kernel interface for p{readv,writev}{64}v is
(unsigned long fd, {const }struct iovec *iov, unsigned long vlen,
unsigned long pos_l, unsigned long pos_h)
The LO_HI_LONG macro is used to pass offset to the pos_l and pos_h pair.
Since pos_h is ignored when size of offset == sizeof of pos_l, x86-64
has
#define LO_HI_LONG(val) (val)
But the kernel interface for p{readv,writev}{64}v2 is
(unsigned long fd, {const }struct iovec *iov, unsigned long vlen,
unsigned long pos_l, unsigned long pos_h, int flags)
Except for targets which define __ARCH_WANT_COMPAT_SYS_PREADV64V2 and
__ARCH_WANT_COMPAT_SYS_PWRITEV64V2,
(unsigned long fd, {const }struct iovec *iov, unsigned long vlen,
unsigned long pos, int flags)
will be used for p{readv,writev}{64}v2. X32 is the only such target.
The x86-64 LO_HI_LONG can't be used for p{readv,writev}{64}v2. Add a
new macro, LO_HI_LONG_FLAGS, to pass the off{64}_t and flags arguments.
Tested on i686, x32 and x86-64.
* sysdeps/unix/sysv/linux/preadv2.c (preadv2): Replace LO_HI_LONG
with LO_HI_LONG_FLAGS.
* sysdeps/unix/sysv/linux/preadv64v2.c (preadv64v2): Likewise.
* sysdeps/unix/sysv/linux/pwritev2.c (pwritev2): Likewise.
* sysdeps/unix/sysv/linux/pwritev64v2.c (pwritev64v2): Likewise.
* sysdeps/unix/sysv/linux/sysdep.h (LO_HI_LONG_FLAGS): New.
* sysdeps/unix/sysv/linux/x86_64/sysdep.h (LO_HI_LONG_FLAGS):
Likewise.
* sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h (LO_HI_LONG_FLAGS):
Likewise.
---
sysdeps/unix/sysv/linux/preadv2.c | 2 +-
sysdeps/unix/sysv/linux/preadv64v2.c | 2 +-
sysdeps/unix/sysv/linux/pwritev2.c | 2 +-
sysdeps/unix/sysv/linux/pwritev64v2.c | 2 +-
sysdeps/unix/sysv/linux/sysdep.h | 7 +++++++
sysdeps/unix/sysv/linux/x86_64/sysdep.h | 5 +++++
sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 5 +++++
7 files changed, 21 insertions(+), 4 deletions(-)
@@ -31,7 +31,7 @@ preadv2 (int fd, const struct iovec *vector, int count, off_t offset,
{
# ifdef __NR_preadv2
ssize_t result = SYSCALL_CANCEL (preadv2, fd, vector, count,
- LO_HI_LONG (offset), flags);
+ LO_HI_LONG_FLAGS (offset, flags));
if (result >= 0 || errno != ENOSYS)
return result;
# endif
@@ -29,7 +29,7 @@ preadv64v2 (int fd, const struct iovec *vector, int count, off64_t offset,
{
#ifdef __NR_preadv64v2
ssize_t result = SYSCALL_CANCEL (preadv64v2, fd, vector, count,
- LO_HI_LONG (offset), flags);
+ LO_HI_LONG_FLAGS (offset, flags));
if (result >= 0 || errno != ENOSYS)
return result;
#endif
@@ -27,7 +27,7 @@ pwritev2 (int fd, const struct iovec *vector, int count, off_t offset,
{
# ifdef __NR_pwritev2
ssize_t result = SYSCALL_CANCEL (pwritev2, fd, vector, count,
- LO_HI_LONG (offset), flags);
+ LO_HI_LONG_FLAGS (offset, flags));
if (result >= 0 || errno != ENOSYS)
return result;
# endif
@@ -29,7 +29,7 @@ pwritev64v2 (int fd, const struct iovec *vector, int count, off64_t offset,
{
#ifdef __NR_pwritev64v2
ssize_t result = SYSCALL_CANCEL (pwritev64v2, fd, vector, count,
- LO_HI_LONG (offset), flags);
+ LO_HI_LONG_FLAGS (offset, flags));
if (result >= 0 || errno != ENOSYS)
return result;
#endif
@@ -63,6 +63,13 @@
(long) (val), \
(long) (((uint64_t) (val)) >> 32)
+/* Provide a macro to pass the off{64}_t and flags arguments on
+ p{readv,writev}{64}v2. */
+#define LO_HI_LONG_FLAGS(val, flags) \
+ (long) (val), \
+ (long) (((uint64_t) (val)) >> 32), \
+ (int) (flags)
+
/* Exports the __send symbol on send.c linux implementation (some ABI have
it missing due the usage of a old generic version without it). */
#define HAVE_INTERNAL_SEND_SYMBOL 1
@@ -389,4 +389,9 @@
#undef LO_HI_LONG
#define LO_HI_LONG(val) (val)
+/* Provide a macro to pass the off{64}_t and flags arguments on
+ p{readv,writev}{64}v2. */
+#undef LO_HI_LONG_FLAGS
+#define LO_HI_LONG_FLAGS(val, flags) (val), 0, (flags)
+
#endif /* linux/x86_64/sysdep.h */
@@ -22,4 +22,9 @@
#include <sysdeps/unix/sysv/linux/x86_64/sysdep.h>
#include <sysdeps/x86_64/x32/sysdep.h>
+/* Provide a macro to pass the off{64}_t and flags arguments on
+ p{readv,writev}{64}v2. */
+#undef LO_HI_LONG_FLAGS
+#define LO_HI_LONG_FLAGS(val, flags) (val), (flags)
+
#endif /* linux/x86_64/x32/sysdep.h */
--
2.9.4