Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1595308 > unrolled thread

[GIT PULL][PATCH 0/2] ktest: Fixes for 4.11

Back to article view | Back to linux.kernel

Contents

  [GIT PULL][PATCH 0/2] ktest: Fixes for 4.11 Steven Rostedt <rostedt@goodmis.org> - 2017-03-08 17:00 +0100
    [GIT PULL][PATCH 1/2] ktest: Fix while loop in wait_for_input Steven Rostedt <rostedt@goodmis.org> - 2017-03-08 17:00 +0100
    [GIT PULL][PATCH 2/2] ktest: Make sure wait_for_input does honor the timeout Steven Rostedt <rostedt@goodmis.org> - 2017-03-08 17:00 +0100
    Re: [GIT PULL][PATCH 0/2] ktest: Fixes for 4.11 Steven Rostedt <rostedt@goodmis.org> - 2017-03-08 17:30 +0100

#1595308 — [GIT PULL][PATCH 0/2] ktest: Fixes for 4.11

Linus,

Greg Kroah-Hartman reported to me that the ktest of v4.10 locked up in an
infinite loop while doing the make mrproper. Looking into the cause I noticed
that a recent update to the function run_command (used for running all
shell commands, including "make mrproper") changed the internal loop to
use the function wait_for_input. The wait_for_input uses select to look
at two file descriptors. One is the file descriptor of the command it is
running, the other is STDIN. The STDIN check was not checking the return
status of the sysread call, and was also just writing a lot of data into
syswrite without regard to the size of the data read.

Changing the code to check the return status of sysread, and also to still
process the passed in descriptor data without looping back to the select
fixed Greg's problem.

While looking at this code I also realized that the loop did not honor
the timeout if STDIN always had input (or for some reason return error).
this could prevent wait_for_input to timeout on the file descriptor it
is suppose to be waiting for. That is fixed too.

Please pull the latest ktest-v4.11-rc1 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-ktest.git
ktest-v4.11-rc1

Tag SHA1: 6692cf546881262ada6b98dc7f0ee374922e669b
Head SHA1: f7c6401ff84ab8ffffc281a29aa0a787f7eb346e


Steven Rostedt (VMware) (2):
      ktest: Fix while loop in wait_for_input
      ktest: Make sure wait_for_input does honor the timeout

----
 tools/testing/ktest/ktest.pl | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

[toc] | [next] | [standalone]

#1595311 — [GIT PULL][PATCH 1/2] ktest: Fix while loop in wait_for_input

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

The run_command function was changed to use the wait_for_input function to
allow having a timeout if the command to run takes too much time. There was
a bug in the wait_for_input where it could end up going into an infinite
loop. There's two issues here. One is that the return value of the sysread
wasn't used for the write (to write a proper size), and that it should
continue processing the passed in file descriptor too even if there was
input. There was no check for error, if for some reason STDIN returned an
error, the function would go into an infinite loop and never exit.

Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Tested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Fixes: 6e98d1b4415f ("ktest: Add timeout to ssh command")
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/testing/ktest/ktest.pl | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index 29470b554711..0c006a2f165d 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -1904,11 +1904,12 @@ sub wait_for_input
 
 	# copy data from stdin to the console
 	if (vec($rout, fileno(\*STDIN), 1) == 1) {
-	    sysread(\*STDIN, $buf, 1000);
-	    syswrite($fp, $buf, 1000);
-	    next;
+	    $nr = sysread(\*STDIN, $buf, 1000);
+	    syswrite($fp, $buf, $nr) if ($nr > 0);
 	}
 
+	next if (vec($rout, fileno($fp), 1) != 1);
+
 	$line = "";
 
 	# try to read one char at a time
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1595316 — [GIT PULL][PATCH 2/2] ktest: Make sure wait_for_input does honor the timeout

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

The function wait_for_input takes in a timeout, and even has a default
timeout. But if for some reason the STDIN descriptor keeps sending in data,
the function will never time out. The timout is to wait for the data from
the passed in file descriptor, not for STDIN. Adding a test in the case
where there's no data from the passed in file descriptor that checks to see
if the timeout passed, will ensure that it will timeout properly even if
there's input in STDIN.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 tools/testing/ktest/ktest.pl | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tools/testing/ktest/ktest.pl b/tools/testing/ktest/ktest.pl
index 0c006a2f165d..49f7c8b9d9c4 100755
--- a/tools/testing/ktest/ktest.pl
+++ b/tools/testing/ktest/ktest.pl
@@ -1880,6 +1880,7 @@ sub get_grub_index {
 sub wait_for_input
 {
     my ($fp, $time) = @_;
+    my $start_time;
     my $rin;
     my $rout;
     my $nr;
@@ -1895,12 +1896,12 @@ sub wait_for_input
     vec($rin, fileno($fp), 1) = 1;
     vec($rin, fileno(\*STDIN), 1) = 1;
 
+    $start_time = time;
+
     while (1) {
 	$nr = select($rout=$rin, undef, undef, $time);
 
-	if ($nr <= 0) {
-	    return undef;
-	}
+	last if ($nr <= 0);
 
 	# copy data from stdin to the console
 	if (vec($rout, fileno(\*STDIN), 1) == 1) {
@@ -1908,7 +1909,11 @@ sub wait_for_input
 	    syswrite($fp, $buf, $nr) if ($nr > 0);
 	}
 
-	next if (vec($rout, fileno($fp), 1) != 1);
+	# The timeout is based on time waiting for the fp data
+	if (vec($rout, fileno($fp), 1) != 1) {
+	    last if (defined($time) && (time - $start_time > $time));
+	    next;
+	}
 
 	$line = "";
 
@@ -1918,12 +1923,11 @@ sub wait_for_input
 	    last if ($ch eq "\n");
 	}
 
-	if (!length($line)) {
-	    return undef;
-	}
+	last if (!length($line));
 
 	return $line;
     }
+    return undef;
 }
 
 sub reboot_to {
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1595332

On Wed, 08 Mar 2017 10:55:21 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> Linus,
> 
> Greg Kroah-Hartman reported to me that the ktest of v4.10 locked up in an

That should have been v4.11-rc1.

-- Steve

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web