All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] python3: Avoid hanging test
@ 2019-04-08 11:59 Richard Purdie
  2019-04-08 11:59 ` [PATCH 2/3] openssh/util-linux/python*: Ensure ptest output is unbuffered Richard Purdie
  2019-04-08 11:59 ` [PATCH 3/3] ptest-runner: Add several logging fixes Richard Purdie
  0 siblings, 2 replies; 4+ messages in thread
From: Richard Purdie @ 2019-04-08 11:59 UTC (permalink / raw)
  To: openembedded-core

There is a python test which hangs with recent kernels, 5.0 onwards. This causes
ptest to timeout for python3. Disable the problematic test until we better understand
the real cause and fix of the issue (discussions are happening with upstream).

See the patch for details/links.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 .../python/python3/ptesthack.patch            | 49 +++++++++++++++++++
 meta/recipes-devtools/python/python3_3.7.2.bb |  1 +
 2 files changed, 50 insertions(+)
 create mode 100644 meta/recipes-devtools/python/python3/ptesthack.patch

diff --git a/meta/recipes-devtools/python/python3/ptesthack.patch b/meta/recipes-devtools/python/python3/ptesthack.patch
new file mode 100644
index 00000000000..3f5589645ed
--- /dev/null
+++ b/meta/recipes-devtools/python/python3/ptesthack.patch
@@ -0,0 +1,49 @@
+This test hangs under 5.0 kernels onwards. It appears to be caused by the commit in the kernel:
+
+commit 4f693b55c3d2d2239b8a0094b518a1e533cf75d5 (HEAD, refs/bisect/bad)
+Author: Eric Dumazet <edumazet@google.com>
+Date:   Tue Nov 27 14:42:03 2018 -0800
+
+    tcp: implement coalescing on backlog queue
+    
+    In case GRO is not as efficient as it should be or disabled,
+    we might have a user thread trapped in __release_sock() while
+    softirq handler flood packets up to the point we have to drop.
+    
+    This patch balances work done from user thread and softirq,
+    to give more chances to __release_sock() to complete its work
+    before new packets are added the the backlog.
+    
+    This also helps if we receive many ACK packets, since GRO
+    does not aggregate them.
+    
+    This patch brings ~60% throughput increase on a receiver
+    without GRO, but the spectacular gain is really on
+    1000x release_sock() latency reduction I have measured.
+    
+    Signed-off-by: Eric Dumazet <edumazet@google.com>
+    Cc: Neal Cardwell <ncardwell@google.com>
+    Cc: Yuchung Cheng <ycheng@google.com>
+    Acked-by: Neal Cardwell <ncardwell@google.com>
+    Signed-off-by: David S. Miller <davem@davemloft.net>
+
+
+Reported to upstream kernel for advice: https://lore.kernel.org/netdev/85aabf9d4f41b6c57629e736993233f80a037e59.camel@linuxfoundation.org/T/#u
+
+Disable the test for now to stop ptests hanging
+
+Upstream-Status: Inappropriate [real cause of issue still TBD]
+
+Index: Python-3.7.2/Lib/test/test_httplib.py
+===================================================================
+--- Python-3.7.2.orig/Lib/test/test_httplib.py
++++ Python-3.7.2/Lib/test/test_httplib.py
+@@ -1114,7 +1114,7 @@ class BasicTest(TestCase):
+         self.assertEqual(sock.file.read(), extradata) #we read to the end
+         resp.close()
+ 
+-    def test_response_fileno(self):
++    def atest_response_fileno(self):
+         # Make sure fd returned by fileno is valid.
+         serv = socket.socket(
+             socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_TCP)
diff --git a/meta/recipes-devtools/python/python3_3.7.2.bb b/meta/recipes-devtools/python/python3_3.7.2.bb
index 5c64bc8aa2a..3d44bdca911 100644
--- a/meta/recipes-devtools/python/python3_3.7.2.bb
+++ b/meta/recipes-devtools/python/python3_3.7.2.bb
@@ -21,6 +21,7 @@ SRC_URI = "http://www.python.org/ftp/python/${PV}/Python-${PV}.tar.xz \
            file://0001-python3-use-cc_basename-to-replace-CC-for-checking-c.patch \
            file://0002-Don-t-do-runtime-test-to-get-float-byte-order.patch \
            file://0003-setup.py-pass-missing-libraries-to-Extension-for-mul.patch \
+           file://ptesthack.patch \
            "
 
 SRC_URI_append_class-native = " \
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] openssh/util-linux/python*: Ensure ptest output is unbuffered
  2019-04-08 11:59 [PATCH 1/3] python3: Avoid hanging test Richard Purdie
