Message ID | CAMe9rOqPfbjYPgEqmkWEAht+ZzGZk4wikhVCejUuMWtTdQjr8w@mail.gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 19441 invoked by alias); 3 Jun 2017 11:27:52 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 19425 invoked by uid 89); 3 Jun 2017 11:27:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.6 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=mater X-HELO: mail-qk0-f194.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=tq3B4WkakUqRzqplOqYKWFDg6q9fEPKmpSPVaEwRU7Q=; b=ES1NEsSIoNW82GudNFsuBmxjgOKAgb0AAMhKUEW82eFszxiMdNEJLWiTvUmTWcw2Mb 4P5/M0jIjIDQpejy3YVCpaDWCFpxlwxD79K7eVFT2GREz0JbrSKtYVkGINfLYr8+/PqQ RpUPLhq+r341rH2ufNdBM0zFYmrETkfGeqTv1isCxDoWAIqOumlp+VrhuUk5Bw+IY5Zy xz1Foi9n2BkCWaEYoqBf3nTvbbm9w2bL3nxVjtw2upWSfFarrtXLqdh3GN8K29tY4dwE EJu4VFBbB2WqBgjDTO4LfBbhIcrc1kvauWoUCEzNwFpuVOiiLyWGSPyvly5TM2q7aMgC RFCQ== X-Gm-Message-State: AKS2vOylHb/idjAUiKeKbP+xLZb24qxVLw0FRx63eWvIc7B1dOZ19j8t MdYDKPz8wJ2fv+u2h6h7MYUIyo2Cow== X-Received: by 10.55.105.133 with SMTP id e127mr13716673qkc.19.1496489273435; Sat, 03 Jun 2017 04:27:53 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <e6d26a42-f5d2-bada-d44a-42e58b4d1543@redhat.com> References: <1493901791-13438-1-git-send-email-adhemerval.zanella@linaro.org> <1493901791-13438-2-git-send-email-adhemerval.zanella@linaro.org> <8ceeaabf-110f-31fe-516b-18f2fe6710f8@gotplt.org> <7412360d-c776-532b-e22c-dd3f0048cfba@linaro.org> <CAMe9rOpu=yRUr=bHE=h2CJA7e0=U8qz5g5M_PL3GKAtLoUmmJA@mail.gmail.com> <CAKCAbMi95KU=kO1vyj7O4GpHFngOpjDGxCog83nnaYjrRwOF4Q@mail.gmail.com> <7f07b067-a074-d670-88b1-3da917451c71@redhat.com> <b2e4c8cc-0fe7-aa9b-08c8-389e7bb0fa76@linaro.org> <CAKCAbMgAuA0-Zc1n_BwjGW767semHr-ApjUWecCo4h9gJk=unQ@mail.gmail.com> <f9508771-891d-090e-49e8-b4e85a0f6f77@linaro.org> <bca5b48f-9a51-f980-eaee-853602dfca34@redhat.com> <77f9ac84-620a-a408-c448-14a6978f5d2e@linaro.org> <CAMe9rOoRXFH0qeuwpCvLtHJDGn0r6B+KqRbpfK-Um8VAoXF+4g@mail.gmail.com> <m2poelh45i.fsf@linux-m68k.org> <CAMe9rOrRuOKfXFoTm2MKpufn2NUZ39Nxo9RSY7U0Ss0LW=TZ2w@mail.gmail.com> <e6d26a42-f5d2-bada-d44a-42e58b4d1543@redhat.com> From: "H.J. Lu" <hjl.tools@gmail.com> Date: Sat, 3 Jun 2017 04:27:52 -0700 Message-ID: <CAMe9rOqPfbjYPgEqmkWEAht+ZzGZk4wikhVCejUuMWtTdQjr8w@mail.gmail.com> Subject: Re: [PATCH v3 2/2] posix: Implement preadv2 and pwritev2 To: Florian Weimer <fweimer@redhat.com> Cc: Andreas Schwab <schwab@linux-m68k.org>, Adhemerval Zanella <adhemerval.zanella@linaro.org>, Zack Weinberg <zackw@panix.com>, Siddhesh Poyarekar <siddhesh@gotplt.org>, GNU C Library <libc-alpha@sourceware.org> Content-Type: text/plain; charset="UTF-8" |
Commit Message
H.J. Lu
June 3, 2017, 11:27 a.m. UTC
On Sat, Jun 3, 2017 at 4:23 AM, Florian Weimer <fweimer@redhat.com> wrote: > 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? My patch has ndex 7b8bd79..a3fe2fa 100644 For LO_HI_LONG, it doesn't mater what the second one is. It makes no difference if -1 is passed. Why bother with 0?
Comments
On Sat, Jun 3, 2017 at 4:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Sat, Jun 3, 2017 at 4:23 AM, Florian Weimer <fweimer@redhat.com> wrote: >> 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? > > My patch has > > ndex 7b8bd79..a3fe2fa 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > @@ -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 */ > > For LO_HI_LONG, it doesn't mater what the second one is. It makes > no difference if -1 is passed. Why bother with 0? > BTW, LO_HI_LONG_FLAGS is still needed for x32 even if LO_HI_LONG passes the second argument for x86-64.
On Jun 03 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> BTW, LO_HI_LONG_FLAGS is still needed for x32
Why?
Andreas.
On Sat, Jun 3, 2017 at 5:52 AM, Andreas Schwab <schwab@linux-m68k.org> wrote: > On Jun 03 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > >> BTW, LO_HI_LONG_FLAGS is still needed for x32 > > Why? 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 64-bit pos and int flags are passed as 2 arguments to kernel here. It is incorrect to pass "pos, 0, flags" to kernel.
On 06/03/2017 07:27 AM, H.J. Lu wrote: > On Sat, Jun 3, 2017 at 4:23 AM, Florian Weimer <fweimer@redhat.com> wrote: >> 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? > > My patch has > > ndex 7b8bd79..a3fe2fa 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > @@ -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 */ > > For LO_HI_LONG, it doesn't mater what the second one is. It makes > no difference if -1 is passed. Why bother with 0? If nothing else, always passing the unused argument as 0 will reduce confusion when people inspect these syscalls, e.g. through the debugger interface or the seccomp filter interface. I think that's a sufficient reason to do it. These are all I/O syscalls that can block, it's the wrong place to be shaving cycles. zw
On Sat, Jun 3, 2017 at 6:48 AM, Zack Weinberg <zackw@panix.com> wrote: > On 06/03/2017 07:27 AM, H.J. Lu wrote: >> On Sat, Jun 3, 2017 at 4:23 AM, Florian Weimer <fweimer@redhat.com> wrote: >>> 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? >> >> My patch has >> >> ndex 7b8bd79..a3fe2fa 100644 >> --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h >> +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h >> @@ -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 */ >> >> For LO_HI_LONG, it doesn't mater what the second one is. It makes >> no difference if -1 is passed. Why bother with 0? > > If nothing else, always passing the unused argument as 0 will reduce > confusion when people inspect these syscalls, e.g. through the debugger > interface or the seccomp filter interface. I think that's a sufficient > reason to do it. These are all I/O syscalls that can block, it's the > wrong place to be shaving cycles. LO_HI_LONG_FLAGS is still needed for x32, regardless what we do with LO_HI_LONG.
On 06/03/2017 09:52 AM, H.J. Lu wrote: > On Sat, Jun 3, 2017 at 6:48 AM, Zack Weinberg <zackw@panix.com> wrote: >> If nothing else, always passing the unused argument as 0 will reduce >> confusion when people inspect these syscalls, e.g. through the debugger >> interface or the seccomp filter interface. I think that's a sufficient >> reason to do it. These are all I/O syscalls that can block, it's the >> wrong place to be shaving cycles. > > LO_HI_LONG_FLAGS is still needed for x32, regardless what we > do with LO_HI_LONG. But x32 won't get the definition of LO_HI_LONG that expands to "(val), 0)" - that would be wrong - so why won't LO_HI_LONG(val), flags work? zw
On Sat, Jun 3, 2017 at 6:58 AM, Zack Weinberg <zackw@panix.com> wrote: > On 06/03/2017 09:52 AM, H.J. Lu wrote: >> On Sat, Jun 3, 2017 at 6:48 AM, Zack Weinberg <zackw@panix.com> wrote: >>> If nothing else, always passing the unused argument as 0 will reduce >>> confusion when people inspect these syscalls, e.g. through the debugger >>> interface or the seccomp filter interface. I think that's a sufficient >>> reason to do it. These are all I/O syscalls that can block, it's the >>> wrong place to be shaving cycles. >> >> LO_HI_LONG_FLAGS is still needed for x32, regardless what we >> do with LO_HI_LONG. > > But x32 won't get the definition of LO_HI_LONG that expands to "(val), > 0)" - that would be wrong - so why won't LO_HI_LONG(val), flags work? > X32 inherits LO_HI_LONG from x86-64. If we change x86-64 LO_HI_LONG, x32 needs to redefine LO_HI_LONG.
On Jun 03 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > X32 inherits LO_HI_LONG from x86-64. If we change x86-64 > LO_HI_LONG, x32 needs to redefine LO_HI_LONG. Your LO_HI_LONG_FLAGS doesn't work for x32 either. Andreas.
--- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h @@ -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 */