[v2] misc: Set generic pselect as ENOSYS

Message ID 20191114220056.16183-1-adhemerval.zanella@linaro.org
State New
Headers show
Series
  • [v2] misc: Set generic pselect as ENOSYS
Related show

Commit Message

Adhemerval Zanella Nov. 14, 2019, 10 p.m.
Changes from previous version:

  - Added some include cleanups.

  - Add the __ASSUME_PSELECT usage on microblaze version.

--

The generic pselect implementation has the very specific race condition
that motived the creation of the pselect syscall (no atomicity in
signal mask set/reset).  Using it as generic implementation is
counterproductive  Also currently only microblaze uses it as fallback
when used on kernel prior 3.15.

This patch moves the generic implementation to a microblaze specific
one, sets the generic internal as a ENOSYS, and cleanups the Linux
generic implementation.

The microblaze implementation mimics the previous Linux generic one,
where it either uses pselect6 directly if __ASSUME_PSELECT or a
first try pselect6 then the fallback otherwise.

Checked on x86_64-linux-gnu and microblaze-linux-gnu.
---
 misc/pselect.c                               | 46 +-----------
 sysdeps/unix/sysv/linux/microblaze/pselect.c | 73 ++++++++++++++++++++
 sysdeps/unix/sysv/linux/pselect.c            | 37 ++--------
 3 files changed, 80 insertions(+), 76 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/microblaze/pselect.c

-- 
2.17.1

Comments

Adhemerval Zanella Nov. 22, 2019, 2:45 p.m. | #1
I will commit this patch with Lukasz Majewski remark if no one opposes.

On 14/11/2019 19:00, Adhemerval Zanella wrote:
> Changes from previous version:

> 

>   - Added some include cleanups.

> 

>   - Add the __ASSUME_PSELECT usage on microblaze version.

> 

> --

> 

> The generic pselect implementation has the very specific race condition

> that motived the creation of the pselect syscall (no atomicity in

> signal mask set/reset).  Using it as generic implementation is

> counterproductive  Also currently only microblaze uses it as fallback

> when used on kernel prior 3.15.

> 

> This patch moves the generic implementation to a microblaze specific

> one, sets the generic internal as a ENOSYS, and cleanups the Linux

> generic implementation.

> 

> The microblaze implementation mimics the previous Linux generic one,

> where it either uses pselect6 directly if __ASSUME_PSELECT or a

> first try pselect6 then the fallback otherwise.

> 

> Checked on x86_64-linux-gnu and microblaze-linux-gnu.

> ---

>  misc/pselect.c                               | 46 +-----------

>  sysdeps/unix/sysv/linux/microblaze/pselect.c | 73 ++++++++++++++++++++

>  sysdeps/unix/sysv/linux/pselect.c            | 37 ++--------

>  3 files changed, 80 insertions(+), 76 deletions(-)

>  create mode 100644 sysdeps/unix/sysv/linux/microblaze/pselect.c

> 

> diff --git a/misc/pselect.c b/misc/pselect.c

> index 76ded850a5..b53d9cdcc1 100644

> --- a/misc/pselect.c

> +++ b/misc/pselect.c

> @@ -17,12 +17,7 @@

>     <https://www.gnu.org/licenses/>.  */

>  

>  #include <errno.h>

> -#include <signal.h>

> -#include <stddef.h>	/* For NULL.  */

> -#include <sys/time.h>

>  #include <sys/select.h>

> -#include <sysdep-cancel.h>

> -

>  

>  /* Check the first NFDS descriptors each in READFDS (if not NULL) for read

>     readiness, in WRITEFDS (if not NULL) for write readiness, and in EXCEPTFDS

> @@ -34,43 +29,8 @@ int

>  __pselect (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,

>  	   const struct timespec *timeout, const sigset_t *sigmask)

>  {

> -  struct timeval tval;

> -  int retval;

> -  sigset_t savemask;

> -

> -  /* Change nanosecond number to microseconds.  This might mean losing

> -     precision and therefore the `pselect` should be available.  But

> -     for now it is hardly found.  */

> -  if (timeout != NULL)