@ 2019-04-08 11:59 ` Richard Purdie
  2019-04-08 11:59 ` [PATCH 3/3] ptest-runner: Add several logging fixes Richard Purdie
  1 sibling, 0 replies; 4+ messages in thread
From: Richard Purdie @ 2019-04-08 11:59 UTC (permalink / raw)
  To: openembedded-core

We need to run sed with the -u option to ensure the output is unbuffered else
ptest-runner may timeout thinkig things were idle. Busybox doesn't have the -u
option so we need to RDEPEND on sed (which is a good thing to do if we use it
anyway).

Alex Kanavin should get credit for discovering the problem.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/recipes-connectivity/openssh/openssh/run-ptest | 2 +-
 meta/recipes-connectivity/openssh/openssh_7.9p1.bb  | 2 +-
 meta/recipes-core/util-linux/util-linux.inc         | 2 +-
 meta/recipes-core/util-linux/util-linux/run-ptest   | 2 +-
 meta/recipes-devtools/python/python/run-ptest       | 2 +-
 meta/recipes-devtools/python/python3/run-ptest      | 2 +-
 meta/recipes-devtools/python/python3_3.7.2.bb       | 2 +-
 meta/recipes-devtools/python/python_2.7.15.bb       | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/meta/recipes-connectivity/openssh/openssh/run-ptest b/meta/recipes-connectivity/openssh/openssh/run-ptest
index 36a3d2a7b7b..daf62cca5b1 100755
--- a/meta/recipes-connectivity/openssh/openssh/run-ptest
+++ b/meta/recipes-connectivity/openssh/openssh/run-ptest
@@ -5,7 +5,7 @@ export TEST_SHELL=sh
 cd regress
 sed -i "/\t\tagent-ptrace /d" Makefile
 make -k .OBJDIR=`pwd` .CURDIR=`pwd` SUDO="sudo" tests \
-        | sed -e 's/^skipped/SKIP: /g' -e 's/^ok /PASS: /g' -e 's/^failed/FAIL: /g'
+        | sed -u -e 's/^skipped/SKIP: /g' -e 's/^ok /PASS: /g' -e 's/^failed/FAIL: /g'
 
 SSHAGENT=`which ssh-agent`
 GDB=`which gdb`
diff --git a/meta/recipes-connectivity/openssh/openssh_7.9p1.bb b/meta/recipes-connectivity/openssh/openssh_7.9p1.bb
index 2a23f64b894..6260135d5b2 100644
--- a/meta/recipes-connectivity/openssh/openssh_7.9p1.bb
+++ b/meta/recipes-connectivity/openssh/openssh_7.9p1.bb
@@ -144,7 +144,7 @@ FILES_${PN}-keygen = "${bindir}/ssh-keygen"
 
 RDEPENDS_${PN} += "${PN}-scp ${PN}-ssh ${PN}-sshd ${PN}-keygen"
 RDEPENDS_${PN}-sshd += "${PN}-keygen ${@bb.utils.contains('DISTRO_FEATURES', 'pam', 'pam-plugin-keyinit pam-plugin-loginuid', '', d)}"
-RDEPENDS_${PN}-ptest += "${PN}-sftp ${PN}-misc ${PN}-sftp-server make"
+RDEPENDS_${PN}-ptest += "${PN}-sftp ${PN}-misc ${PN}-sftp-server make sed"
 
 RPROVIDES_${PN}-ssh = "ssh"
 RPROVIDES_${PN}-sshd = "sshd"
