zdtm: Fix fd01 cleanup

Submitted by Kirill Tkhai on Jan. 30, 2018, 3:19 p.m.

Details

Message ID 151732555603.18526.1166209794241505931.stgit@localhost.localdomain
State New
Series "zdtm: Fix fd01 cleanup"
Headers show

Commit Message

Kirill Tkhai Jan. 30, 2018, 3:19 p.m.
waitpid() does not return child pid, when child has not exited.
So, we can't use it to find pids of children.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 test/zdtm/static/fd01.c |   17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/test/zdtm/static/fd01.c b/test/zdtm/static/fd01.c
index 7f0256a03..390d9d126 100644
--- a/test/zdtm/static/fd01.c
+++ b/test/zdtm/static/fd01.c
@@ -22,11 +22,19 @@  int main(int argc, char **argv)
 	unsigned int i, max_nr, flags;
 	int fd, status, ret;
 	struct rlimit rlim;
+	futex_t *futex;
 	char buf[16];
 	pid_t pid;
 
 	test_init(argc, argv);
 
+	futex = mmap(NULL, sizeof(*futex), PROT_WRITE | PROT_READ, MAP_ANONYMOUS|MAP_SHARED, -1, 0);
+	if (futex == MAP_FAILED) {
+		fail("mmap");
+		exit(1);
+	}
+	futex_init(futex);
+
 	fd = open("/proc/sys/fs/nr_open", O_RDONLY);
 	if (fd < 0) {
 		fail("Can't open /proc/sys/fs/nr_open");
@@ -86,7 +94,7 @@  int main(int argc, char **argv)
 			fail("fork");
 			exit(1);
 		} else if (!pid) {
-			pause();
+			futex_wait_while(futex, 0);
 			exit(0);
 		}
 	}
@@ -95,10 +103,9 @@  int main(int argc, char **argv)
 	test_waitsig();
 
 	/* Cleanup */
-	while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
-		if (kill(pid, SIGTERM) == 0)
-			waitpid(-1, &status, 0); /* Ignore errors */
-	}
+	futex_set_and_wake(futex, 1);
+	while (wait(&status) > 0) /* Ignore errors */
+		;
 
 	pass();
 

Comments

Andrey Vagin Jan. 31, 2018, 6:20 p.m.
On Tue, Jan 30, 2018 at 06:19:31PM +0300, Kirill Tkhai wrote:
> waitpid() does not return child pid, when child has not exited.
> So, we can't use it to find pids of children.
> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  test/zdtm/static/fd01.c |   17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/test/zdtm/static/fd01.c b/test/zdtm/static/fd01.c
> index 7f0256a03..390d9d126 100644
> --- a/test/zdtm/static/fd01.c
> +++ b/test/zdtm/static/fd01.c
> @@ -22,11 +22,19 @@ int main(int argc, char **argv)
>  	unsigned int i, max_nr, flags;
>  	int fd, status, ret;
>  	struct rlimit rlim;
> +	futex_t *futex;
>  	char buf[16];
>  	pid_t pid;
>  
>  	test_init(argc, argv);
>  
> +	futex = mmap(NULL, sizeof(*futex), PROT_WRITE | PROT_READ, MAP_ANONYMOUS|MAP_SHARED, -1, 0);
> +	if (futex == MAP_FAILED) {
> +		fail("mmap");
> +		exit(1);
> +	}
> +	futex_init(futex);
> +
>  	fd = open("/proc/sys/fs/nr_open", O_RDONLY);
>  	if (fd < 0) {
>  		fail("Can't open /proc/sys/fs/nr_open");
> @@ -86,7 +94,7 @@ int main(int argc, char **argv)
>  			fail("fork");
>  			exit(1);
>  		} else if (!pid) {
> -			pause();
> +			futex_wait_while(futex, 0);
>  			exit(0);
>  		}
>  	}
> @@ -95,10 +103,9 @@ int main(int argc, char **argv)
>  	test_waitsig();
>  
>  	/* Cleanup */
> -	while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
> -		if (kill(pid, SIGTERM) == 0)
> -			waitpid(-1, &status, 0); /* Ignore errors */
> -	}
> +	futex_set_and_wake(futex, 1);
> +	while (wait(&status) > 0) /* Ignore errors */
> +		;

Pls, check an exit code from children, otherwise it isn't a test,
because it nevel fails. In addition, it would be good if you add
basic checks to be sure that file descriptors are restored correctly.

>  
>  	pass();
>  
>
Kirill Tkhai Feb. 1, 2018, 2:48 p.m.
On 31.01.2018 21:20, Andrei Vagin wrote:
> On Tue, Jan 30, 2018 at 06:19:31PM +0300, Kirill Tkhai wrote:
>> waitpid() does not return child pid, when child has not exited.
>> So, we can't use it to find pids of children.
>>
>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>> ---
>>  test/zdtm/static/fd01.c |   17 ++++++++++++-----
>>  1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/test/zdtm/static/fd01.c b/test/zdtm/static/fd01.c
>> index 7f0256a03..390d9d126 100644
>> --- a/test/zdtm/static/fd01.c
>> +++ b/test/zdtm/static/fd01.c
>> @@ -22,11 +22,19 @@ int main(int argc, char **argv)
>>  	unsigned int i, max_nr, flags;
>>  	int fd, status, ret;
>>  	struct rlimit rlim;
>> +	futex_t *futex;
>>  	char buf[16];
>>  	pid_t pid;
>>  
>>  	test_init(argc, argv);
>>  
>> +	futex = mmap(NULL, sizeof(*futex), PROT_WRITE | PROT_READ, MAP_ANONYMOUS|MAP_SHARED, -1, 0);
>> +	if (futex == MAP_FAILED) {
>> +		fail("mmap");
>> +		exit(1);
>> +	}
>> +	futex_init(futex);
>> +
>>  	fd = open("/proc/sys/fs/nr_open", O_RDONLY);
>>  	if (fd < 0) {
>>  		fail("Can't open /proc/sys/fs/nr_open");
>> @@ -86,7 +94,7 @@ int main(int argc, char **argv)
>>  			fail("fork");
>>  			exit(1);
>>  		} else if (!pid) {
>> -			pause();
>> +			futex_wait_while(futex, 0);
>>  			exit(0);
>>  		}
>>  	}
>> @@ -95,10 +103,9 @@ int main(int argc, char **argv)
>>  	test_waitsig();
>>  
>>  	/* Cleanup */
>> -	while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
>> -		if (kill(pid, SIGTERM) == 0)
>> -			waitpid(-1, &status, 0); /* Ignore errors */
>> -	}
>> +	futex_set_and_wake(futex, 1);
>> +	while (wait(&status) > 0) /* Ignore errors */
>> +		;
> 
> Pls, check an exit code from children, otherwise it isn't a test,
> because it nevel fails.

Ok, no problem. But the idea was to use zdtm.py debug hooks, which checks
for file descriptors in check_visible_state()...:

    print "%s: Old files lost: %s" % (pid, fold - fnew)
    print "%s: New files appeared: %s" % (pid, fnew - fold)

>In addition, it would be good if you add
>basic checks to be sure that file descriptors are restored correctly.

Sadly, but it won't be possible to check that in sane way, because
the behaviour of created children and files is almost "random".
I don't see a good way to check everything in the test having so many
entities. Maybe we better implement one more? And remain this test
for checks I wrote above.

Kirill
Andrey Vagin Feb. 1, 2018, 5:55 p.m.
On Thu, Feb 01, 2018 at 05:48:22PM +0300, Kirill Tkhai wrote:
> On 31.01.2018 21:20, Andrei Vagin wrote:
> > On Tue, Jan 30, 2018 at 06:19:31PM +0300, Kirill Tkhai wrote:
> >> waitpid() does not return child pid, when child has not exited.
> >> So, we can't use it to find pids of children.
> >>
> >> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> >> ---
> >>  test/zdtm/static/fd01.c |   17 ++++++++++++-----
> >>  1 file changed, 12 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/test/zdtm/static/fd01.c b/test/zdtm/static/fd01.c
> >> index 7f0256a03..390d9d126 100644
> >> --- a/test/zdtm/static/fd01.c
> >> +++ b/test/zdtm/static/fd01.c
> >> @@ -22,11 +22,19 @@ int main(int argc, char **argv)
> >>  	unsigned int i, max_nr, flags;
> >>  	int fd, status, ret;
> >>  	struct rlimit rlim;
> >> +	futex_t *futex;
> >>  	char buf[16];
> >>  	pid_t pid;
> >>  
> >>  	test_init(argc, argv);
> >>  
> >> +	futex = mmap(NULL, sizeof(*futex), PROT_WRITE | PROT_READ, MAP_ANONYMOUS|MAP_SHARED, -1, 0);
> >> +	if (futex == MAP_FAILED) {
> >> +		fail("mmap");
> >> +		exit(1);
> >> +	}
> >> +	futex_init(futex);
> >> +
> >>  	fd = open("/proc/sys/fs/nr_open", O_RDONLY);
> >>  	if (fd < 0) {
> >>  		fail("Can't open /proc/sys/fs/nr_open");
> >> @@ -86,7 +94,7 @@ int main(int argc, char **argv)
> >>  			fail("fork");
> >>  			exit(1);
> >>  		} else if (!pid) {
> >> -			pause();
> >> +			futex_wait_while(futex, 0);
> >>  			exit(0);
> >>  		}
> >>  	}
> >> @@ -95,10 +103,9 @@ int main(int argc, char **argv)
> >>  	test_waitsig();
> >>  
> >>  	/* Cleanup */
> >> -	while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
> >> -		if (kill(pid, SIGTERM) == 0)
> >> -			waitpid(-1, &status, 0); /* Ignore errors */
> >> -	}
> >> +	futex_set_and_wake(futex, 1);
> >> +	while (wait(&status) > 0) /* Ignore errors */
> >> +		;
> > 
> > Pls, check an exit code from children, otherwise it isn't a test,
> > because it nevel fails.
> 
> Ok, no problem. But the idea was to use zdtm.py debug hooks, which checks
> for file descriptors in check_visible_state()...:
> 
>     print "%s: Old files lost: %s" % (pid, fold - fnew)
>     print "%s: New files appeared: %s" % (pid, fnew - fold)
> 
> >In addition, it would be good if you add
> >basic checks to be sure that file descriptors are restored correctly.
> 
> Sadly, but it won't be possible to check that in sane way, because
> the behaviour of created children and files is almost "random".
> I don't see a good way to check everything in the test having so many
> entities. Maybe we better implement one more? And remain this test
> for checks I wrote above.

You are right, the zdtm hook does some basic checks, so I think it is
enough. Thank you.


> 
> Kirill