[ptest-runner] util.c: remove the usage of pipe for parent/child communication

Submitted by Yi Fan Yu on Feb. 3, 2021, 11:44 p.m. | Patch ID: 179367

Details

Message ID 20210203234410.6742-1-yifan.yu@windriver.com
State New
Headers show

Commit Message

Yi Fan Yu Feb. 3, 2021, 11:44 p.m.
If the output of the testing script is very large,
the fwrite called by the parent in
wait_child would start blocking. That would slow down
the parent speed in clearing the pipe buffer.

This is an issue because the pipe is Non-Blocking.
so the child(ex: a bash script)
who was designed to output to what he thinks is a
is stdout, might error out due to EAGAIN (pipe full).

The proposed fix is to remove pipes and just let the child
write directly to stdout (same his parent)

This would mean that the parent no longer control the output
of the child.

Signed-off-by: Yi Fan Yu <yifan.yu@windriver.com>
---
 utils.c | 53 ++++++++---------------------------------------------
 1 file changed, 8 insertions(+), 45 deletions(-)

Patch hide | download patch | download mbox

diff --git a/utils.c b/utils.c
index a4e190e..5d62ced 100644
--- a/utils.c
+++ b/utils.c
@@ -274,12 +274,14 @@  run_child(char *run_ptest, int fd_stdout, int fd_stderr)
 	argv[0] = run_ptest;
 	argv[1] = NULL;
 
+	/* since it isn't use by the child, close(fd_stderr) ? */
+	close(fd_stderr);
+
 	dup2(fd_stdout, STDOUT_FILENO);
 	// XXX: Redirect stderr to stdout to avoid buffer ordering problems.
 	dup2(fd_stdout, STDERR_FILENO);
 
-	/* since it isn't use by the child, close(fd_stderr) ? */
-	close(fd_stderr); /* try using to see if this fixes bash run-read. rwm todo */
+
 	close_fds();
 
 	execv(run_ptest, argv);
@@ -290,20 +292,13 @@  run_child(char *run_ptest, int fd_stdout, int fd_stderr)
 static inline int
 wait_child(pid_t pid, int timeout, int *fds, FILE **fps, int *timeouted)
 {
-	struct pollfd pfds[2];
 	struct timespec sentinel;
 	clockid_t clock = CLOCK_MONOTONIC;
 	int looping = 1;
-	int r;
 
 	int status = -1;
 	int waitflags;
 
-	pfds[0].fd = fds[0];
-	pfds[0].events = POLLIN;
-	pfds[1].fd = fds[1];
-	pfds[1].events = POLLIN;
-
 	if (clock_gettime(clock, &sentinel) == -1) {
 		clock = CLOCK_REALTIME;
 		clock_gettime(clock, &sentinel);
@@ -328,29 +323,10 @@  wait_child(pid_t pid, int timeout, int *fds, FILE **fps, int *timeouted)
 		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];
-			ssize_t n;
-
-			if (pfds[0].revents != 0) {
-				while ((n = read(fds[0], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0)
-					fwrite(buf, (size_t)n, 1, fps[0]);
-			}
-
-			if (pfds[1].revents != 0) {
-				while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0) {
-					fflush(fps[0]);
-					fwrite(buf, (size_t)n, 1, fps[1]);
-					fflush(fps[1]);
-				}
-			}
-
-			clock_gettime(clock, &sentinel);
-		}
-	}
+        }
 
 	fflush(fps[0]);
+
 	return status;
 }
 
@@ -412,8 +388,6 @@  run_ptests(struct ptest_list *head, const struct ptest_options opts,
 	char stime[GET_STIME_BUF_SIZE];
 
 	pid_t child;
-	int pipefd_stdout[2];
-	int pipefd_stderr[2];
 	int timeouted;
 	time_t sttime, entime;
 	time_t duration;
@@ -428,14 +402,6 @@  run_ptests(struct ptest_list *head, const struct ptest_options opts,
 
 	do
 	{
-		if ((rc = pipe2(pipefd_stdout, O_NONBLOCK)) == -1) 
-			break;
-
-		if ((rc = pipe2(pipefd_stderr, O_NONBLOCK)) == -1) {
-			close(pipefd_stdout[0]);
-			close(pipefd_stdout[1]);
-			break;
-		}
 		fprintf(fp, "START: %s\n", progname);
 		if (isatty(0) && ioctl(0, TIOCNOTTY) == -1) {
 			fprintf(fp, "ERROR: Unable to detach from controlling tty, %s\n", strerror(errno));
@@ -474,11 +440,10 @@  run_ptests(struct ptest_list *head, const struct ptest_options opts,
 					fprintf(fp, "ERROR: Unable to attach to controlling tty, %s\n", strerror(errno));
 				}
 
-				run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]);
+				run_child(p->run_ptest, STDOUT_FILENO, STDERR_FILENO);
 
 			} else {
 				int status;
-				int fds[2]; fds[0] = pipefd_stdout[0]; fds[1] = pipefd_stderr[0];
 				FILE *fps[2]; fps[0] = fp; fps[1] = fp_stderr;
 
 				if (setpgid(child, pgid) == -1) {
@@ -489,7 +454,7 @@  run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, sttime));
 				fprintf(fp, "BEGIN: %s\n", ptest_dir);
 
-				status = wait_child(child, opts.timeout, fds, fps, &timeouted);
+				status = wait_child(child, opts.timeout, NULL, fps, &timeouted);
 				entime = time(NULL);
 				duration = entime - sttime;
 
@@ -510,8 +475,6 @@  run_ptests(struct ptest_list *head, const struct ptest_options opts,
 		PTEST_LIST_ITERATE_END
 		fprintf(fp, "STOP: %s\n", progname);
 
-		close(pipefd_stdout[0]); close(pipefd_stdout[1]);
-		close(pipefd_stderr[0]); close(pipefd_stderr[1]);
 	} while (0);
 
 	if (rc == -1)