diff --git a/meta/recipes-core/util-linux/util-linux.inc b/meta/recipes-core/util-linux/util-linux.inc
index 18c3af240eb..c7ba8c446f1 100644
--- a/meta/recipes-core/util-linux/util-linux.inc
+++ b/meta/recipes-core/util-linux/util-linux.inc
@@ -142,7 +142,7 @@ RDEPENDS_${PN}_class-nativesdk = ""
 RPROVIDES_${PN}-dev = "${PN}-libblkid-dev ${PN}-libmount-dev ${PN}-libuuid-dev"
 
 RDEPENDS_${PN}-bash-completion += "${PN}-lsblk"
-RDEPENDS_${PN}-ptest = "bash grep coreutils which btrfs-tools ${PN}"
+RDEPENDS_${PN}-ptest = "bash grep coreutils which btrfs-tools ${PN} sed"
 RDEPENDS_${PN}-swaponoff = "${PN}-swapon ${PN}-swapoff"
 ALLOW_EMPTY_${PN}-swaponoff = "1"
 
diff --git a/meta/recipes-core/util-linux/util-linux/run-ptest b/meta/recipes-core/util-linux/util-linux/run-ptest
index fbc2f9b56a9..8c57bd20740 100644
--- a/meta/recipes-core/util-linux/util-linux/run-ptest
+++ b/meta/recipes-core/util-linux/util-linux/run-ptest
@@ -16,7 +16,7 @@ res=0
 count=0
 for ts in $comps; 
 do 
-   $ts | sed '{        
+   $ts | sed -u '{        
       s/^\(.*\):\(.*\) \.\.\. OK$/PASS: \1:\2/                              
       s/^\(.*\):\(.*\) \.\.\. FAILED \(.*\)$/FAIL: \1:\2 \3/                
       s/^\(.*\):\(.*\) \.\.\. SKIPPED \(.*\)$/SKIP: \1:\2 \3/               
diff --git a/meta/recipes-devtools/python/python/run-ptest b/meta/recipes-devtools/python/python/run-ptest
index 13dfc99efd5..c7002a4560c 100644
--- a/meta/recipes-devtools/python/python/run-ptest
+++ b/meta/recipes-devtools/python/python/run-ptest
@@ -1,3 +1,3 @@
 #!/bin/sh
 
-python -mtest -W | sed -e '/\.\.\. ok/ s/^/PASS: /g' -e '/\.\.\. [ERROR|FAIL]/ s/^/FAIL: /g' -e '/\.\.\. skipped/ s/^/SKIP: /g' -e 's/ \.\.\. ok//g' -e 's/ \.\.\. ERROR//g' -e 's/ \.\.\. FAIL//g' -e 's/ \.\.\. skipped//g'
+python -mtest -W | sed -u -e '/\.\.\. ok/ s/^/PASS: /g' -e '/\.\.\. [ERROR|FAIL]/ s/^/FAIL: /g' -e '/\.\.\. skipped/ s/^/SKIP: /g' -e 's/ \.\.\. ok//g' -e 's/ \.\.\. ERROR//g' -e 's/ \.\.\. FAIL//g' -e 's/ \.\.\. skipped//g'
diff --git a/meta/recipes-devtools/python/python3/run-ptest b/meta/recipes-devtools/python/python3/run-ptest
index 20c9274dfa7..50f92916eb6 100644
--- a/meta/recipes-devtools/python/python3/run-ptest
+++ b/meta/recipes-devtools/python/python3/run-ptest
@@ -1,3 +1,3 @@
 #!/bin/sh
 