> -    {

> -      /* Catch bugs which would be hidden by the TIMESPEC_TO_TIMEVAL

> -	 computations.  The division by 1000 truncates values.  */

> -      if (__glibc_unlikely (timeout->tv_nsec < 0))

> -	{

> -	  __set_errno (EINVAL);

> -	  return -1;

> -	}

> -

> -      TIMESPEC_TO_TIMEVAL (&tval, timeout);

> -    }

> -

> -  /* The setting and restoring of the signal mask and the select call

> -     should be an atomic operation.  This can't be done without kernel

> -     help.  */

> -  if (sigmask != NULL)

> -    __sigprocmask (SIG_SETMASK, sigmask, &savemask);

> -

> -  /* Note the pselect() is a cancellation point.  But since we call

> -     select() which itself is a cancellation point we do not have

> -     to do anything here.  */

> -  retval = __select (nfds, readfds, writefds, exceptfds,

> -		     timeout != NULL ? &tval : NULL);

> -

> -  if (sigmask != NULL)

> -    __sigprocmask (SIG_SETMASK, &savemask, NULL);

> -

> -  return retval;

> +  __set_errno (ENOSYS);

> +  return -1;

>  }

> -#ifndef __pselect

>  weak_alias (__pselect, pselect)

> -#endif

> +stub_warning (pselect)

> diff --git a/sysdeps/unix/sysv/linux/microblaze/pselect.c b/sysdeps/unix/sysv/linux/microblaze/pselect.c

> new file mode 100644

> index 0000000000..561ff99fb4

> --- /dev/null

> +++ b/sysdeps/unix/sysv/linux/microblaze/pselect.c

> @@ -0,0 +1,73 @@

> +/* Synchronous I/O multiplexing.  Linux/microblaze version.

> +   Copyright (C) 2019 Free Software Foundation, Inc.

> +   This file is part of the GNU C Library.

> +

> +   The GNU C Library is free software; you can redistribute it and/or

> +   modify it under the terms of the GNU Lesser General Public

> +   License as published by the Free Software Foundation; either

> +   version 2.1 of the License, or (at your option) any later version.

> +

> +   The GNU C Library is distributed in the hope that it will be useful,

> +   but WITHOUT ANY WARRANTY; without even the implied warranty of

> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

> +   Lesser General Public License for more details.

> +

> +   You should have received a copy of the GNU Lesser General Public

> +   License along with the GNU C Library; if not, see

> +   <https://www.gnu.org/licenses/>.  */

> +

> +#include <errno.h>

> +#include <signal.h>

> +#include <time.h>

> +#include <sys/poll.h>

> +#include <sysdep-cancel.h>

> +

> +#ifndef __ASSUME_PSELECT

> +# define __pselect __pselect_syscall

> +#endif

> +

> +/* If pselect is supported, just use the Linux generic implementation.  */

> +#include <sysdeps/unix/sysv/linux/pselect.c>

> +

> +#ifndef __ASSUME_PSELECT

> +# undef __pselect

> +int

> +__pselect (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,

> +	   const struct timespec *timeout, const sigset_t *sigmask)

> +{

> +  int ret = __pselect_syscall (nfds, readfds, writefds, exceptfds, timeout,

> +			       sigmask);

> +  if (ret < 0 && errno != ENOSYS)

> +    return ret;

> +

> +  /* The fallback uses 'select' which shows the race condition regarding

> +     signal mask set/restore, requires two additional syscalls, and has

> +     a worse timeout precision (microseconds instead of nanoseconds).  */

> +

> +  struct timeval tval, *ptval = NULL;

> +  if (timeout != NULL)

> +    {

> +      if (! valid_nanoseconds (timeout->tv_nsec))

> +	{

> +	  __set_errno (EINVAL);

> +	  return -1;

> +	}

> +

> +      TIMESPEC_TO_TIMEVAL (&tval, timeout);

> +      ptval = &tval;

> +    }

> +

> +  sigset_t savemask;

> +  if (sigmask != NULL)

> +    __sigprocmask (SIG_SETMASK, sigmask, &savemask);

> +

> +  /* select itself is a cancellation entrypoint.  */

> +  ret = __select (nfds, readfds, writefds, exceptfds, ptval);

> +

> +  if (sigmask != NULL)

> +    __sigprocmask (SIG_SETMASK, &savemask, NULL);

> +

> +  return ret;

> +}

> +weak_alias (__pselect, pselect)

> +#endif

