public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH] setrlimit01: wait for all children to exit in test3
@ 2012-03-20 15:39 Salvatore CRO'
  2012-03-21  8:15 ` Carmelo AMOROSO
  2012-03-21  9:41 ` ZhoupingLiu
  0 siblings, 2 replies; 6+ messages in thread
From: Salvatore CRO' @ 2012-03-20 15:39 UTC (permalink / raw)
  To: ltp-list

In test3, parent just wait for one child to exit but it
actually has 10.
As it could happen there may be other children around that
didn't get yet the chance to exit, they will get signaled
by ltp-pan (SIGTERM) and pollute the output of subsequent
test (setrlimit02).
Parent definitely needs to wait for all children to exit.

Signed-off-by: Salvatore Cro <salvatore.cro@st.com>
---
 testcases/kernel/syscalls/setrlimit/setrlimit01.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
index ac32450..430835c 100644
--- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
+++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
@@ -267,7 +267,7 @@ void test3()
 			exit(0);
 		}
 	}
-	waitpid(pid, &status, 0);
+	while(wait(&status) > 0) { /* no-op */ ; }
 	if (WEXITSTATUS(status) != 0) {
 		tst_resm(TFAIL, "RLIMIT_NPROC functionality is not correct");
 	} else {
-- 
1.7.4.4


------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] setrlimit01: wait for all children to exit in test3
  2012-03-20 15:39 [LTP] [PATCH] setrlimit01: wait for all children to exit in test3 Salvatore CRO'
@ 2012-03-21  8:15 ` Carmelo AMOROSO
  2012-03-21  9:41 ` ZhoupingLiu
  1 sibling, 0 replies; 6+ messages in thread
From: Carmelo AMOROSO @ 2012-03-21  8:15 UTC (permalink / raw)
  To: ltp-list

On 20/03/2012 16.39, Salvatore CRO' wrote:
> In test3, parent just wait for one child to exit but it
> actually has 10.
> As it could happen there may be other children around that
> didn't get yet the chance to exit, they will get signaled
> by ltp-pan (SIGTERM) and pollute the output of subsequent
> test (setrlimit02).
> Parent definitely needs to wait for all children to exit.
> 
> Signed-off-by: Salvatore Cro <salvatore.cro@st.com>

Acked-by: Carmelo Amoroso <carmelo.amoroso@st.com>

> ---
>  testcases/kernel/syscalls/setrlimit/setrlimit01.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
> index ac32450..430835c 100644
> --- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
> +++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
> @@ -267,7 +267,7 @@ void test3()
>  			exit(0);
>  		}
>  	}
> -	waitpid(pid, &status, 0);
> +	while(wait(&status) > 0) { /* no-op */ ; }
>  	if (WEXITSTATUS(status) != 0) {
>  		tst_resm(TFAIL, "RLIMIT_NPROC functionality is not correct");
>  	} else {


------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] setrlimit01: wait for all children to exit in test3
  2012-03-20 15:39 [LTP] [PATCH] setrlimit01: wait for all children to exit in test3 Salvatore CRO'
  2012-03-21  8:15 ` Carmelo AMOROSO
@ 2012-03-21  9:41 ` ZhoupingLiu
  2012-03-21  9:46   ` Caspar Zhang
  1 sibling, 1 reply; 6+ messages in thread
From: ZhoupingLiu @ 2012-03-21  9:41 UTC (permalink / raw)
  To: Salvatore CRO'; +Cc: ltp-list

On 03/20/2012 11:39 PM, Salvatore CRO' wrote:
> In test3, parent just wait for one child to exit but it
> actually has 10.
hi, Salvatore

yes, you are right, but your patch didn't resolve the issue completely.
comments in-line.

> As it could happen there may be other children around that
> didn't get yet the chance to exit, they will get signaled
> by ltp-pan (SIGTERM) and pollute the output of subsequent
> test (setrlimit02).
> Parent definitely needs to wait for all children to exit.
>
> Signed-off-by: Salvatore Cro<salvatore.cro@st.com>
> ---
>   testcases/kernel/syscalls/setrlimit/setrlimit01.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
> index ac32450..430835c 100644
> --- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
> +++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
> @@ -267,7 +267,7 @@ void test3()
>   			exit(0);
>   		}
>   	}
> -	waitpid(pid,&status, 0);
> +	while(wait(&status)>  0) { /* no-op */ ; }
>   	if (WEXITSTATUS(status) != 0) {
>   		tst_resm(TFAIL, "RLIMIT_NPROC functionality is not correct");
>   	} else {
if one child exit unexpectedly, e.g: exit(9);
the case can't catch it. how about this patch set:

diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c 
b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
index ac32450..ec3e54e 100644
--- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
+++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
@@ -226,6 +226,8 @@ void test2()
   */
  void test3()
  {
+       int flag = 0;
+
         if (getrlimit(RLIMIT_NPROC, &save_rlim) < 0) {
                 tst_brkm(TBROK, cleanup, "getrlimit failed, errno: %d", 
errno);
         }
@@ -267,12 +269,16 @@ void test3()
                         exit(0);
                 }
         }
-       waitpid(pid, &status, 0);
-       if (WEXITSTATUS(status) != 0) {
-               tst_resm(TFAIL, "RLIMIT_NPROC functionality is not 
correct");
-       } else {
-               tst_resm(TPASS, "RLIMIT_NPROC functionality is correct");
+       while (wait(&status) > 0) {
+               if (WEXITSTATUS(status) != 0) {
+                       tst_resm(TFAIL,
+                           "RLIMIT_NPROC functionality is not correct");
+                       flag = 1;
+               }
         }
+
+       if (flag == 0)
+               tst_resm(TPASS, "RLIMIT_NPROC functionality is correct");
  }

  /*


------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] setrlimit01: wait for all children to exit in test3
  2012-03-21  9:41 ` ZhoupingLiu