-python3 -m test -W | sed -e '/\.\.\. ok/ s/^/PASS: /g' -e '/\.\.\. [ERROR|FAIL]/ s/^/FAIL: /g' -e '/\.\.\. skipped/ s/^/SKIP: /g' -e 's/ \.\.\. ok//g' -e 's/ \.\.\. ERROR//g' -e 's/ \.\.\. FAIL//g' -e 's/ \.\.\. skipped//g'
+python3 -m test -W | sed -u -e '/\.\.\. ok/ s/^/PASS: /g' -e '/\.\.\. [ERROR|FAIL]/ s/^/FAIL: /g' -e '/\.\.\. skipped/ s/^/SKIP: /g' -e 's/ \.\.\. ok//g' -e 's/ \.\.\. ERROR//g' -e 's/ \.\.\. FAIL//g' -e 's/ \.\.\. skipped//g'
diff --git a/meta/recipes-devtools/python/python3_3.7.2.bb b/meta/recipes-devtools/python/python3_3.7.2.bb
index 3d44bdca911..4ff8cff3a1f 100644
--- a/meta/recipes-devtools/python/python3_3.7.2.bb
+++ b/meta/recipes-devtools/python/python3_3.7.2.bb
@@ -285,7 +285,7 @@ FILES_${PN}-misc = "${libdir}/python${PYTHON_MAJMIN} ${libdir}/python${PYTHON_MA
 PACKAGES += "${PN}-man"
 FILES_${PN}-man = "${datadir}/man"
 
-RDEPENDS_${PN}-ptest = "${PN}-modules ${PN}-tests unzip bzip2 libgcc tzdata-europe coreutils"
+RDEPENDS_${PN}-ptest = "${PN}-modules ${PN}-tests unzip bzip2 libgcc tzdata-europe coreutils sed"
 RDEPENDS_${PN}-tkinter += "${@bb.utils.contains('PACKAGECONFIG', 'tk', 'tk', '', d)}"
 RDEPENDS_${PN}-dev = ""
 
diff --git a/meta/recipes-devtools/python/python_2.7.15.bb b/meta/recipes-devtools/python/python_2.7.15.bb
index c459af06f10..62051a227b8 100644
--- a/meta/recipes-devtools/python/python_2.7.15.bb
+++ b/meta/recipes-devtools/python/python_2.7.15.bb
@@ -176,7 +176,7 @@ FILES_${PN}-misc = "${libdir}/python${PYTHON_MAJMIN}"
 RDEPENDS_${PN}-modules += "${PN}-misc"
 
 # ptest
