From patchwork Mon Oct 19 17:49:17 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 9230 Received: (qmail 54557 invoked by alias); 19 Oct 2015 17:49:21 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 54529 invoked by uid 89); 19 Oct 2015 17:49:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ob0-f178.google.com MIME-Version: 1.0 X-Received: by 10.60.76.42 with SMTP id h10mr9316499oew.13.1445276957237; Mon, 19 Oct 2015 10:49:17 -0700 (PDT) In-Reply-To: References: <20151019164832.GA28619@intel.com> Date: Mon, 19 Oct 2015 10:49:17 -0700 Message-ID: Subject: Re: [PATCH] Avoid reading errno in syscall implementations From: "H.J. Lu" To: Joseph Myers Cc: GNU C Library On Mon, Oct 19, 2015 at 10:00 AM, Joseph Myers wrote: > On Mon, 19 Oct 2015, H.J. Lu wrote: > >> Reading errno is expensive for x86 PIC. With INTERNAL_SYSCALL, >> INTERNAL_SYSCALL_ERROR_P, INTERNAL_SYSCALL_ERRNO and >> INLINE_SYSCALL_ERROR_RETURN_VALUE, we can avoid reading errno. > > I don't follow how this patch works. How do you ensure that in the cases > where there is an error that is not ENOSYS, errno does get set as it would > have been before? > >> - int res = INLINE_SYSCALL (eventfd2, 2, count, flags); >> # ifndef __ASSUME_EVENTFD2 >> - if (res != -1 || errno != ENOSYS) >> -# endif >> + INTERNAL_SYSCALL_DECL (err); >> + int res = INTERNAL_SYSCALL (eventfd2, err, 2, count, flags); >> + if (!__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res, err)) >> + || INTERNAL_SYSCALL_ERRNO (res, err) != ENOSYS) >> return res; > > E.g. this appears to be semantically different from the previous code - > the previous code would have set errno here from a non-ENOSYS error, and > the new code wouldn't. > > (__ASSUME_EVENTFD2 is always defined except on alpha. Presumably your "no > code changes" was because code paths used on x86_64 didn't get changed by > the patch.) Here is the updated patch. Only getrlimit64.os and setrlimit64.os are changed on i686. OK for master? Thanks. From 6f6e3ee567cbff328df83dcf9ee0a5459746fce0 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Fri, 21 Aug 2015 14:46:05 -0700 Subject: [PATCH] Avoid reading errno in syscall implementations Reading errno is expensive for x86 PIC. With INTERNAL_SYSCALL, INTERNAL_SYSCALL_ERROR_P, INTERNAL_SYSCALL_ERRNO and INLINE_SYSCALL_ERROR_RETURN_VALUE, we can avoid reading errno. There are no code changes on x86-64. On i686, libc.so sizes in bytes show: text data bss dec after 1748495 11380 11132 1771007 before 1748403 11380 11132 1770915 * sysdeps/unix/sysv/linux/eventfd.c (eventfd): Use INTERNAL_SYSCALL, INTERNAL_SYSCALL_ERROR_P and INTERNAL_SYSCALL_ERRNO to avoid reading errno. * sysdeps/unix/sysv/linux/fstatfs64.c (__fstatfs64): Likewise. * sysdeps/unix/sysv/linux/getrlimit64.c (__getrlimit64): Likewise. * sysdeps/unix/sysv/linux/setrlimit64.c (setrlimit64): Likewise. * sysdeps/unix/sysv/linux/signalfd.c (signalfd): Likewise. * sysdeps/unix/sysv/linux/statfs64.c (__statfs64): Likewise. --- sysdeps/unix/sysv/linux/eventfd.c | 11 ++++++++--- sysdeps/unix/sysv/linux/fstatfs64.c | 13 +++++++++---- sysdeps/unix/sysv/linux/getrlimit64.c | 8 ++++++-- sysdeps/unix/sysv/linux/lxstat64.c | 7 +++++-- sysdeps/unix/sysv/linux/setrlimit64.c | 8 ++++++-- sysdeps/unix/sysv/linux/signalfd.c | 12 +++++++++--- sysdeps/unix/sysv/linux/statfs64.c | 14 ++++++++++---- 7 files changed, 53 insertions(+), 20 deletions(-) diff --git a/sysdeps/unix/sysv/linux/eventfd.c b/sysdeps/unix/sysv/linux/eventfd.c index efce282..3a91781 100644 --- a/sysdeps/unix/sysv/linux/eventfd.c +++ b/sysdeps/unix/sysv/linux/eventfd.c @@ -25,11 +25,16 @@ int eventfd (unsigned int count, int flags) { #ifdef __NR_eventfd2 - int res = INLINE_SYSCALL (eventfd2, 2, count, flags); # ifndef __ASSUME_EVENTFD2 - if (res != -1 || errno != ENOSYS) -# endif + INTERNAL_SYSCALL_DECL (err); + int res = INTERNAL_SYSCALL (eventfd2, err, 2, count, flags); + if (!__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res, err))) return res; + else if (INTERNAL_SYSCALL_ERRNO (res, err) != ENOSYS) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (INTERNAL_SYSCALL_ERRNO (res, err)); +# else + return INLINE_SYSCALL (eventfd2, 2, count, flags); +# endif #endif #ifndef __ASSUME_EVENTFD2 diff --git a/sysdeps/unix/sysv/linux/fstatfs64.c b/sysdeps/unix/sysv/linux/fstatfs64.c index af83830..819b623 100644 --- a/sysdeps/unix/sysv/linux/fstatfs64.c +++ b/sysdeps/unix/sysv/linux/fstatfs64.c @@ -35,12 +35,17 @@ __fstatfs64 (int fd, struct statfs64 *buf) if (! __no_statfs64) # endif { - int result = INLINE_SYSCALL (fstatfs64, 3, fd, sizeof (*buf), buf); - # if __ASSUME_STATFS64 == 0 - if (result == 0 || errno != ENOSYS) -# endif + INTERNAL_SYSCALL_DECL (err); + int result = INTERNAL_SYSCALL (fstatfs64, err, 3, fd, + sizeof (*buf), buf); + if (!__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, err))) return result; + else if (INTERNAL_SYSCALL_ERRNO (result, err) != ENOSYS) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (INTERNAL_SYSCALL_ERRNO (result, err)); +# else + return INLINE_SYSCALL (fstatfs64, 3, fd, sizeof (*buf), buf); +# endif # if __ASSUME_STATFS64 == 0 __no_statfs64 = 1; diff --git a/sysdeps/unix/sysv/linux/getrlimit64.c b/sysdeps/unix/sysv/linux/getrlimit64.c index 100ba62..bc36c14 100644 --- a/sysdeps/unix/sysv/linux/getrlimit64.c +++ b/sysdeps/unix/sysv/linux/getrlimit64.c @@ -30,9 +30,13 @@ __getrlimit64 (enum __rlimit_resource resource, struct rlimit64 *rlimits) return INLINE_SYSCALL (prlimit64, 4, 0, resource, NULL, rlimits); #else # ifdef __NR_prlimit64 - int res = INLINE_SYSCALL (prlimit64, 4, 0, resource, NULL, rlimits); - if (res == 0 || errno != ENOSYS) + INTERNAL_SYSCALL_DECL (err); + int res = INTERNAL_SYSCALL (prlimit64, err, 4, 0, resource, NULL, + rlimits); + if (!__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res, err))) return res; + else if (INTERNAL_SYSCALL_ERRNO (res, err) != ENOSYS) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (INTERNAL_SYSCALL_ERRNO (res, err)); # endif struct rlimit rlimits32; diff --git a/sysdeps/unix/sysv/linux/lxstat64.c b/sysdeps/unix/sysv/linux/lxstat64.c index 5d0c051..63959cf 100644 --- a/sysdeps/unix/sysv/linux/lxstat64.c +++ b/sysdeps/unix/sysv/linux/lxstat64.c @@ -30,8 +30,11 @@ int ___lxstat64 (int vers, const char *name, struct stat64 *buf) { - int result; - result = INLINE_SYSCALL (lstat64, 2, name, buf); + INTERNAL_SYSCALL_DECL (err); + int result = INTERNAL_SYSCALL (lstat64, err, 2, name, buf); + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, err))) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (INTERNAL_SYSCALL_ERRNO (result, + err)); #if defined _HAVE_STAT64___ST_INO && __ASSUME_ST_INO_64_BIT == 0 if (__builtin_expect (!result, 1) && buf->__st_ino != (__ino_t) buf->st_ino) buf->st_ino = buf->__st_ino; diff --git a/sysdeps/unix/sysv/linux/setrlimit64.c b/sysdeps/unix/sysv/linux/setrlimit64.c index 5b1f657..b13014a 100644 --- a/sysdeps/unix/sysv/linux/setrlimit64.c +++ b/sysdeps/unix/sysv/linux/setrlimit64.c @@ -31,9 +31,13 @@ setrlimit64 (enum __rlimit_resource resource, const struct rlimit64 *rlimits) return INLINE_SYSCALL (prlimit64, 4, 0, resource, rlimits, NULL); #else # ifdef __NR_prlimit64 - int res = INLINE_SYSCALL (prlimit64, 4, 0, resource, rlimits, NULL); - if (res == 0 || errno != ENOSYS) + INTERNAL_SYSCALL_DECL (err); + int res = INTERNAL_SYSCALL (prlimit64, err, 4, 0, resource, rlimits, + NULL); + if (!__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res, err))) return res; + else if (INTERNAL_SYSCALL_ERRNO (res, err) != ENOSYS) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (INTERNAL_SYSCALL_ERRNO (res, err)); # endif struct rlimit rlimits32; diff --git a/sysdeps/unix/sysv/linux/signalfd.c b/sysdeps/unix/sysv/linux/signalfd.c index 8573450..95a9707 100644 --- a/sysdeps/unix/sysv/linux/signalfd.c +++ b/sysdeps/unix/sysv/linux/signalfd.c @@ -26,11 +26,17 @@ int signalfd (int fd, const sigset_t *mask, int flags) { #ifdef __NR_signalfd4 - int res = INLINE_SYSCALL (signalfd4, 4, fd, mask, _NSIG / 8, flags); # ifndef __ASSUME_SIGNALFD4 - if (res != -1 || errno != ENOSYS) -# endif + INTERNAL_SYSCALL_DECL (err); + int res = INTERNAL_SYSCALL (signalfd4, err, 4, fd, mask, _NSIG / 8, + flags); + if (!__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (res, err))) return res; + else if (INTERNAL_SYSCALL_ERRNO (res, err) != ENOSYS) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (INTERNAL_SYSCALL_ERRNO (res, err)); +# else + return INLINE_SYSCALL (signalfd4, 4, fd, mask, _NSIG / 8, flags); +# endif #endif #ifndef __ASSUME_SIGNALFD4 diff --git a/sysdeps/unix/sysv/linux/statfs64.c b/sysdeps/unix/sysv/linux/statfs64.c index ac5c33f..469eda1 100644 --- a/sysdeps/unix/sysv/linux/statfs64.c +++ b/sysdeps/unix/sysv/linux/statfs64.c @@ -37,12 +37,18 @@ __statfs64 (const char *file, struct statfs64 *buf) if (! __no_statfs64) # endif { - int result = INLINE_SYSCALL (statfs64, 3, file, sizeof (*buf), buf); - # if __ASSUME_STATFS64 == 0 - if (result == 0 || errno != ENOSYS) -# endif + INTERNAL_SYSCALL_DECL (err); + int result = INTERNAL_SYSCALL (statfs64, err, 3, file, + sizeof (*buf), buf); + + if (!__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, err))) return result; + else if (INTERNAL_SYSCALL_ERRNO (result, err) != ENOSYS) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (INTERNAL_SYSCALL_ERRNO (result, err)); +# else + return INLINE_SYSCALL (statfs64, 3, file, sizeof (*buf), buf); +# endif # if __ASSUME_STATFS64 == 0 __no_statfs64 = 1; -- 2.4.3