> diff --git a/sysdeps/unix/sysv/linux/pselect.c b/sysdeps/unix/sysv/linux/pselect.c

> index acda3e0cdd..7a3dc8c4ed 100644

> --- a/sysdeps/unix/sysv/linux/pselect.c

> +++ b/sysdeps/unix/sysv/linux/pselect.c

> @@ -16,23 +16,9 @@

>     License along with the GNU C Library; if not, see

>     <https://www.gnu.org/licenses/>.  */

>  

> -#include <errno.h>

> -#include <signal.h>

> -#include <time.h>

> -#include <sys/poll.h>

> -#include <kernel-features.h>

> +#include <sys/select.h>

>  #include <sysdep-cancel.h>

>  

> -

> -#ifdef __NR_pselect6

> -# ifndef __ASSUME_PSELECT

> -static int __generic_pselect (int nfds, fd_set *readfds, fd_set *writefds,

> -			      fd_set *exceptfds,

> -			      const struct timespec *timeout,

> -			      const sigset_t *sigmask);

> -# endif

> -

> -

>  int

>  __pselect (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,

>  	   const struct timespec *timeout, const sigset_t *sigmask)

> @@ -59,24 +45,9 @@ __pselect (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,

>    data.ss = (__syscall_ulong_t) (uintptr_t) sigmask;

>    data.ss_len = _NSIG / 8;

>  

> -  int result = SYSCALL_CANCEL (pselect6, nfds, readfds, writefds, exceptfds,

> -                               timeout, &data);

> -

> -# ifndef __ASSUME_PSELECT

> -  if (result == -1 && errno == ENOSYS)

> -    result = __generic_pselect (nfds, readfds, writefds, exceptfds, timeout,

> -				sigmask);

> -# endif

> -

> -  return result;

> +  return SYSCALL_CANCEL (pselect6, nfds, readfds, writefds, exceptfds,

> +                         timeout, &data);

>  }

> +#ifndef __pselect

>  weak_alias (__pselect, pselect)

> -

> -# ifndef __ASSUME_PSELECT

> -#  define __pselect static __generic_pselect

> -# endif

> -#endif

> -

> -#ifndef __ASSUME_PSELECT

> -# include <misc/pselect.c>

>  #endif

>

Patch

diff --git a/misc/pselect.c b/misc/pselect.c
index 76ded850a5..b53d9cdcc1 100644
--- a/misc/pselect.c
+++ b/misc/pselect.c
@@ -17,12 +17,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <errno.h>
-#include <signal.h>
-#include <stddef.h>	/* For NULL.  */
-#include <sys/time.h>
 #include <sys/select.h>
-#include <sysdep-cancel.h>
-
 
 /* Check the first NFDS descriptors each in READFDS (if not NULL) for read
    readiness, in WRITEFDS (if not NULL) for write readiness, and in EXCEPTFDS
@@ -34,43 +29,8 @@  int
 __pselect (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
 	   const struct timespec *timeout, const sigset_t *sigmask)
 {
-  struct timeval tval;
-  int retval;
-  sigset_t savemask;
-
-  /* Change nanosecond number to microseconds.  This might mean losing
-     precision and therefore the `pselect` should be available.  But
-     for now it is hardly found.  */
-  if (timeout != NULL)
-    {
-      /* Catch bugs which would be hidden by the TIMESPEC_TO_TIMEVAL
-	 computations.  The division by 1000 truncates values.  */
-      if (__glibc_unlikely (timeout->tv_nsec < 0))
-	{
-	  __set_errno (EINVAL);
-	  return -1;
-	}
-
-      TIMESPEC_TO_TIMEVAL (&tval, timeout);
-    }
-
-  /* The setting and restoring of the signal mask and the select call
-     should be an atomic operation.  This can't be done without kernel
-     help.  */
-  if (sigmask != NULL)
-    __sigprocmask (SIG_SETMASK, sigmask, &savemask);
-
-  /* Note the pselect() is a cancellation point.  But since we call
-     select() which itself is a cancellation point we do not have
-     to do anything here.  */
-  retval = __select (nfds, readfds, writefds, exceptfds,
-		     timeout != NULL ? &tval : NULL);
-
-  if (sigmask != NULL)
-    __sigprocmask (SIG_SETMASK, &savemask, NULL);
-
-  return retval;
+  __set_errno (ENOSYS);
+  return -1;
 }
