[LTP] [PATCH] memcg_function:fix the wrong of test_proc_kill

[LTP] [PATCH] memcg_function:fix the wrong of test_proc_kill

[Peng Haitao]

test_proc_kill() tests process will be killed due to exceed memory limit, but
when memcg_process exit due to mmap/munmap fails, the case still pass.

Signed-off-by: Peng Haitao <penght@cn.fujitsu.com>
---
 .../memcg/functional/memcg_function_test.sh        |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
index 1f07cb5..6cfe9a5 100755
--- a/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
+++ b/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
@@ -175,13 +175,23 @@ test_failcnt()
 test_proc_kill()
 {
 	echo $1 > memory.limit_in_bytes
-	$TEST_PATH/memcg_process $2 -s $3 &
+	($TEST_PATH/memcg_process $2 -s $3 2>/tmp/errlog) &
 	pid=$!
 	sleep 1
 	echo $pid > tasks
 
 	/bin/kill -s SIGUSR1 $pid 2> /dev/null
 	sleep 1
+
+	if [ -s /tmp/errlog ]; then
+		err_msg=`cat /tmp/errlog`
+		rm -f /tmp/errlog
+		result $FAIL "$err_msg"
+		return
+	fi
+
+	rm -f /tmp/errlog
+
 	ps -p $pid > /dev/null 2> /dev/null
 	if [ $? -ne 0 ]; then
 		result $PASS "process $pid is killed"
-- 
1.7.1

-- 
Best Regards,
Peng Haitao


------------------------------------------------------------------------------
Learn Windows Azure Live!  Tuesday, Dec 13, 2011
Microsoft is holding a special Learn Windows Azure training event for 
developers. It will provide a great way to learn Windows Azure and what it 
provides. You can attend the event by watching it streamed LIVE online.  
Learn more at http://p.sf.net/sfu/ms-windowsazure
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] memcg_function:fix the wrong of test_proc_kill

[Cyril Hrubis]

Hi!
> test_proc_kill() tests process will be killed due to exceed memory limit, but
> when memcg_process exit due to mmap/munmap fails, the case still pass.
> 
> Signed-off-by: Peng Haitao <penght@cn.fujitsu.com>
> ---
>  .../memcg/functional/memcg_function_test.sh        |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh b/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
> index 1f07cb5..6cfe9a5 100755
> --- a/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
> +++ b/testcases/kernel/controllers/memcg/functional/memcg_function_test.sh
> @@ -175,13 +175,23 @@ test_failcnt()
>  test_proc_kill()
>  {
>  	echo $1 > memory.limit_in_bytes
> -	$TEST_PATH/memcg_process $2 -s $3 &
> +	($TEST_PATH/memcg_process $2 -s $3 2>/tmp/errlog) &
>  	pid=$!
>  	sleep 1
>  	echo $pid > tasks
>  
>  	/bin/kill -s SIGUSR1 $pid 2> /dev/null
>  	sleep 1
> +
> +	if [ -s /tmp/errlog ]; then
> +		err_msg=`cat /tmp/errlog`
> +		rm -f /tmp/errlog
> +		result $FAIL "$err_msg"
> +		return
> +	fi
> +
> +	rm -f /tmp/errlog
> +

Looking into the memcg_process.c it returns return value 1 on failure,
catching it correctly is better option here.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Write once. Port to many.
Get the SDK and tools to simplify cross-platform app development. Create 
new or port existing apps to sell to consumers worldwide. Explore the 
Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
http://p.sf.net/sfu/intel-appdev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] memcg_function:fix the wrong of test_proc_kill

[Cyril Hrubis]

> >> -	$TEST_PATH/memcg_process $2 -s $3 &
> >> +	($TEST_PATH/memcg_process $2 -s $3 2>/tmp/errlog) &
> >>  	pid=$!
> >>  	sleep 1
> >>  	echo $pid > tasks
> >>  
> >>  	/bin/kill -s SIGUSR1 $pid 2> /dev/null
> >>  	sleep 1
> >> +
> >> +	if [ -s /tmp/errlog ]; then
> >> +		err_msg=`cat /tmp/errlog`
> >> +		rm -f /tmp/errlog
> >> +		result $FAIL "$err_msg"
> >> +		return
> >> +	fi
> >> +
> >> +	rm -f /tmp/errlog
> >> +
> > 
> > Looking into the memcg_process.c it returns return value 1 on failure,
> > catching it correctly is better option here.
> > 
> 
> I also think catching memcg_process's return value is better option,
> but I think this way is simple.
> 
> Can you tell me a simple way to catch return value of background process?
> Thanks.

See manual pages for jobs and wait.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Write once. Port to many.
Get the SDK and tools to simplify cross-platform app development. Create 
new or port existing apps to sell to consumers worldwide. Explore the 
Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
http://p.sf.net/sfu/intel-appdev
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] memcg_function:fix the wrong of test_proc_kill

