diff mbox series

nptl: Fix tst-cancel4 sendto tests

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

Commit Message

Adhemerval Zanella Feb. 22, 2018, 7:38 p.m. UTC 
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

-- 
2.7.4

Comments

Adhemerval Zanella March 2, 2018, 6:27 p.m. UTC | #1 
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 mbox series

Patch

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