-#ifndef __pselect
 weak_alias (__pselect, pselect)
-#endif
+stub_warning (pselect)
diff --git a/sysdeps/unix/sysv/linux/microblaze/pselect.c b/sysdeps/unix/sysv/linux/microblaze/pselect.c
new file mode 100644
index 0000000000..561ff99fb4
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/microblaze/pselect.c
@@ -0,0 +1,73 @@ 
+/* Synchronous I/O multiplexing.  Linux/microblaze version.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <signal.h>
+#include <time.h>
+#include <sys/poll.h>
+#include <sysdep-cancel.h>
+
+#ifndef __ASSUME_PSELECT
+# define __pselect __pselect_syscall
+#endif
+
+/* If pselect is supported, just use the Linux generic implementation.  */
+#include <sysdeps/unix/sysv/linux/pselect.c>
+
+#ifndef __ASSUME_PSELECT
+# undef __pselect
+int
+__pselect (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
+	   const struct timespec *timeout, const sigset_t *sigmask)
+{
+  int ret = __pselect_syscall (nfds, readfds, writefds, exceptfds, timeout,
+			       sigmask);
+  if (ret < 0 && errno != ENOSYS)
+    return ret;
+
+  /* The fallback uses 'select' which shows the race condition regarding
+     signal mask set/restore, requires two additional syscalls, and has
+     a worse timeout precision (microseconds instead of nanoseconds).  */
+
+  struct timeval tval, *ptval = NULL;
+  if (timeout != NULL)
+    {
+      if (! valid_nanoseconds (timeout->tv_nsec))
+	{
+	  __set_errno (EINVAL);
+	  return -1;
+	}
+
+      TIMESPEC_TO_TIMEVAL (&tval, timeout);
+      ptval = &tval;
+    }
+
+  sigset_t savemask;
+  if (sigmask != NULL)
+    __sigprocmask (SIG_SETMASK, sigmask, &savemask);
+
+  /* select itself is a cancellation entrypoint.  */
+  ret = __select (nfds, readfds, writefds, exceptfds, ptval);
+
+  if (sigmask != NULL)
+    __sigprocmask (SIG_SETMASK, &savemask, NULL);
+
+  return ret;
+}
+weak_alias (__pselect, pselect)
+#endif
diff --git a/sysdeps/unix/sysv/linux/pselect.c b/sysdeps/unix/sysv/linux/pselect.c
index acda3e0cdd..7a3dc8c4ed 100644
--- a/sysdeps/unix/sysv/linux/pselect.c
+++ b/sysdeps/unix/sysv/linux/pselect.c
@@ -16,23 +16,9 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <signal.h>
-#include <time.h>
-#include <sys/poll.h>
-#include <kernel-features.h>
+#include <sys/select.h>
 #include <sysdep-cancel.h>
 
-
-#ifdef __NR_pselect6
-# ifndef __ASSUME_PSELECT
-static int __generic_pselect (int nfds, fd_set *readfds, fd_set *writefds,
-			      fd_set *exceptfds,
-			      const struct timespec *timeout,
-			      const sigset_t *sigmask);
-# endif
-
-
 int
 __pselect (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
 	   const struct timespec *timeout, const sigset_t *sigmask)
@@ -59,24 +45,9 @@  __pselect (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
   data.ss = (__syscall_ulong_t) (uintptr_t) sigmask;
   data.ss_len = _NSIG / 8;
 
-  int result = SYSCALL_CANCEL (pselect6, nfds, readfds, writefds, exceptfds,
-                               timeout, &data);
-
-# ifndef __ASSUME_PSELECT
-  if (result == -1 && errno == ENOSYS)
-    result = __generic_pselect (nfds, readfds, writefds, exceptfds, timeout,
-				sigmask);
-# endif
-
-  return result;
+  return SYSCALL_CANCEL (pselect6, nfds, readfds, writefds, exceptfds,
+                         timeout, &data);
 }
+#ifndef __pselect
 weak_alias (__pselect, pselect)
-
-# ifndef __ASSUME_PSELECT
-#  define __pselect static __generic_pselect
-# endif
-#endif
-
-#ifndef __ASSUME_PSELECT
-# include <misc/pselect.c>
 #endif