[Cyril Hrubis]

Hi!
> Cyril Hrubis said the following on 2011-12-28 19:54:
> >> Can you tell me a simple way to catch return value of background process?
> >> Thanks.
> > 
> > See manual pages for jobs and wait.
> > 
> 
> Thanks for your suggest.
> I know wait, but I think wait is not better than catching error message.
> 
> If you think wait is better, the following patch use wait.

Yes, I think this way is better than looking for error logs.

Also note that running the sleep 2; kill ... in background is not
necessary as shell is required to save return status from background
command if it's pid was saved by $!.

Generally sequence of

command&
pid=$!
kill $pid
wait $pid

Should work as expected.

>  	sleep 1
>  	echo $pid > tasks
>  
> -	/bin/kill -s SIGUSR1 $pid 2> /dev/null
> -	sleep 1
> +	{
> +		sleep 2
> +		/bin/kill -s SIGUSR1 $pid 2> /dev/null
> +	}&
> +
> +	wait $pid
> +	if [ $? -eq 1 ]; then
> +		result $FAIL "process $pid is killed by error"
> +		return
> +	fi
> +
>  	ps -p $pid > /dev/null 2> /dev/null
>  	if [ $? -ne 0 ]; then
>  		result $PASS "process $pid is killed"

Hmm I see the problem here, if the test wouldn't exit by the 
getting the SIGUSR1 the wait will run forever. So the fail based
solution may actually be better here, alghough I still think it's a
little fragile.

And I recall saying that earlier, the way the memcg_process is
controlled by a signal is fundamentally flaved. The correct way
for this would be creating a unix socket (or use whatever interface to
interprocess communication). Then you can comunicate with the
memcg_process running in background and gets it's status that way.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Ridiculously easy VDI. With Citrix VDI-in-a-Box, you don't need a complex
infrastructure or vast IT resources to deliver seamless, secure access to
virtual desktops. With this all-in-one solution, easily deploy virtual 
desktops for less than the cost of PCs and save 60% on VDI infrastructure 
costs. Try it free! http://p.sf.net/sfu/Citrix-VDIinabox
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] memcg_function:fix the wrong of test_proc_kill

[Cyril Hrubis]

Hi!
> > Also note that running the sleep 2; kill ... in background is not
> > necessary as shell is required to save return status from background
> > command if it's pid was saved by $!.
> > 
> > Generally sequence of
> > 
> > command&
> > pid=$!
> > kill $pid
> > wait $pid
> > 
> > Should work as expected.
> > 
> 
> Thanks for you comment.
> I think the following way is better.
> 
> test_proc_kill() tests process will be killed due to exceed memory limit, but
> when memcg_process exit due to mmap/munmap fails, the case still pass.

This one seems right to me as well, commited.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Ridiculously easy VDI. With Citrix VDI-in-a-Box, you don't need a complex
infrastructure or vast IT resources to deliver seamless, secure access to
virtual desktops. With this all-in-one solution, easily deploy virtual 
desktops for less than the cost of PCs and save 60% on VDI infrastructure 
costs. Try it free! http://p.sf.net/sfu/Citrix-VDIinabox
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list