misc/tst-ttyname time outs

Message ID 87ef9uvuz0.fsf@oldenburg2.str.redhat.com
State Committed
Headers

Commit Message

Florian Weimer Jan. 3, 2019, 11:30 a.m. UTC
  * Szabolcs Nagy:

> On 02/01/2019 18:51, Florian Weimer wrote:
>> * Carlos O'Donell:
>>> "info:  posix_openpt: Failed with errno %d. Consider increasing limits."
>> 
>> Yes, that's what I meant.
>> 
>> I don't think we should start printing sysctls and mount options here
>> (or systemd-nspawn arguments).
>
> makes sense to me.
>
>> This ENOSPC comes from the max= mount option on the devpts instance, I
>> think.  So it's different.
>
> thanks, this was not clear to me.
>
> EMFILE happens here when the slavename pts number > 1023,
> i don't know how those numbers are allocated and how to
> make them smaller.
>
> [pid 16749] newfstatat(AT_FDCWD, "/dev/pts/1852", 0xffffdb4ff040, 0) = -1 ENOENT (No such file or directory)
> [pid 16749] openat(AT_FDCWD, "/dev/ptmx", O_RDWR|O_NOCTTY|O_NONBLOCK) = 1022
> [pid 16749] newfstatat(AT_FDCWD, "/dev/pts/1852", 0xffffdb4ff040, 0) = -1 ENOENT (No such file or directory)
> [pid 16749] openat(AT_FDCWD, "/dev/ptmx", O_RDWR|O_NOCTTY|O_NONBLOCK) = 1023
> [pid 16749] newfstatat(AT_FDCWD, "/dev/pts/1852", 0xffffdb4ff040, 0) = -1 ENOENT (No such file or directory)
> [pid 16749] openat(AT_FDCWD, "/dev/ptmx", O_RDWR|O_NOCTTY|O_NONBLOCK) = -1 EMFILE (Too many open files)
> [pid 16749] newfstatat(AT_FDCWD, "/dev/pts/1852", 0xffffdb4ff040, 0) = -1 ENOENT (No such file or directory)
> [pid 16749] openat(AT_FDCWD, "/dev/ptmx", O_RDWR|O_NOCTTY|O_NONBLOCK) = -1 EMFILE (Too many open files)
> ...
>
> ENOSPC happens when i run multiple instances of this test
> (with --direct so they are in infinite loop) so pty limit
> explains it:
>
> $ sysctl kernel.pty
> kernel.pty.max = 4096
> kernel.pty.nr = 1864
> kernel.pty.reserve = 1024
>
> mount options:
> udev on /dev type devtmpfs (rw,nosuid,relatime,size=65955416k,nr_inodes=16488854,mode=755)
> devpts on /dev/pts type devpts (rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000)

ENOSPC is tough to deal with here.  I suspect it involves asynchronous
namespace deallocation (so the next run of the test starts before the
previous namespace is gone).

For EMFILE, please try the attached patch.  It should help.

It also guards against the infinite loop.

Thanks,
Florian

Subject: [PATCH] Linux: Improve handling of resource limits in misc/tst-ttyname

An attempt to re-create a different PTY under the same name can fail
if the PTY has a very high number.  Try to increase the file
descriptor limit in this case, and bail out if this still does not
allow the test to proceed.

2019-01-03  Florian Weimer  <fweimer@redhat.com>

	* sysdeps/unix/sysv/linux/tst-ttyname.c (adjust_file_limit): New
	function.
	(do_in_chroot_1): Call it.
	(run_chroot_tests):
	Improve error reporting in case it is not possible to create a
	collision for the PTY name required by the test.
  

Comments

Szabolcs Nagy Jan. 3, 2019, 1:52 p.m. UTC | #1
On 03/01/2019 11:30, Florian Weimer wrote:
> ENOSPC is tough to deal with here.  I suspect it involves asynchronous

> namespace deallocation (so the next run of the test starts before the

> previous namespace is gone).

> 

> For EMFILE, please try the attached patch.  It should help.

> 

> It also guards against the infinite loop.

> 

> Thanks,

> Florian

> 

> Subject: [PATCH] Linux: Improve handling of resource limits in misc/tst-ttyname

> 

> An attempt to re-create a different PTY under the same name can fail

> if the PTY has a very high number.  Try to increase the file

> descriptor limit in this case, and bail out if this still does not

> allow the test to proceed.

> 

> 2019-01-03  Florian Weimer  <fweimer@redhat.com>

> 

> 	* sysdeps/unix/sysv/linux/tst-ttyname.c (adjust_file_limit): New

