This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Use SYSCALL_LL[64] to pass 64-bit value [BZ #20349]


On 7/11/2016 5:04 PM, H.J. Lu wrote:
On Mon, Jul 11, 2016 at 1:36 PM, Chris Metcalf <cmetcalf@mellanox.com> wrote:
On 7/11/2016 3:26 PM, H.J. Lu wrote:
SYSCALL_LL/SYSCALL_LL64 should be used to pass 64-bit value to system
calls.

What problem are you trying to solve?

SYSCALL_LL and LO_HI_LONG are different on big-endian systems.  In this
case, LO_HI_LONG is correct, since the kernel API is "unsigned long pos_l,
unsigned long pos_h".  SYSCALL_LL won't do the right thing.

__ALIGNMENT_ARG introduces an extra dummy arguments to be inserted before
a 64-bit value split into two 32-bit registers, if required by the platform.
Since preadv/pwritev explicitly use split arguments to construct a 64-bit
loff_t, __ALIGNMENT_ARG will just add a random inappropriate dummy arg
to an API that doesn't need one.

I reviewed the casting for LO_HI_LONG and it looks OK; I initially was
wondering whether losing the "val >> 31, val" semantics from SYSCALL_LL()
might have been problematic, but it seems like LO_HI_LONG should generate
the same results.

/* Provide a macro to pass the off{64}_t argument on p{readv,writev}{64}.  */
#define LO_HI_LONG(val) \
  (long) (val), \
  (long) (((uint64_t) (val)) >> 32)

is wrong for __ASSUME_WORDSIZE64_ILP32 platform.

On 64-bit platforms, the default definition ends up generating a garbage
32-bit shifted "hi" arg that the kernel then discards.  So I think we
should just use a simple #ifdef _LP64 to avoid the shift, which would
be helpful for all the 64-bit platforms.

But while we're at it, I suspect we should actually pass a constant zero as
the "hi" value.  The kernel discards it, but it's not immediately obvious,
and specifying a zero here seems like good conservative programming.  In
addition, if at some point we want to use the preadv2 syscall ABI, I can
pretty much guarantee we will have to re-discover all this again because
someone will just write "LO_HI_LONG(val), flags", and be baffled as to why
the flags word is ignored on some platforms.  (And honestly, something
called LO_HI_LONG *not* producing a lo and a hi value is inherently odd.)

So I think this makes the most sense:

#ifdef _LP64
# define LO_HI_LONG(val) (val), 0
#else
# define LO_HI_LONG(val) \
 (long) (val), \
 (long) (((uint64_t) (val)) >> 32)
#endif

And then x32-like platforms that want to pass a single 64-bit loff_t
register value can override, as HJ has already done for x86_64.  It's
reasonable to consider this as the exceptional case since the kernel has
the standard version of the syscall as taking two arguments, and x32
is the only kernel architecture using the compat_sys_preadv64
implementation that only takes one loff_t argument.

This does point out that the __ASSUME_WORDSIZE64_ILP32 symbol really
isn't as general as one might like, since it really depends on which
32-bit ABI compat functions the kernel is using for each syscall.

--
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]