From: Lukasz Majewski <lukma@denx.de>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: libc-alpha@sourceware.org, Carlos O'Donell <carlos@redhat.com>
Subject: Re: [PATCH 05/18] linux: Only use 64-bit syscall if required for pselect
Date: Mon, 21 Jun 2021 09:42:58 +0200 [thread overview]
Message-ID: <20210621094258.373528a3@ktm> (raw)
In-Reply-To: <20210617115104.1359598-6-adhemerval.zanella@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 10320 bytes --]
On Thu, 17 Jun 2021 08:50:51 -0300
Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> For !__ASSUME_TIME64_SYSCALLS there is no need to issue a 64-bit
> syscall if the provided timeout fits in a 32-bit one. The 64-bit
> usage should be rare since the timeout is a relative one. This also
> avoids the need to use supports_time64() (which breaks the usage case
> of live migration like CRIU or similar).
>
> Checked on i686-linux-gnu on a 4.15 kernel and on a 5.11 kernel
> (with and without --enable-kernel=5.1) and on x86_64-linux-gnu.
> ---
> misc/Makefile | 9 ++
> misc/tst-pselect.c | 120
> ++++++++---------- .../unix/sysv/linux/microblaze/pselect32.c |
> 3 +- sysdeps/unix/sysv/linux/pselect.c | 47 ++++---
> sysdeps/unix/sysv/linux/pselect32.c | 6 -
> 5 files changed, 95 insertions(+), 90 deletions(-)
>
> diff --git a/misc/Makefile b/misc/Makefile
> index f34ab09fe3..fa40bf0e11 100644
> --- a/misc/Makefile
> +++ b/misc/Makefile
> @@ -148,6 +148,12 @@ CFLAGS-brk.op = $(no-stack-protector)
>
> include ../Rules
>
> +ifeq (yes,$(build-shared))
> +librt = $(common-objpfx)rt/librt.so
> +else
> +librt = $(common-objpfx)rt/librt.a
> +endif
> +
> $(objpfx)libg.a: $(dep-dummy-lib); $(make-dummy-lib)
>
> $(objpfx)tst-tsearch: $(libm)
> @@ -162,3 +168,6 @@ tst-allocate_once-ENV =
> MALLOC_TRACE=$(objpfx)tst-allocate_once.mtrace
> $(objpfx)tst-allocate_once-mem.out: $(objpfx)tst-allocate_once.out
> $(common-objpfx)malloc/mtrace $(objpfx)tst-allocate_once.mtrace > $@;
> \ $(evaluate-test) +
> +$(objpfx)tst-pselect: $(librt)
> +$(objpfx)tst-pselect-time64: $(librt)
> diff --git a/misc/tst-pselect.c b/misc/tst-pselect.c
> index c89176b058..f8c404c275 100644
> --- a/misc/tst-pselect.c
> +++ b/misc/tst-pselect.c
> @@ -16,14 +16,14 @@
> <https://www.gnu.org/licenses/>. */
>
> #include <errno.h>
> -#include <signal.h>
> -#include <stdio.h>
> -#include <unistd.h>
> -#include <sys/select.h>
> -#include <sys/wait.h>
> +#include <intprops.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/xsignal.h>
> +#include <support/xunistd.h>
> +#include <support/xtime.h>
> #include <stdlib.h>
>
> -
> static volatile int handler_called;
>
> static void
> @@ -33,59 +33,43 @@ handler (int sig)
> }
>
>
> -static int
> -do_test (void)
> +static void
> +test_pselect_basic (void)
> {
> struct sigaction sa;
> sa.sa_handler = handler;
> sa.sa_flags = 0;
> sigemptyset (&sa.sa_mask);
>
> - if (sigaction (SIGUSR1, &sa, NULL) != 0)
> - {
> - puts ("sigaction failed");
> - return 1;
> - }
> + xsigaction (SIGUSR1, &sa, NULL);
>
> sa.sa_handler = SIG_IGN;
> - if (sigaction (SIGCHLD, &sa, NULL) != 0)
> - {
> - puts ("2nd sigaction failed");
> - return 1;
> - }
> + xsigaction (SIGCHLD, &sa, NULL);
>
> sigset_t ss_usr1;
> sigemptyset (&ss_usr1);
> sigaddset (&ss_usr1, SIGUSR1);
> - if (sigprocmask (SIG_BLOCK, &ss_usr1, NULL) != 0)
> - {
> - puts ("sigprocmask failed");
> - return 1;
> - }
> + TEST_COMPARE (sigprocmask (SIG_BLOCK, &ss_usr1, NULL), 0);
>
> int fds[2][2];
> -
> - if (pipe (fds[0]) != 0 || pipe (fds[1]) != 0)
> - {
> - puts ("pipe failed");
> - return 1;
> - }
> + xpipe (fds[0]);
> + xpipe (fds[1]);
>
> fd_set rfds;
> FD_ZERO (&rfds);
>
> sigset_t ss;
> - sigprocmask (SIG_SETMASK, NULL, &ss);
> + TEST_COMPARE (sigprocmask (SIG_SETMASK, NULL, &ss), 0);
> sigdelset (&ss, SIGUSR1);
>
> struct timespec to = { .tv_sec = 0, .tv_nsec = 500000000 };
>
> pid_t parent = getpid ();
> - pid_t p = fork ();
> + pid_t p = xfork ();
> if (p == 0)
> {
> - close (fds[0][1]);
> - close (fds[1][0]);
> + xclose (fds[0][1]);
> + xclose (fds[1][0]);
>
> FD_SET (fds[0][0], &rfds);
>
> @@ -93,55 +77,63 @@ do_test (void)
> do
> {
> if (getppid () != parent)
> - exit (2);
> + FAIL_EXIT1 ("getppid()=%d != parent=%d", getppid(),
> parent);
> errno = 0;
> e = pselect (fds[0][0] + 1, &rfds, NULL, NULL, &to, &ss);
> }
> while (e == 0);
>
> - if (e != -1)
> - {
> - puts ("child: pselect did not fail");
> - return 0;
> - }
> - if (errno != EINTR)
> - {
> - puts ("child: pselect did not set errno to EINTR");
> - return 0;
> - }
> + TEST_COMPARE (e, -1);
> + TEST_COMPARE (errno, EINTR);
>
> TEMP_FAILURE_RETRY (write (fds[1][1], "foo", 3));
>
> exit (0);
> }
>
> - close (fds[0][0]);
> - close (fds[1][1]);
> + xclose (fds[0][0]);
> + xclose (fds[1][1]);
>
> FD_SET (fds[1][0], &rfds);
>
> - kill (p, SIGUSR1);
> + TEST_COMPARE (kill (p, SIGUSR1), 0);
>
> int e = pselect (fds[1][0] + 1, &rfds, NULL, NULL, NULL, &ss);
> - if (e == -1)
> - {
> - puts ("parent: pselect failed");
> - return 1;
> - }
> - if (e != 1)
> - {
> - puts ("parent: pselect did not report readable fd");
> - return 1;
> - }
> - if (!FD_ISSET (fds[1][0], &rfds))
> - {
> - puts ("parent: pselect reports wrong fd");
> - return 1;
> - }
> + TEST_COMPARE (e, 1);
> + TEST_VERIFY (FD_ISSET (fds[1][0], &rfds));
> +}
> +
> +static void
> +test_pselect_large_timeout (void)
> +{
> + support_create_timer (0, 100000000, false, NULL);
> +
> + int fds[2];
> + xpipe (fds);
> +
> + fd_set rfds;
> + FD_ZERO (&rfds);
> + FD_SET (fds[0], &rfds);
> +
> + sigset_t ss;
> + TEST_COMPARE (sigprocmask (SIG_SETMASK, NULL, &ss), 0);
> + sigdelset (&ss, SIGALRM);
> +
> + struct timespec ts = { TYPE_MAXIMUM (time_t), 0 };
> +
> + TEST_COMPARE (pselect (fds[0] + 1, &rfds, NULL, NULL, &ts, &ss),
> -1);
> + TEST_VERIFY (errno == EINTR || errno == EOVERFLOW);
> +}
> +
> +static int
> +do_test (void)
> +{
> + test_pselect_basic ();
> +
> + test_pselect_large_timeout ();
>
> return 0;
> }
>
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> +#include <support/test-driver.c>
> diff --git a/sysdeps/unix/sysv/linux/microblaze/pselect32.c
> b/sysdeps/unix/sysv/linux/microblaze/pselect32.c index
> a4f436462b..70b7b52a48 100644 ---
> a/sysdeps/unix/sysv/linux/microblaze/pselect32.c +++
> b/sysdeps/unix/sysv/linux/microblaze/pselect32.c @@ -35,8 +35,7 @@
> __pselect32 (int nfds, fd_set *readfds, fd_set *writefds, struct
> timeval tv32, *ptv32 = NULL; if (timeout != NULL)
> {
> - if (! in_time_t_range (timeout->tv_sec)
> - || ! valid_nanoseconds (timeout->tv_nsec))
> + if (! valid_nanoseconds (timeout->tv_nsec))
> {
> __set_errno (EINVAL);
> return -1;
> diff --git a/sysdeps/unix/sysv/linux/pselect.c
> b/sysdeps/unix/sysv/linux/pselect.c index 5e8a0cc2dc..79e8584ea7
> 100644 --- a/sysdeps/unix/sysv/linux/pselect.c
> +++ b/sysdeps/unix/sysv/linux/pselect.c
> @@ -18,7 +18,23 @@
>
> #include <sys/select.h>
> #include <sysdep-cancel.h>
> -#include <time64-support.h>
> +
> +static int
> +pselect64_syscall (int nfds, fd_set *readfds, fd_set *writefds,
> + fd_set *exceptfds, const struct __timespec64
> *timeout,
> + const sigset_t *sigmask)
> +{
> +#ifndef __NR_pselect6_time64
> +# define __NR_pselect6_time64 __NR_pselect6
> +#endif
> + /* NB: This is required by ARGIFY used in x32 internal_syscallN.
> */
> + __syscall_ulong_t data[2] =
> + {
> + (uintptr_t) sigmask, __NSIG_BYTES
> + };
> + return SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds,
> exceptfds,
> + timeout, data);
> +}
>
> int
> __pselect64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set
> *exceptfds, @@ -37,30 +53,25 @@ __pselect64 (int nfds, fd_set
> *readfds, fd_set *writefds, fd_set *exceptfds, we can only pass in 6
> directly. If there is an architecture with support for more
> parameters a new version of this file needs to be created. */
> -
> -#ifndef __NR_pselect6_time64
> -# define __NR_pselect6_time64 __NR_pselect6
> -#endif
> +#ifdef __ASSUME_TIME64_SYSCALLS
> + return pselect64_syscall (nfds, readfds, writefds, exceptfds,
> timeout,
> + sigmask);
> +#else
> + bool is32bit = timeout != NULL
> + ? in_time_t_range (timeout->tv_sec) : true;
> int r;
> - if (supports_time64 ())
> + if (!is32bit)
> {
> - /* NB: This is required by ARGIFY used in x32
> internal_syscallN. */
> - __syscall_ulong_t data[2] =
> - {
> - (uintptr_t) sigmask, __NSIG_BYTES
> - };
> - r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds,
> exceptfds,
> - timeout, data);
> + r = pselect64_syscall (nfds, readfds, writefds, exceptfds,
> timeout,
> + sigmask);
> if (r == 0 || errno != ENOSYS)
> return r;
> -
> - mark_time64_unsupported ();
> + __set_errno (EOVERFLOW);
> + return -1;
> }
>
> -#ifndef __ASSUME_TIME64_SYSCALLS
> - r = __pselect32 (nfds, readfds, writefds, exceptfds, timeout,
> sigmask);
> + return __pselect32 (nfds, readfds, writefds, exceptfds, timeout,
> sigmask); #endif
> - return r;
> }
>
> #if __TIMESIZE != 64
> diff --git a/sysdeps/unix/sysv/linux/pselect32.c
> b/sysdeps/unix/sysv/linux/pselect32.c index eb59b51cdf..48724d8727
> 100644 --- a/sysdeps/unix/sysv/linux/pselect32.c
> +++ b/sysdeps/unix/sysv/linux/pselect32.c
> @@ -29,12 +29,6 @@ __pselect32 (int nfds, fd_set *readfds, fd_set
> *writefds, struct timespec ts32, *pts32 = NULL;
> if (timeout != NULL)
> {
> - if (! in_time_t_range (timeout->tv_sec))
> - {
> - __set_errno (EINVAL);
> - return -1;
> - }
> -
> ts32 = valid_timespec64_to_timespec (*timeout);
> pts32 = &ts32;
> }
Reviewed-by: Lukasz Majewski <lukma@denx.de>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2021-06-21 7:43 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-17 11:50 [PATCH 00/18] More y2038 fixes Adhemerval Zanella
2021-06-17 11:50 ` [PATCH 01/18] Use 64 bit time_t stat internally Adhemerval Zanella
2021-06-21 7:42 ` Lukasz Majewski
2021-06-22 19:37 ` Florian Weimer
2021-06-22 19:51 ` Adhemerval Zanella
2021-06-17 11:50 ` [PATCH 02/18] Use LFS and 64 bit time for installed programs Adhemerval Zanella
2021-06-17 12:19 ` Andreas Schwab
2021-06-18 18:50 ` Adhemerval Zanella
2021-06-17 20:49 ` Joseph Myers
2021-06-18 18:14 ` Adhemerval Zanella
2021-06-17 11:50 ` [PATCH 03/18] support: Add support_create_timer Adhemerval Zanella
2021-06-21 7:42 ` Lukasz Majewski
2021-06-17 11:50 ` [PATCH 04/18] linux: Only use 64-bit syscall if required for ppoll Adhemerval Zanella
2021-06-21 7:42 ` Lukasz Majewski
2021-06-17 11:50 ` [PATCH 05/18] linux: Only use 64-bit syscall if required for pselect Adhemerval Zanella
2021-06-21 7:42 ` Lukasz Majewski [this message]
2021-06-17 11:50 ` [PATCH 06/18] linux: Only use 64-bit syscall if required for select Adhemerval Zanella
2021-06-21 7:43 ` Lukasz Majewski
2021-06-17 11:50 ` [PATCH 07/18] linux: Remove supports_time64 () from clock_getres Adhemerval Zanella
2021-06-21 7:43 ` Lukasz Majewski
2021-06-17 11:50 ` [PATCH 08/18] linux: Remove supports_time64 () from clock_gettime Adhemerval Zanella
2021-06-21 7:43 ` Lukasz Majewski
2021-06-17 11:50 ` [PATCH 09/18] linux: Remove time64-support Adhemerval Zanella
2021-06-21 7:43 ` Lukasz Majewski
2021-06-17 11:50 ` [PATCH 10/18] linux: timerfd_gettime minor cleanup Adhemerval Zanella
2021-06-21 7:43 ` Lukasz Majewski
2021-06-17 11:50 ` [PATCH 11/18] linux: Only use 64-bit syscall if required for semtimedop Adhemerval Zanella
2021-06-21 7:43 ` Lukasz Majewski
2021-06-17 11:50 ` [PATCH 12/18] linux: Only use 64-bit syscall if required for timerfd_settime Adhemerval Zanella
2021-06-21 7:44 ` Lukasz Majewski
2021-06-17 11:50 ` [PATCH 13/18] linux: Only use 64-bit syscall if required for mq_timedreceive Adhemerval Zanella
2021-06-21 7:44 ` Lukasz Majewski
2021-06-17 11:51 ` [PATCH 14/18] linux: Only use 64-bit syscall if required for mq_timedsend Adhemerval Zanella
2021-06-21 7:44 ` Lukasz Majewski
2021-06-17 11:51 ` [PATCH 15/18] linux: Only use 64-bit syscall if required for sigtimedwait Adhemerval Zanella
2021-06-17 12:25 ` Andreas Schwab
2021-06-22 14:58 ` Adhemerval Zanella
2021-06-17 11:51 ` [PATCH 16/18] linux: Only use 64-bit syscall if required for utimensat family Adhemerval Zanella
2021-06-21 7:45 ` Lukasz Majewski
2021-06-17 11:51 ` [PATCH 17/18] linux: Only use 64-bit syscall if required for internal futex Adhemerval Zanella
2021-06-21 7:45 ` Lukasz Majewski
2021-06-17 11:51 ` [PATCH 18/18] linux: Only use 64-bit syscall if required for clock_nanosleep Adhemerval Zanella
2021-06-17 15:11 ` Lukasz Majewski
2021-06-17 17:45 ` Adhemerval Zanella
2021-06-21 7:46 ` Lukasz Majewski
2021-06-17 14:20 ` [PATCH 00/18] More y2038 fixes Lukasz Majewski
2021-06-17 17:41 ` Adhemerval Zanella
2021-06-17 20:58 ` Joseph Myers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210621094258.373528a3@ktm \
--to=lukma@denx.de \
--cc=adhemerval.zanella@linaro.org \
--cc=carlos@redhat.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).