-RDEPENDS_${PN}-ptest = "${PN}-modules ${PN}-tests unzip tzdata-europe coreutils"
+RDEPENDS_${PN}-ptest = "${PN}-modules ${PN}-tests unzip tzdata-europe coreutils sed"
 RDEPENDS_${PN}-tkinter += "${@bb.utils.contains('PACKAGECONFIG', 'tk', 'tk', '', d)}"
 # catch manpage
 PACKAGES += "${PN}-man"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] ptest-runner: Add several logging fixes
  2019-04-08 11:59 [PATCH 1/3] python3: Avoid hanging test Richard Purdie
  2019-04-08 11:59 ` [PATCH 2/3] openssh/util-linux/python*: Ensure ptest output is unbuffered Richard Purdie
@ 2019-04-08 11:59 ` Richard Purdie
  2019-06-06 20:26   ` Randy MacLeod
  1 sibling, 1 reply; 4+ messages in thread
From: Richard Purdie @ 2019-04-08 11:59 UTC (permalink / raw)
  To: openembedded-core

This change adds three patches to improve the handling of stdout/stderr and child
processes to try and improve logging reliability in ptest-runner.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 ...ils-Ensure-stdout-stderr-are-flushed.patch | 45 +++++++++++
 ...002-use-process-groups-when-spawning.patch | 35 +++++++++
 ...ils-Ensure-pipes-are-read-after-exit.patch | 76 +++++++++++++++++++
 .../ptest-runner/ptest-runner_2.3.1.bb        |  6 +-
 4 files changed, 161 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch
 create mode 100644 meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch
 create mode 100644 meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch

diff --git a/meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch b/meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch
new file mode 100644
index 00000000000..c9a9dd7cf49
--- /dev/null
+++ b/meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch
@@ -0,0 +1,45 @@
+From 9b36993794c1de733c521b2477370c874c07b617 Mon Sep 17 00:00:00 2001
+From: Richard Purdie <richard.purdie@linuxfoundation.org>
+Date: Thu, 4 Apr 2019 14:18:55 +0100
+Subject: [PATCH 1/3] utils: Ensure stdout/stderr are flushed
+
+There is no guarantee that the data written with fwrite will be flushed to the
+buffer. If stdout and stderr are the same thing, this could lead to interleaved
+writes. The common case is stdout output so flush the output pipes when writing to
+stderr. Also flush stdout before the function returns.
+
+Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
+Upstream-Status: Pending [code being tested]
+---
+ utils.c | 7 +++++--
+ 1 file changed, 5 insertions(+), 2 deletions(-)
+
+diff --git a/utils.c b/utils.c
+index 504df0b..3ceb342 100644
+--- a/utils.c
++++ b/utils.c
+@@ -295,8 +295,11 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
+ 			}
+ 
+ 			if (pfds[1].revents != 0) {
+-				while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0)
++				while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0) {
++					fflush(fps[0]);
+ 					fwrite(buf, n, 1, fps[1]);
++					fflush(fps[1]);
++				}
+ 			}
+ 
+ 			clock_gettime(clock, &sentinel);
+@@ -315,7 +318,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
+ 			break;
+ 	}
+ 
+-
++	fflush(fps[0]);
+ 	return status;
+ }
+ 
+-- 
+2.17.1
+
diff --git a/meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch b/meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch
new file mode 100644
index 00000000000..5436a3340c4
--- /dev/null
+++ b/meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch
@@ -0,0 +1,35 @@
+From f0c42a65633341ad048718c7a6dbd035818e9eaf Mon Sep 17 00:00:00 2001
+From: Richard Purdie <richard.purdie@linuxfoundation.org>
+Date: Thu, 4 Apr 2019 14:20:31 +0100
+Subject: [PATCH 2/3] use process groups when spawning
+
+Rather than just killing the process we've swawned, set the process group
+for spawned children and then kill the group of processes.
+
+Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
+Upstream-Status: Pending [code being tested]
+---
+ utils.c | 9 +++++----
+ 1 file changed, 5 insertions(+), 4 deletions(-)
+
+diff --git a/utils.c b/utils.c
+index 3ceb342..c5b3b8d 100644
+--- a/utils.c
++++ b/utils.c
+@@ -309,7 +309,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
+ 			clock_gettime(clock, &time);
+ 			if ((time.tv_sec - sentinel.tv_sec) > timeout) {
+ 				*timeouted = 1;
+-				kill(pid, SIGKILL);
++				kill(-pid, SIGKILL);
+ 				waitflags = 0;
+ 			}
+ 		}
+@@ -371,6 +371,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
+ 				rc = -1;
+ 				break;
+ 			} else if (child == 0) {
++				setsid();
+ 				run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]);
+ 			} else {
+ 				int status;
diff --git a/meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch b/meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch
new file mode 100644
index 00000000000..f7c3ebe6f2c
--- /dev/null
+++ b/meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch
@@ -0,0 +1,76 @@
+From e58e4e1a7f854953f823dc5135d35f728f253f31 Mon Sep 17 00:00:00 2001
+From: Richard Purdie <richard.purdie@linuxfoundation.org>
+Date: Thu, 4 Apr 2019 14:24:14 +0100
+Subject: [PATCH 3/3] utils: Ensure pipes are read after exit
+
+There was a race in the code where the pipes may not be read after the process has exited
+and data may be left behind in them. This change to ordering ensures the pipes are read
+after the exit code has been read meaning no data can be left behind and the logs should
+be complete.
+
+Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
+Upstream-Status: Pending [code being tested]
+---
+ utils.c | 29 ++++++++++++++++-------------
+ 1 file changed, 16 insertions(+), 13 deletions(-)
+
+diff --git a/utils.c b/utils.c
+index c5b3b8d..37e88ab 100644
+--- a/utils.c
++++ b/utils.c
+@@ -264,6 +264,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group,
+ 	struct pollfd pfds[2];
+ 	struct timespec sentinel;
+ 	clockid_t clock = CLOCK_MONOTONIC;
++	int looping = 1;
+ 	int r;
+ 
+ 	int status;
+@@ -281,9 +282,23 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group,
+ 
+ 	*timeouted = 0;
+ 
+-	while (1) {
++	while (looping) {
+ 		waitflags = WNOHANG;
+ 
++		if (timeout >= 0) {
++			struct timespec time;
++
++			clock_gettime(clock, &time);
++			if ((time.tv_sec - sentinel.tv_sec) > timeout) {
++				*timeouted = 1;
++				kill(-pid, SIGKILL);
++				waitflags = 0;
++			}
++		}
++
++		if (waitpid(pid, &status, waitflags) == pid)
++			looping = 0;
++
+ 		r = poll(pfds, 2, WAIT_CHILD_POLL_TIMEOUT_MS);
+ 		if (r > 0) {
+ 			char buf[WAIT_CHILD_BUF_MAX_SIZE];
+@@ -303,19 +318,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group,
+ 			}
+ 
+ 			clock_gettime(clock, &sentinel);
+-		} else if (timeout >= 0) {
+-			struct timespec time;
+-
+-			clock_gettime(clock, &time);
+-			if ((time.tv_sec - sentinel.tv_sec) > timeout) {
+-				*timeouted = 1;
+-				kill(-pid, SIGKILL);
+-				waitflags = 0;
+-			}
+ 		}
+-
+-		if (waitpid(pid, &status, waitflags) == pid)
+-			break;
+ 	}
+ 
+ 	fflush(fps[0]);
+-- 
+2.17.1
+
diff --git a/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb b/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
index 4b7992bf2fd..e2eb258d0b0 100644
--- a/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
+++ b/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
@@ -10,7 +10,11 @@ LIC_FILES_CHKSUM = "file://LICENSE;md5=751419260aa954499f7abaabaa882bbe"
 SRCREV = "05b112bda7ac2adba8e9b0f088d6e5843b148a38"
 PV = "2.3.1+git${SRCPV}"
 
-SRC_URI = "git://git.yoctoproject.org/ptest-runner2"
+SRC_URI = "git://git.yoctoproject.org/ptest-runner2 \
+ file://0001-utils-Ensure-stdout-stderr-are-flushed.patch \
+ file://0002-use-process-groups-when-spawning.patch \
+ file://0003-utils-Ensure-pipes-are-read-after-exit.patch"
+
 S = "${WORKDIR}/git"
 
 FILES_${PN} = "${bindir}/ptest-runner"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/3] ptest-runner: Add several logging fixes
  2019-04-08 11:59 ` [PATCH 3/3] ptest-runner: Add several logging fixes Richard Purdie
