Message ID | 1519328339-25282-1-git-send-email-adhemerval.zanella@linaro.org |
---|---|
State | Accepted |
Commit | fbd01e6c4427b558b63fedb938b7fc5fada8c6b8 |
Headers | show |
Series | nptl: Fix tst-cancel4 sendto tests | expand |
If no one opposes it, I will commit this shortly. On 22/02/2018 16:38, Adhemerval Zanella wrote: > Now that send might be implemented calling sendto syscall on Linux, > I am seeing some issue in some kernel configurations where tst-cancel4 > sendto do not block as expected. > > The socket used to force the syscall blocking is used with default > system configuration for buffer sending size, which might not be > correct to force blocking. This patch fixes it by explicit setting > buffer socket lower than the buffer size used. It also enabled sendto > cancellation tests to work in both ways (since internally send is > implemented routing to sendto on Linux kernel). > > The patch also removes unrequired make rules on some archictures > for send/recv. The generic nptl Makefile already set the compiler flags > required on some architectures for correct unwinding and libc object > are not strictly required to support unwind (since pthread_cancel > requires linking against libpthread). > > Checked on aarch64-linux-gnu and x86_64-linux-gnu. I also did a > sniff test with tst-cancel{4,5} on a simulated mips64-linux-gnu. > > * nptl/tst-cancel4-common.h (set_socket_buffer): New function. > * nptl/tst-cancel4-common.c (do_test): Call set_socket_buffer > for socketpair endpoint. > * nptl/tst-cancel4.c (tf_send): Call set_socket_buffer and use > WRITE_BUFFER_SIZE as buffer size for sending socket. > (tf_sendto): Use SOCK_STREAM instead of SOCK_DGRAM and fix an > issue on system where send is implemented with sendto syscall. > * sysdeps/unix/sysv/linux/mips/mips64/Makefile [$(subdir) = socket] > (CFLAGS-recv.c, CFLAGS-send.c): Remove rules. > [$(subdir) = nptl] (CFLAGS-recv.c, CFLAGS-send.c): Likewise. > * sysdeps/unix/sysv/linux/riscv/rv64/Makefile: Remove file. > --- > ChangeLog | 14 +++++++++++ > nptl/tst-cancel4-common.c | 18 +------------- > nptl/tst-cancel4-common.h | 14 +++++++++++ > nptl/tst-cancel4.c | 37 +++++++++++++++------------- > sysdeps/unix/sysv/linux/mips/mips64/Makefile | 10 -------- > sysdeps/unix/sysv/linux/riscv/rv64/Makefile | 4 --- > 6 files changed, 49 insertions(+), 48 deletions(-) > delete mode 100644 sysdeps/unix/sysv/linux/riscv/rv64/Makefile > > diff --git a/nptl/tst-cancel4-common.c b/nptl/tst-cancel4-common.c > index 5bc7e44..c6eee73 100644 > --- a/nptl/tst-cancel4-common.c > +++ b/nptl/tst-cancel4-common.c > @@ -20,29 +20,13 @@ > static int > do_test (void) > { > - int val; > - socklen_t len; > - > if (socketpair (AF_UNIX, SOCK_STREAM, PF_UNIX, fds) != 0) > { > perror ("socketpair"); > exit (1); > } > > - val = 1; > - len = sizeof(val); > - setsockopt (fds[1], SOL_SOCKET, SO_SNDBUF, &val, sizeof(val)); > - if (getsockopt (fds[1], SOL_SOCKET, SO_SNDBUF, &val, &len) < 0) > - { > - perror ("getsockopt"); > - exit (1); > - } > - if (val >= WRITE_BUFFER_SIZE) > - { > - puts ("minimum write buffer size too large"); > - exit (1); > - } > - setsockopt (fds[1], SOL_SOCKET, SO_SNDBUF, &val, sizeof(val)); > + set_socket_buffer (fds[1]); > > if (mktemp (fifoname) == NULL) > { > diff --git a/nptl/tst-cancel4-common.h b/nptl/tst-cancel4-common.h > index a526c0c..10cc4f3 100644 > --- a/nptl/tst-cancel4-common.h > +++ b/nptl/tst-cancel4-common.h > @@ -62,6 +62,20 @@ static pthread_barrier_t b2; > > #define WRITE_BUFFER_SIZE 16384 > > +/* Set the send buffer of socket S to 1 byte so any send operation > + done with WRITE_BUFFER_SIZE bytes will force syscall blocking. */ > +static void > +set_socket_buffer (int s) > +{ > + int val = 1; > + socklen_t len = sizeof(val); > + > + TEST_VERIFY_EXIT (setsockopt (s, SOL_SOCKET, SO_SNDBUF, &val, > + sizeof(val)) == 0); > + TEST_VERIFY_EXIT (getsockopt (s, SOL_SOCKET, SO_SNDBUF, &val, &len) == 0); > + TEST_VERIFY_EXIT (val < WRITE_BUFFER_SIZE); > +} > + > /* Cleanup handling test. */ > static int cl_called; > > diff --git a/nptl/tst-cancel4.c b/nptl/tst-cancel4.c > index 92a3d80..0532538 100644 > --- a/nptl/tst-cancel4.c > +++ b/nptl/tst-cancel4.c > @@ -726,6 +726,8 @@ tf_send (void *arg) > if (tempfd2 == -1) > FAIL_EXIT1 ("socket (AF_UNIX, SOCK_STREAM, 0): %m"); > > + set_socket_buffer (tempfd2); > + > if (connect (tempfd2, (struct sockaddr *) &sun, sizeof (sun)) != 0) > FAIL_EXIT1 ("connect: %m"); > > @@ -738,8 +740,7 @@ tf_send (void *arg) > > pthread_cleanup_push (cl, NULL); > > - /* Very large block, so that the send call blocks. */ > - char mem[700000]; > + char mem[WRITE_BUFFER_SIZE]; > > send (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0); > > @@ -1230,16 +1231,11 @@ tf_msync (void *arg) > static void * > tf_sendto (void *arg) > { > - if (arg == NULL) > - // XXX If somebody can provide a portable test case in which sendto() > - // blocks we can enable this test to run in both rounds. > - abort (); > - > struct sockaddr_un sun; > > - tempfd = socket (AF_UNIX, SOCK_DGRAM, 0); > + tempfd = socket (AF_UNIX, SOCK_STREAM, 0); > if (tempfd == -1) > - FAIL_EXIT1 ("socket (AF_UNIX, SOCK_DGRAM, 0): %m"); > + FAIL_EXIT1 ("socket (AF_UNIX, SOCK_STREAM, 0): %m"); > > int tries = 0; > do > @@ -1254,23 +1250,30 @@ tf_sendto (void *arg) > while (bind (tempfd, (struct sockaddr *) &sun, > offsetof (struct sockaddr_un, sun_path) > + strlen (sun.sun_path) + 1) != 0); > - tempfname = strdup (sun.sun_path); > > - tempfd2 = socket (AF_UNIX, SOCK_DGRAM, 0); > + listen (tempfd, 5); > + > + tempfd2 = socket (AF_UNIX, SOCK_STREAM, 0); > if (tempfd2 == -1) > - FAIL_EXIT1 ("socket (AF_UNIX, SOCK_DGRAM, 0): %m"); > + FAIL_EXIT1 ("socket (AF_UNIX, SOCK_STREAM, 0): %m"); > > - xpthread_barrier_wait (&b2); > + set_socket_buffer (tempfd2); > + > + if (connect (tempfd2, (struct sockaddr *) &sun, sizeof (sun)) != 0) > + FAIL_EXIT1 ("connect: %m"); > + > + unlink (sun.sun_path); > > xpthread_barrier_wait (&b2); > > + if (arg != NULL) > + xpthread_barrier_wait (&b2); > + > pthread_cleanup_push (cl, NULL); > > - char mem[1]; > + char mem[WRITE_BUFFER_SIZE]; > > - sendto (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0, > - (struct sockaddr *) &sun, > - offsetof (struct sockaddr_un, sun_path) + strlen (sun.sun_path) + 1); > + sendto (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0, NULL, 0); > > pthread_cleanup_pop (0); > > diff --git a/sysdeps/unix/sysv/linux/mips/mips64/Makefile b/sysdeps/unix/sysv/linux/mips/mips64/Makefile > index b4fb190..fcb48c0 100644 > --- a/sysdeps/unix/sysv/linux/mips/mips64/Makefile > +++ b/sysdeps/unix/sysv/linux/mips/mips64/Makefile > @@ -1,13 +1,3 @@ > -ifeq ($(subdir),socket) > -CFLAGS-recv.c += -fexceptions > -CFLAGS-send.c += -fexceptions > -endif > - > -ifeq ($(subdir),nptl) > -CFLAGS-recv.c += -fexceptions > -CFLAGS-send.c += -fexceptions > -endif > - > ifeq ($(subdir),signal) > # sigaction.c defines static functions in asms and refers to them from > # C code, resulting in "'restore_rt' used but never defined" (which > diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/Makefile b/sysdeps/unix/sysv/linux/riscv/rv64/Makefile > deleted file mode 100644 > index cb60d74..0000000 > --- a/sysdeps/unix/sysv/linux/riscv/rv64/Makefile > +++ /dev/null > @@ -1,4 +0,0 @@ > -ifeq ($(subdir),socket) > -CFLAGS-recv.c += -fexceptions > -CFLAGS-send.c += -fexceptions > -endif >
diff --git a/nptl/tst-cancel4-common.c b/nptl/tst-cancel4-common.c index 5bc7e44..c6eee73 100644 --- a/nptl/tst-cancel4-common.c +++ b/nptl/tst-cancel4-common.c @@ -20,29 +20,13 @@ static int do_test (void) { - int val; - socklen_t len; - if (socketpair (AF_UNIX, SOCK_STREAM, PF_UNIX, fds) != 0) { perror ("socketpair"); exit (1); } - val = 1; - len = sizeof(val); - setsockopt (fds[1], SOL_SOCKET, SO_SNDBUF, &val, sizeof(val)); - if (getsockopt (fds[1], SOL_SOCKET, SO_SNDBUF, &val, &len) < 0) - { - perror ("getsockopt"); - exit (1); - } - if (val >= WRITE_BUFFER_SIZE) - { - puts ("minimum write buffer size too large"); - exit (1); - } - setsockopt (fds[1], SOL_SOCKET, SO_SNDBUF, &val, sizeof(val)); + set_socket_buffer (fds[1]); if (mktemp (fifoname) == NULL) { diff --git a/nptl/tst-cancel4-common.h b/nptl/tst-cancel4-common.h index a526c0c..10cc4f3 100644 --- a/nptl/tst-cancel4-common.h +++ b/nptl/tst-cancel4-common.h @@ -62,6 +62,20 @@ static pthread_barrier_t b2; #define WRITE_BUFFER_SIZE 16384 +/* Set the send buffer of socket S to 1 byte so any send operation + done with WRITE_BUFFER_SIZE bytes will force syscall blocking. */ +static void +set_socket_buffer (int s) +{ + int val = 1; + socklen_t len = sizeof(val); + + TEST_VERIFY_EXIT (setsockopt (s, SOL_SOCKET, SO_SNDBUF, &val, + sizeof(val)) == 0); + TEST_VERIFY_EXIT (getsockopt (s, SOL_SOCKET, SO_SNDBUF, &val, &len) == 0); + TEST_VERIFY_EXIT (val < WRITE_BUFFER_SIZE); +} + /* Cleanup handling test. */ static int cl_called; diff --git a/nptl/tst-cancel4.c b/nptl/tst-cancel4.c index 92a3d80..0532538 100644 --- a/nptl/tst-cancel4.c +++ b/nptl/tst-cancel4.c @@ -726,6 +726,8 @@ tf_send (void *arg) if (tempfd2 == -1) FAIL_EXIT1 ("socket (AF_UNIX, SOCK_STREAM, 0): %m"); + set_socket_buffer (tempfd2); + if (connect (tempfd2, (struct sockaddr *) &sun, sizeof (sun)) != 0) FAIL_EXIT1 ("connect: %m"); @@ -738,8 +740,7 @@ tf_send (void *arg) pthread_cleanup_push (cl, NULL); - /* Very large block, so that the send call blocks. */ - char mem[700000]; + char mem[WRITE_BUFFER_SIZE]; send (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0); @@ -1230,16 +1231,11 @@ tf_msync (void *arg) static void * tf_sendto (void *arg) { - if (arg == NULL) - // XXX If somebody can provide a portable test case in which sendto() - // blocks we can enable this test to run in both rounds. - abort (); - struct sockaddr_un sun; - tempfd = socket (AF_UNIX, SOCK_DGRAM, 0); + tempfd = socket (AF_UNIX, SOCK_STREAM, 0); if (tempfd == -1) - FAIL_EXIT1 ("socket (AF_UNIX, SOCK_DGRAM, 0): %m"); + FAIL_EXIT1 ("socket (AF_UNIX, SOCK_STREAM, 0): %m"); int tries = 0; do @@ -1254,23 +1250,30 @@ tf_sendto (void *arg) while (bind (tempfd, (struct sockaddr *) &sun, offsetof (struct sockaddr_un, sun_path) + strlen (sun.sun_path) + 1) != 0); - tempfname = strdup (sun.sun_path); - tempfd2 = socket (AF_UNIX, SOCK_DGRAM, 0); + listen (tempfd, 5); + + tempfd2 = socket (AF_UNIX, SOCK_STREAM, 0); if (tempfd2 == -1) - FAIL_EXIT1 ("socket (AF_UNIX, SOCK_DGRAM, 0): %m"); + FAIL_EXIT1 ("socket (AF_UNIX, SOCK_STREAM, 0): %m"); - xpthread_barrier_wait (&b2); + set_socket_buffer (tempfd2); + + if (connect (tempfd2, (struct sockaddr *) &sun, sizeof (sun)) != 0) + FAIL_EXIT1 ("connect: %m"); + + unlink (sun.sun_path); xpthread_barrier_wait (&b2); + if (arg != NULL) + xpthread_barrier_wait (&b2); + pthread_cleanup_push (cl, NULL); - char mem[1]; + char mem[WRITE_BUFFER_SIZE]; - sendto (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0, - (struct sockaddr *) &sun, - offsetof (struct sockaddr_un, sun_path) + strlen (sun.sun_path) + 1); + sendto (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0, NULL, 0); pthread_cleanup_pop (0); diff --git a/sysdeps/unix/sysv/linux/mips/mips64/Makefile b/sysdeps/unix/sysv/linux/mips/mips64/Makefile index b4fb190..fcb48c0 100644 --- a/sysdeps/unix/sysv/linux/mips/mips64/Makefile +++ b/sysdeps/unix/sysv/linux/mips/mips64/Makefile @@ -1,13 +1,3 @@ -ifeq ($(subdir),socket) -CFLAGS-recv.c += -fexceptions -CFLAGS-send.c += -fexceptions -endif - -ifeq ($(subdir),nptl) -CFLAGS-recv.c += -fexceptions -CFLAGS-send.c += -fexceptions -endif - ifeq ($(subdir),signal) # sigaction.c defines static functions in asms and refers to them from # C code, resulting in "'restore_rt' used but never defined" (which diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/Makefile b/sysdeps/unix/sysv/linux/riscv/rv64/Makefile deleted file mode 100644 index cb60d74..0000000 --- a/sysdeps/unix/sysv/linux/riscv/rv64/Makefile +++ /dev/null @@ -1,4 +0,0 @@ -ifeq ($(subdir),socket) -CFLAGS-recv.c += -fexceptions -CFLAGS-send.c += -fexceptions -endif