Avoid reading errno in syscall implementations

Message ID 20151019164832.GA28619@intel.com
State Superseded
Headers

Commit Message

Lu, Hongjiu Oct. 19, 2015, 4:48 p.m. UTC
  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 1748355	  11380	  11132	  1770867

OK for master?  Thanks.

H.J.
---
	* 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     | 10 +++++++---
 sysdeps/unix/sysv/linux/fstatfs64.c   | 12 ++++++++----
 sysdeps/unix/sysv/linux/getrlimit64.c |  7 +++++--
 sysdeps/unix/sysv/linux/lxstat64.c    |  7 +++++--
 sysdeps/unix/sysv/linux/setrlimit64.c |  7 +++++--
 sysdeps/unix/sysv/linux/signalfd.c    | 11 ++++++++---
 sysdeps/unix/sysv/linux/statfs64.c    | 13 +++++++++----
 7 files changed, 47 insertions(+), 20 deletions(-)
  

Comments

Joseph Myers Oct. 19, 2015, 5 p.m. UTC | #1
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.)
  

Patch

diff --git a/sysdeps/unix/sysv/linux/eventfd.c b/sysdeps/unix/sysv/linux/eventfd.c
index efce282..a13be32 100644
--- a/sysdeps/unix/sysv/linux/eventfd.c
+++ b/sysdeps/unix/sysv/linux/eventfd.c
@@ -25,11 +25,15 @@  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))
+      || INTERNAL_SYSCALL_ERRNO (res, err) != ENOSYS)
     return res;
+# 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..eb21d42 100644
--- a/sysdeps/unix/sysv/linux/fstatfs64.c
+++ b/sysdeps/unix/sysv/linux/fstatfs64.c
@@ -35,12 +35,16 @@  __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))
+	  || INTERNAL_SYSCALL_ERRNO (result, err) != ENOSYS)
 	return result;
+# 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..cca1265 100644
--- a/sysdeps/unix/sysv/linux/getrlimit64.c
+++ b/sysdeps/unix/sysv/linux/getrlimit64.c
@@ -30,8 +30,11 @@  __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))
+      || INTERNAL_SYSCALL_ERRNO (res, err) != ENOSYS)
     return res;
 # 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..cefa687 100644
--- a/sysdeps/unix/sysv/linux/setrlimit64.c
+++ b/sysdeps/unix/sysv/linux/setrlimit64.c
@@ -31,8 +31,11 @@  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))
+      || INTERNAL_SYSCALL_ERRNO (res, err) != ENOSYS)
     return res;
 # endif
   struct rlimit rlimits32;
diff --git a/sysdeps/unix/sysv/linux/signalfd.c b/sysdeps/unix/sysv/linux/signalfd.c
index 8573450..d58edc7 100644
--- a/sysdeps/unix/sysv/linux/signalfd.c
+++ b/sysdeps/unix/sysv/linux/signalfd.c
@@ -26,11 +26,16 @@  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))
+      || INTERNAL_SYSCALL_ERRNO (res, err) != ENOSYS)
     return res;
+# 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..fdd1667 100644
--- a/sysdeps/unix/sysv/linux/statfs64.c
+++ b/sysdeps/unix/sysv/linux/statfs64.c
@@ -37,12 +37,17 @@  __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))
+	  || INTERNAL_SYSCALL_ERRNO (result, err) != ENOSYS)
 	return result;
+# else
+      return INLINE_SYSCALL (statfs64, 3, file, sizeof (*buf), buf);
+# endif
 
 # if __ASSUME_STATFS64 == 0
       __no_statfs64 = 1;