@ 2019-06-06 20:26   ` Randy MacLeod
  0 siblings, 0 replies; 4+ messages in thread
From: Randy MacLeod @ 2019-06-06 20:26 UTC (permalink / raw)
  To: Richard Purdie, openembedded-core, Sakib Sajal, Aníbal Limón

On 4/8/19 7:59 AM, Richard Purdie wrote:
> This change adds three patches to improve the handling of stdout/stderr and child
> processes to try and improve logging reliability in ptest-runner.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>   ...ils-Ensure-stdout-stderr-are-flushed.patch | 45 +++++++++++
>   ...002-use-process-groups-when-spawning.patch | 35 +++++++++
>   ...ils-Ensure-pipes-are-read-after-exit.patch | 76 +++++++++++++++++++

Sakib and I found another problem with ptest-runner and job control.
When bash runs some ptests, there's a warning:

< bash: cannot set terminal process group (2980): Inappropriate ioctl 
for device
< bash: no job control in this shell

This only happens when being run under ptest-runner instead of say
under /usr/lib/bash/ptest/run-ptest. There may also be some problems
with su'ing to the bash_user account but we'll see.

So we're working on this and we'd like to know if the three patches here
are considered stable and should therefore be merged to ptest-runner?

Sakib is testing a patch to detach in the parent and hand the 
controlling terminal to the child using a couple of
ioctl(0, TIOCSCTTY, NULL) type of calls.

../Randy

>   .../ptest-runner/ptest-runner_2.3.1.bb        |  6 +-
>   4 files changed, 161 insertions(+), 1 deletion(-)
>   create mode 100644 meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch
>   create mode 100644 meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch
>   create mode 100644 meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch
> 
> diff --git a/meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch b/meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch
> new file mode 100644
> index 00000000000..c9a9dd7cf49
> --- /dev/null
> +++ b/meta/recipes-support/ptest-runner/ptest-runner/0001-utils-Ensure-stdout-stderr-are-flushed.patch
> @@ -0,0 +1,45 @@
> +From 9b36993794c1de733c521b2477370c874c07b617 Mon Sep 17 00:00:00 2001
> +From: Richard Purdie <richard.purdie@linuxfoundation.org>
> +Date: Thu, 4 Apr 2019 14:18:55 +0100
> +Subject: [PATCH 1/3] utils: Ensure stdout/stderr are flushed
> +
> +There is no guarantee that the data written with fwrite will be flushed to the
> +buffer. If stdout and stderr are the same thing, this could lead to interleaved
> +writes. The common case is stdout output so flush the output pipes when writing to
> +stderr. Also flush stdout before the function returns.
> +
> +Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> +Upstream-Status: Pending [code being tested]
> +---
> + utils.c | 7 +++++--
> + 1 file changed, 5 insertions(+), 2 deletions(-)
> +
> +diff --git a/utils.c b/utils.c
> +index 504df0b..3ceb342 100644
> +--- a/utils.c
> ++++ b/utils.c
> +@@ -295,8 +295,11 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
> + 			}
> +
> + 			if (pfds[1].revents != 0) {
> +-				while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0)
> ++				while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0) {
> ++					fflush(fps[0]);
> + 					fwrite(buf, n, 1, fps[1]);
> ++					fflush(fps[1]);
> ++				}
> + 			}
> +
> + 			clock_gettime(clock, &sentinel);
> +@@ -315,7 +318,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
> + 			break;
> + 	}
> +
> +-
> ++	fflush(fps[0]);
> + 	return status;
> + }
> +
> +--
> +2.17.1
> +
> diff --git a/meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch b/meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch
> new file mode 100644
> index 00000000000..5436a3340c4
> --- /dev/null
> +++ b/meta/recipes-support/ptest-runner/ptest-runner/0002-use-process-groups-when-spawning.patch
> @@ -0,0 +1,35 @@
> +From f0c42a65633341ad048718c7a6dbd035818e9eaf Mon Sep 17 00:00:00 2001
> +From: Richard Purdie <richard.purdie@linuxfoundation.org>
> +Date: Thu, 4 Apr 2019 14:20:31 +0100
> +Subject: [PATCH 2/3] use process groups when spawning
> +
> +Rather than just killing the process we've swawned, set the process group
> +for spawned children and then kill the group of processes.
> +
> +Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> +Upstream-Status: Pending [code being tested]
> +---
> + utils.c | 9 +++++----
> + 1 file changed, 5 insertions(+), 4 deletions(-)
> +
> +diff --git a/utils.c b/utils.c
> +index 3ceb342..c5b3b8d 100644
> +--- a/utils.c
> ++++ b/utils.c
> +@@ -309,7 +309,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
> + 			clock_gettime(clock, &time);
> + 			if ((time.tv_sec - sentinel.tv_sec) > timeout) {
> + 				*timeouted = 1;
> +-				kill(pid, SIGKILL);
> ++				kill(-pid, SIGKILL);
> + 				waitflags = 0;
> + 			}
> + 		}
> +@@ -371,6 +371,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
> + 				rc = -1;
> + 				break;
> + 			} else if (child == 0) {
> ++				setsid();
> + 				run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]);
> + 			} else {
> + 				int status;
> diff --git a/meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch b/meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch
> new file mode 100644
> index 00000000000..f7c3ebe6f2c
> --- /dev/null
> +++ b/meta/recipes-support/ptest-runner/ptest-runner/0003-utils-Ensure-pipes-are-read-after-exit.patch
> @@ -0,0 +1,76 @@
> +From e58e4e1a7f854953f823dc5135d35f728f253f31 Mon Sep 17 00:00:00 2001
> +From: Richard Purdie <richard.purdie@linuxfoundation.org>
> +Date: Thu, 4 Apr 2019 14:24:14 +0100
> +Subject: [PATCH 3/3] utils: Ensure pipes are read after exit
> +
> +There was a race in the code where the pipes may not be read after the process has exited
> +and data may be left behind in them. This change to ordering ensures the pipes are read
> +after the exit code has been read meaning no data can be left behind and the logs should
> +be complete.
> +
> +Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> +Upstream-Status: Pending [code being tested]
> +---
> + utils.c | 29 ++++++++++++++++-------------
> + 1 file changed, 16 insertions(+), 13 deletions(-)
> +
> +diff --git a/utils.c b/utils.c
> +index c5b3b8d..37e88ab 100644
> +--- a/utils.c
> ++++ b/utils.c
> +@@ -264,6 +264,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group,
> + 	struct pollfd pfds[2];
> + 	struct timespec sentinel;
> + 	clockid_t clock = CLOCK_MONOTONIC;
> ++	int looping = 1;
> + 	int r;
> +
> + 	int status;
> +@@ -281,9 +282,23 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group,
> +
> + 	*timeouted = 0;
> +
> +-	while (1) {
> ++	while (looping) {
> + 		waitflags = WNOHANG;
> +
> ++		if (timeout >= 0) {
> ++			struct timespec time;
> ++
> ++			clock_gettime(clock, &time);
> ++			if ((time.tv_sec - sentinel.tv_sec) > timeout) {
> ++				*timeouted = 1;
> ++				kill(-pid, SIGKILL);
> ++				waitflags = 0;
> ++			}
> ++		}
> ++
> ++		if (waitpid(pid, &status, waitflags) == pid)
> ++			looping = 0;
> ++
> + 		r = poll(pfds, 2, WAIT_CHILD_POLL_TIMEOUT_MS);
> + 		if (r > 0) {
> + 			char buf[WAIT_CHILD_BUF_MAX_SIZE];
> +@@ -303,19 +318,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid, pid_t group,
> + 			}
> +
> + 			clock_gettime(clock, &sentinel);
> +-		} else if (timeout >= 0) {
> +-			struct timespec time;
> +-
> +-			clock_gettime(clock, &time);
> +-			if ((time.tv_sec - sentinel.tv_sec) > timeout) {
> +-				*timeouted = 1;
> +-				kill(-pid, SIGKILL);
> +-				waitflags = 0;
> +-			}
> + 		}
> +-
> +-		if (waitpid(pid, &status, waitflags) == pid)
> +-			break;
> + 	}
> +
> + 	fflush(fps[0]);
> +--
> +2.17.1
> +
> diff --git a/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb b/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
> index 4b7992bf2fd..e2eb258d0b0 100644
> --- a/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
> +++ b/meta/recipes-support/ptest-runner/ptest-runner_2.3.1.bb
> @@ -10,7 +10,11 @@ LIC_FILES_CHKSUM = "file://LICENSE;md5=751419260aa954499f7abaabaa882bbe"
>   SRCREV = "05b112bda7ac2adba8e9b0f088d6e5843b148a38"
>   PV = "2.3.1+git${SRCPV}"
>   
> -SRC_URI = "git://git.yoctoproject.org/ptest-runner2"
> +SRC_URI = "git://git.yoctoproject.org/ptest-runner2 \
> + file://0001-utils-Ensure-stdout-stderr-are-flushed.patch \
> + file://0002-use-process-groups-when-spawning.patch \
> + file://0003-utils-Ensure-pipes-are-read-after-exit.patch"
> +
>   S = "${WORKDIR}/git"
>   
>   FILES_${PN} = "${bindir}/ptest-runner"
> 


-- 
# Randy MacLeod
# Wind River Linux


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-06-06 20:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-08 11:59 [PATCH 1/3] python3: Avoid hanging test Richard Purdie
2019-04-08 11:59 ` [PATCH 2/3] openssh/util-linux/python*: Ensure ptest output is unbuffered Richard Purdie
2019-04-08 11:59 ` [PATCH 3/3] ptest-runner: Add several logging fixes Richard Purdie
2019-06-06 20:26   ` Randy MacLeod

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.