> 	function.

> 	(do_in_chroot_1): Call it.

> 	(run_chroot_tests):

> 	Improve error reporting in case it is not possible to create a

> 	collision for the PTY name required by the test.

> 


this works for me, i get

$ cat misc/tst-ttyname.test-result
UNSUPPORTED: misc/tst-ttyname
original exit status 77
$ cat misc/tst-ttyname.out
info:  entering chroot 1
info: adjusting RLIMIT_NOFILE from 1024 to 1671
info:    testcase: basic smoketest
info:      ttyname: PASS {name="/dev/pts/1661", errno=0}
info:      ttyname_r: PASS {name="/dev/pts/1661", ret=0, errno=0}
info:    testcase: no conflict, no match
info:      ttyname: PASS {name=NULL, errno=19}
info:      ttyname_r: PASS {name=NULL, ret=19, errno=19}
info:    testcase: no conflict, console
info:      ttyname: PASS {name="/dev/console", errno=0}
info:      ttyname_r: PASS {name="/dev/console", ret=0, errno=0}
error: ../sysdeps/unix/sysv/linux/tst-ttyname.c:486: cannot re-create PTY "/dev/pts/1661" in chroot: No space left on device (consider
increasing limits)

> +	if (posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK) < 0)

> +	  {

> +	    if (errno == ENOSPC || errno == EMFILE || errno == ENOSPC)


ENOSPC is checked twice.

> +	      FAIL_UNSUPPORTED ("cannot re-create PTY \"%s\" in chroot: %m"

> +				" (consider increasing limits)", slavename);

> +	    else

> +	      FAIL_EXIT1 ("cannot re-create PTY \"%s\" chroot: %m", slavename);

> +	  }

> +      }
  
Florian Weimer Jan. 3, 2019, 1:57 p.m. UTC | #2
* Szabolcs Nagy:

> On 03/01/2019 11:30, Florian Weimer wrote:
>> ENOSPC is tough to deal with here.  I suspect it involves asynchronous
>> namespace deallocation (so the next run of the test starts before the
>> previous namespace is gone).
>> 
>> For EMFILE, please try the attached patch.  It should help.
>> 
>> It also guards against the infinite loop.
>> 
>> Thanks,
>> Florian
>> 
>> Subject: [PATCH] Linux: Improve handling of resource limits in misc/tst-ttyname
>> 
>> An attempt to re-create a different PTY under the same name can fail
>> if the PTY has a very high number.  Try to increase the file
>> descriptor limit in this case, and bail out if this still does not
>> allow the test to proceed.
>> 
>> 2019-01-03  Florian Weimer  <fweimer@redhat.com>
>> 
>> 	* sysdeps/unix/sysv/linux/tst-ttyname.c (adjust_file_limit): New
>> 	function.
>> 	(do_in_chroot_1): Call it.
>> 	(run_chroot_tests):
>> 	Improve error reporting in case it is not possible to create a
>> 	collision for the PTY name required by the test.
>> 
>
> this works for me, i get
>
> $ cat misc/tst-ttyname.test-result
> UNSUPPORTED: misc/tst-ttyname
> original exit status 77
> $ cat misc/tst-ttyname.out
> info:  entering chroot 1
> info: adjusting RLIMIT_NOFILE from 1024 to 1671
> info:    testcase: basic smoketest
> info:      ttyname: PASS {name="/dev/pts/1661", errno=0}
> info:      ttyname_r: PASS {name="/dev/pts/1661", ret=0, errno=0}
> info:    testcase: no conflict, no match
> info:      ttyname: PASS {name=NULL, errno=19}
> info:      ttyname_r: PASS {name=NULL, ret=19, errno=19}
> info:    testcase: no conflict, console
> info:      ttyname: PASS {name="/dev/console", errno=0}
> info:      ttyname_r: PASS {name="/dev/console", ret=0, errno=0}
> error: ../sysdeps/unix/sysv/linux/tst-ttyname.c:486: cannot re-create PTY "/dev/pts/1661" in chroot: No space left on device (consider
> increasing limits)
>
>> +	if (posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK) < 0)
>> +	  {
>> +	    if (errno == ENOSPC || errno == EMFILE || errno == ENOSPC)
>
> ENOSPC is checked twice.

Thanks, fixed thusly:

-	    if (errno == ENOSPC || errno == EMFILE || errno == ENOSPC)
+	    if (errno == ENOSPC || errno == EMFILE || errno == ENFILE)

Siddhesh, what do you think?  Should we put this into 2.29?

Florian
  
Siddhesh Poyarekar Jan. 3, 2019, 3:24 p.m. UTC | #3
On 03/01/19 7:27 PM, Florian Weimer wrote:
> Thanks, fixed thusly:
> 
> -	    if (errno == ENOSPC || errno == EMFILE || errno == ENOSPC)
> +	    if (errno == ENOSPC || errno == EMFILE || errno == ENFILE)
> 
> Siddhesh, what do you think?  Should we put this into 2.29?

OK for 2.29.

Thanks,
Siddhesh
  

Patch

diff --git a/sysdeps/unix/sysv/linux/tst-ttyname.c b/sysdeps/unix/sysv/linux/tst-ttyname.c
index 5d2a54fbbf..7743699816 100644
--- a/sysdeps/unix/sysv/linux/tst-ttyname.c
+++ b/sysdeps/unix/sysv/linux/tst-ttyname.c
@@ -27,6 +27,7 @@ 
 #include <sys/prctl.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
+#include <sys/resource.h>
 #include <unistd.h>
 
 #include <support/check.h>
@@ -239,6 +240,32 @@  prepare (int argc, char **argv)
 }
 #define PREPARE prepare
 