@ 2012-03-21  9:46   ` Caspar Zhang
  2012-03-21 11:10     ` Salvatore CRO'
  0 siblings, 1 reply; 6+ messages in thread
From: Caspar Zhang @ 2012-03-21  9:46 UTC (permalink / raw)
  To: ZhoupingLiu; +Cc: ltp-list

On 03/21/2012 05:41 PM, ZhoupingLiu wrote:
> On 03/20/2012 11:39 PM, Salvatore CRO' wrote:
>> In test3, parent just wait for one child to exit but it
>> actually has 10.
> hi, Salvatore
> 
> yes, you are right, but your patch didn't resolve the issue completely.
> comments in-line.
> 
>> As it could happen there may be other children around that
>> didn't get yet the chance to exit, they will get signaled
>> by ltp-pan (SIGTERM) and pollute the output of subsequent
>> test (setrlimit02).
>> Parent definitely needs to wait for all children to exit.
>>
>> Signed-off-by: Salvatore Cro<salvatore.cro@st.com>
>> ---
>>   testcases/kernel/syscalls/setrlimit/setrlimit01.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>> index ac32450..430835c 100644
>> --- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>> +++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>> @@ -267,7 +267,7 @@ void test3()
>>   			exit(0);
>>   		}
>>   	}
>> -	waitpid(pid,&status, 0);
>> +	while(wait(&status)>  0) { /* no-op */ ; }
>>   	if (WEXITSTATUS(status) != 0) {
>>   		tst_resm(TFAIL, "RLIMIT_NPROC functionality is not correct");
>>   	} else {
> if one child exit unexpectedly, e.g: exit(9);
> the case can't catch it. how about this patch set:
> 
> diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c 
> b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
> index ac32450..ec3e54e 100644
> --- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
> +++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
> @@ -226,6 +226,8 @@ void test2()
>    */
>   void test3()
>   {
> +       int flag = 0;
> +
>          if (getrlimit(RLIMIT_NPROC, &save_rlim) < 0) {
>                  tst_brkm(TBROK, cleanup, "getrlimit failed, errno: %d", 
> errno);
>          }
> @@ -267,12 +269,16 @@ void test3()
>                          exit(0);
>                  }
>          }
> -       waitpid(pid, &status, 0);
> -       if (WEXITSTATUS(status) != 0) {
> -               tst_resm(TFAIL, "RLIMIT_NPROC functionality is not 
> correct");
> -       } else {
> -               tst_resm(TPASS, "RLIMIT_NPROC functionality is correct");
> +       while (wait(&status) > 0) {
> +               if (WEXITSTATUS(status) != 0) {
> +                       tst_resm(TFAIL,
> +                           "RLIMIT_NPROC functionality is not correct");
> +                       flag = 1;
> +               }
>          }
> +
> +       if (flag == 0)
> +               tst_resm(TPASS, "RLIMIT_NPROC functionality is correct");

flag is not necessary. no TFAIL means TPASS. Rest looks good for your patch.

Thanks,
Caspar

------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] setrlimit01: wait for all children to exit in test3
  2012-03-21  9:46   ` Caspar Zhang
@ 2012-03-21 11:10     ` Salvatore CRO'
  2012-03-23  7:13       ` Salvatore CRO'
  0 siblings, 1 reply; 6+ messages in thread
From: Salvatore CRO' @ 2012-03-21 11:10 UTC (permalink / raw)
  To: Caspar Zhang; +Cc: ltp-list

On 3/21/2012 10:46 AM, Caspar Zhang wrote:
> On 03/21/2012 05:41 PM, ZhoupingLiu wrote:
>> On 03/20/2012 11:39 PM, Salvatore CRO' wrote:
>>> In test3, parent just wait for one child to exit but it
>>> actually has 10.
>> hi, Salvatore
>>
>> yes, you are right, but your patch didn't resolve the issue completely.
>> comments in-line.
>>
>>> As it could happen there may be other children around that
>>> didn't get yet the chance to exit, they will get signaled
>>> by ltp-pan (SIGTERM) and pollute the output of subsequent
>>> test (setrlimit02).
>>> Parent definitely needs to wait for all children to exit.
>>>
>>> Signed-off-by: Salvatore Cro<salvatore.cro@st.com>
>>> ---
>>>    testcases/kernel/syscalls/setrlimit/setrlimit01.c |    2 +-
>>>    1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>> index ac32450..430835c 100644
>>> --- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>> +++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>> @@ -267,7 +267,7 @@ void test3()
>>>    			exit(0);
>>>    		}
>>>    	}
>>> -	waitpid(pid,&status, 0);
>>> +	while(wait(&status)>   0) { /* no-op */ ; }
>>>    	if (WEXITSTATUS(status) != 0) {
>>>    		tst_resm(TFAIL, "RLIMIT_NPROC functionality is not correct");
>>>    	} else {
>> if one child exit unexpectedly, e.g: exit(9);
>> the case can't catch it. how about this patch set:

Question is, do we care about prematurely ended child?
We could just test for this condition ( if may use WIFEXITED in this case)
and emit a TWARN.

>> diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>> b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>> index ac32450..ec3e54e 100644
>> --- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>> +++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>> @@ -226,6 +226,8 @@ void test2()
>>     */
>>    void test3()
>>    {
>> +       int flag = 0;
>> +
>>           if (getrlimit(RLIMIT_NPROC,&save_rlim)<  0) {
>>                   tst_brkm(TBROK, cleanup, "getrlimit failed, errno: %d",
>> errno);
>>           }
>> @@ -267,12 +269,16 @@ void test3()
>>                           exit(0);
>>                   }
>>           }
>> -       waitpid(pid,&status, 0);
>> -       if (WEXITSTATUS(status) != 0) {
>> -               tst_resm(TFAIL, "RLIMIT_NPROC functionality is not
>> correct");
>> -       } else {
>> -               tst_resm(TPASS, "RLIMIT_NPROC functionality is correct");
>> +       while (wait(&status)>  0) {
>> +               if (WEXITSTATUS(status) != 0) {
>> +                       tst_resm(TFAIL,
>> +                           "RLIMIT_NPROC functionality is not correct");
>> +                       flag = 1;
>> +               }
>>           }
>> +
>> +       if (flag == 0)
>> +               tst_resm(TPASS, "RLIMIT_NPROC functionality is correct");
> flag is not necessary. no TFAIL means TPASS. Rest looks good for your patch.

Anyhow, in my opinion, this condition is not suitable to decide for test 
pass/fail.
Test is supposed to fail when the fork from 11th child on does not fail 
or fail with
an errno != EAGAIN.
If so, the test logic wasn't actually accurate independently of my patch.

BR,
Salvo.

> Thanks,
> Caspar


------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] setrlimit01: wait for all children to exit in test3
  2012-03-21 11:10     ` Salvatore CRO'
@ 2012-03-23  7:13       ` Salvatore CRO'
  0 siblings, 0 replies; 6+ messages in thread
From: Salvatore CRO' @ 2012-03-23  7:13 UTC (permalink / raw)
  To: Caspar Zhang; +Cc: ltp-list

On 3/21/2012 12:10 PM, Salvatore CRO' wrote:
> On 3/21/2012 10:46 AM, Caspar Zhang wrote:
>> On 03/21/2012 05:41 PM, ZhoupingLiu wrote:
>>> On 03/20/2012 11:39 PM, Salvatore CRO' wrote:
>>>> In test3, parent just wait for one child to exit but it
>>>> actually has 10.
>>> hi, Salvatore
>>>
>>> yes, you are right, but your patch didn't resolve the issue completely.
>>> comments in-line.
>>>
>>>> As it could happen there may be other children around that
>>>> didn't get yet the chance to exit, they will get signaled
>>>> by ltp-pan (SIGTERM) and pollute the output of subsequent
>>>> test (setrlimit02).
>>>> Parent definitely needs to wait for all children to exit.
>>>>
>>>> Signed-off-by: Salvatore Cro<salvatore.cro@st.com>
>>>> ---
>>>>    testcases/kernel/syscalls/setrlimit/setrlimit01.c |    2 +-
>>>>    1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c 
>>>> b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>>> index ac32450..430835c 100644
>>>> --- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>>> +++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>>> @@ -267,7 +267,7 @@ void test3()
>>>>                exit(0);
>>>>            }
>>>>        }
>>>> -    waitpid(pid,&status, 0);
>>>> +    while(wait(&status)>   0) { /* no-op */ ; }
>>>>        if (WEXITSTATUS(status) != 0) {
>>>>            tst_resm(TFAIL, "RLIMIT_NPROC functionality is not 
>>>> correct");
>>>>        } else {
>>> if one child exit unexpectedly, e.g: exit(9);
>>> the case can't catch it. how about this patch set:
>
> Question is, do we care about prematurely ended child?
> We could just test for this condition ( if may use WIFEXITED in this 
> case)
> and emit a TWARN.
>
>>> diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>> b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>> index ac32450..ec3e54e 100644
>>> --- a/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>> +++ b/testcases/kernel/syscalls/setrlimit/setrlimit01.c
>>> @@ -226,6 +226,8 @@ void test2()
>>>     */
>>>    void test3()
>>>    {
>>> +       int flag = 0;
>>> +
>>>           if (getrlimit(RLIMIT_NPROC,&save_rlim)<  0) {
>>>                   tst_brkm(TBROK, cleanup, "getrlimit failed, errno: 
>>> %d",
>>> errno);
>>>           }
>>> @@ -267,12 +269,16 @@ void test3()
>>>                           exit(0);
>>>                   }
>>>           }
>>> -       waitpid(pid,&status, 0);
>>> -       if (WEXITSTATUS(status) != 0) {
>>> -               tst_resm(TFAIL, "RLIMIT_NPROC functionality is not
>>> correct");
>>> -       } else {
>>> -               tst_resm(TPASS, "RLIMIT_NPROC functionality is 
>>> correct");
>>> +       while (wait(&status)>  0) {
>>> +               if (WEXITSTATUS(status) != 0) {
>>> +                       tst_resm(TFAIL,
>>> +                           "RLIMIT_NPROC functionality is not 
>>> correct");
>>> +                       flag = 1;
>>> +               }
>>>           }
>>> +
>>> +       if (flag == 0)
>>> +               tst_resm(TPASS, "RLIMIT_NPROC functionality is 
>>> correct");
>> flag is not necessary. no TFAIL means TPASS. Rest looks good for your 
>> patch.
>
> Anyhow, in my opinion, this condition is not suitable to decide for 
> test pass/fail.
> Test is supposed to fail when the fork from 11th child on does not 
> fail or fail with
> an errno != EAGAIN.
> If so, the test logic wasn't actually accurate independently of my patch.
>
> BR,
> Salvo.
>

What about above analysis ? Any comment is welcome.

BR,
Salvo.


>> Thanks,
>> Caspar
>


------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

end of thread, other threads:[~2012-03-23  7:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-20 15:39 [LTP] [PATCH] setrlimit01: wait for all children to exit in test3 Salvatore CRO'
2012-03-21  8:15 ` Carmelo AMOROSO
2012-03-21  9:41 ` ZhoupingLiu
2012-03-21  9:46   ` Caspar Zhang
2012-03-21 11:10     ` Salvatore CRO'
2012-03-23  7:13       ` Salvatore CRO'

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox