From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756618AbcEaWAl (ORCPT ); Tue, 31 May 2016 18:00:41 -0400 Received: from smtp.gentoo.org ([140.211.166.183]:40152 "EHLO smtp.gentoo.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750813AbcEaWAk (ORCPT ); Tue, 31 May 2016 18:00:40 -0400 Date: Tue, 31 May 2016 18:00:37 -0400 From: Mike Frysinger To: Chris Metcalf Cc: Yury Norov , cmetcalf@tilera.com, arnd@arndb.de, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, libc-alpha@sourceware.org, pinskia@gmail.com Subject: Re: [glibc] preadv/pwritev question Message-ID: <20160531220037.GB7717@vapier.lan> Mail-Followup-To: Chris Metcalf , Yury Norov , cmetcalf@tilera.com, arnd@arndb.de, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, libc-alpha@sourceware.org, pinskia@gmail.com References: <20160531200454.GA32677@yury-N73SV> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="8GpibOaaTibBMecb" Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --8GpibOaaTibBMecb Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 31 May 2016 17:00, Chris Metcalf wrote: > On 5/31/2016 4:04 PM, Yury Norov wrote: > > In path a63c7fa18a (Add sysdeps/unix/sysv/linux/generic/.) you add > > this: > > +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/preadv.c > > > > [...] > > > > +static ssize_t > > +do_preadv (int fd, const struct iovec *vector, int count, off_t > > offset) > > +{ > > + assert (sizeof (offset) =3D=3D 4); > > + return INLINE_SYSCALL (preadv, __ALIGNMENT_COUNT (5, 6), fd, > > + vector, count, __ALIGNMENT_ARG > > + __LONG_LONG_PAIR (offset >> 31, offset)); > > +} > > + > > > > And this is the code that is picked up if I choose wordsize-32 for my > > AARCH64/ILP32. So I have questions. > > > > 1. What is the assert for? We agreed that all new ABIs will be 64-bit > > off_t only. > > > > I fixed it internally like this: > > +#ifndef __OFF_T_MATCHES_OFF64_T > > assert (sizeof (offset) =3D=3D 4); > > +#endif > > > > There is a bunch of similar assertions in glibc. > > > > 2. This one looks weird: > > __LONG_LONG_PAIR (offset >> 31, offset)) > > Why 31-bit offset? And why you don't mask 2nd argument? > > Later in your patch I see this: > > +static ssize_t > > +do_preadv64 (int fd, const struct iovec *vector, int count, off64_t > > offset) > > > > +{ > > + return INLINE_SYSCALL (preadv, __ALIGNMENT_COUNT (5, 6), fd, > > + vector, count, __ALIGNMENT_ARG > > + __LONG_LONG_PAIR ((off_t) (offset >> 32), > > + (off_t) (offset & 0xfffffff= f))); > > +} > > > > And it looks correct to me. If 1st version is correct as well, I think > > it should be commented. >=20 > I did this work before x32 came out, so I tried to model it more closely = on > the existing x86 compat API. I agree that a 64-bit off_t model seems rea= sonable; > however, the code does exactly what it does to match x86, namely preadv()= takes > a 32-bit offset, and preadv64() take a 64-bit offset. The assert() in pr= eadv to force > sizeof to be 4 is exactly why in that routine we use (offset >> 31, offse= t). Since > we know offset fits in 32 bits, all we need to do is properly sign-extend= it into > 64 bits in the high register of the pair, which is what (offset >> 31) do= es - you end > up with only 0 or -1, thus sign-extending the 32-bit signed off_t. Then in > preadv64() we actually need to break apart the 64-bit offset into a high = 32 bits > and a low 32 bits, which is what (offset >> 32, offset & 0xffffffff) does. >=20 > For a 64-bit off_t you will want to not compile preadv.c at all, and inst= ead make > __libc_preadv() and friends be aliases of __libc_preadv64(). sounds like Adhemerval's pread/pwrite unify work should be extended to the preadv/pwritev funcs. it deals with the ilp32 case and uses the new SYSCALL_LL macro to deal with the ugly shifting/masking. check out these commits: https://sourceware.org/git/?p=3Dglibc.git;a=3Dcommit;h=3D071af4769fcdfe2cd3= 49157b01f27c9571478ace https://sourceware.org/git/?p=3Dglibc.git;a=3Dcommit;h=3D77a4fbd53611720cd6= ae845de560df5dd332b28e https://sourceware.org/git/?p=3Dglibc.git;a=3Dcommit;h=3Deeddfa91cbb1a619af= 135c7a9ac14251ec094b7a -mike --8GpibOaaTibBMecb Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXTgmEAAoJEEFjO5/oN/WBuM8QALYCiE/27AQA+2ITHC/tEiey v0njBP1CLIFjgpN8hDxb/p3zGtd/KtEMwrVubbG2Ktrg1TAlVVOqGwF2mbkGGz1B Q+XjjV+CN1FWxGPwZUZGRFCVPXLhQF3fEUEfMTK9CY/MywBazGnZkFu+uejp9XCs JhqkptbBcnlT+H2z6i8BaTV1ZEDm2mqKOpfkDoIYqwBZnK2/QFSgarMEX5FJx/EY SFCdgissTkuML3ayXhaxIJlXJmJ7Sh97sef0tu4eSkyhSY/1VD876hB627srHDXm 6kmF9/TEGEZ/iv4Q23T9iHcPAnlt1wdUf3mW5aKCnt/zIL0DYl6YS1eX/CyNI9Y4 HFN+YgcnmN/ekdZCOj/g0DI+vNuyAzv3uj4i2/DAE5xWDEKKs7y3FhfgfVkFH5Eo UJALy6D06tL20E8vpctl8UdfSozCXkeIdbaYdiuCxIDVN8Zf+dj1SVqPbfYx+Nnl a5Ti9UHwh2BoW8ZZC01z0hbZXs/1/ApYwwlBIILe40l3FfIanWkyYcI/lTHiUzQn C+gFCcNhOS0abDIYw+Rrc2pGQBxeVsCH++FTKcCSamm9hAbes7An5+xusHDjueT2 jOcA5uJkNsxikQXUT2VJsievDCJZ7sQ9w25f91YQlBIWVsWtfjfqCah1xO/Y3Aj6 ZOnbPY+9brmeQGHsL02J =kkDo -----END PGP SIGNATURE----- --8GpibOaaTibBMecb--