+/* Adjust the file limit so that we have a chance to open PTY.  */
+static void
+adjust_file_limit (const char *pty)
+{
+  int number = -1;
+  if (sscanf (pty, "/dev/pts/%d", &number) != 1 || number < 0)
+    FAIL_EXIT1 ("invalid PTY name: \"%s\"", pty);
+
+  /* Add a few additional descriptors to cover standard I/O streams
+     etc.  */
+  rlim_t desired_limit = number + 10;
+
+  struct rlimit lim;
+  if (getrlimit (RLIMIT_NOFILE, &lim) != 0)
+    FAIL_EXIT1 ("getrlimit (RLIMIT_NOFILE): %m");
+  if (lim.rlim_cur < desired_limit)
+    {
+      printf ("info: adjusting RLIMIT_NOFILE from %llu to %llu\n",
+	      (unsigned long long int) lim.rlim_cur,
+	      (unsigned long long int) desired_limit);
+      lim.rlim_cur = desired_limit;
+      if (setrlimit (RLIMIT_NOFILE, &lim) != 0)
+	printf ("warning: setrlimit (RLIMIT_NOFILE) failed: %m\n");
+    }
+}
+
 /* These chroot setup functions put the TTY at at "/console" (where it
    won't be found by ttyname), and create "/dev/console" as an
    ordinary file.  This way, it's easier to write test-cases that
@@ -266,6 +293,7 @@  do_in_chroot_1 (int (*cb)(const char *, int))
   if (strncmp (slavename, "/dev/pts/", 9) != 0)
     FAIL_UNSUPPORTED ("slave pseudo-terminal is not under /dev/pts/: %s",
                       slavename);
+  adjust_file_limit (slavename);
   int slave = xopen (slavename, O_RDWR, 0);
   if (!doit (slave, "basic smoketest",
              (struct result_r){.name=slavename, .ret=0, .err=0}))
@@ -332,6 +360,7 @@  do_in_chroot_2 (int (*cb)(const char *, int))
   if (strncmp (slavename, "/dev/pts/", 9) != 0)
     FAIL_UNSUPPORTED ("slave pseudo-terminal is not under /dev/pts/: %s",
                       slavename);
+  adjust_file_limit (slavename);
   /* wait until in a new mount ns to open the slave */
 
   /* enable `wait`ing on grandchildren */
@@ -445,10 +474,20 @@  run_chroot_tests (const char *slavename, int slave)
       ok = false;
     VERIFY (umount ("/dev/console") == 0);
 
-    /* keep creating PTYs until we we get a name collision */
-    while (stat (slavename, &st) < 0)
-      posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK);
-    VERIFY (stat (slavename, &st) == 0);
+    /* Keep creating PTYs until we we get a name collision.  */
+    while (true)
+      {
+	if (stat (slavename, &st) == 0)
+	  break;
+	if (posix_openpt (O_RDWR|O_NOCTTY|O_NONBLOCK) < 0)
+	  {
+	    if (errno == ENOSPC || errno == EMFILE || errno == ENOSPC)
+	      FAIL_UNSUPPORTED ("cannot re-create PTY \"%s\" in chroot: %m"
+				" (consider increasing limits)", slavename);
+	    else
+	      FAIL_EXIT1 ("cannot re-create PTY \"%s\" chroot: %m", slavename);
+	  }
+      }
 
     if (!doit (slave, "conflict, no match",
                (struct result_r){.name=NULL, .ret=ENODEV, .err=ENODEV}))