[LTP] [PATCH] pthread_cond_signal/1-1: use sched_yield to schedule child threads

[LTP] [PATCH] pthread_cond_signal/1-1: use sched_yield to schedule child threads

[Kang Kai]

This case fails on mips board routerstation randomly. The root cause is
that when main thread call usleep(100) after signal the child threads,
no child thread is scheduled to run.  So the case fails.

I update it to set main thread with a lower priority and child threads
with a higher one, then when sched_yield is called in main thread, one
of the child threads will be scheduled and run.

Signed-off-by: Kang Kai <kai.kang@windriver.com>
---
 .../interfaces/pthread_cond_signal/1-1.c           |   55 ++++++++++++++++++--
 1 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c
index bf55b31..51c0cfb 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c
@@ -11,6 +11,7 @@
  */
 
 #define _XOPEN_SOURCE 600
+#define _GNU_SOURCE
 
 #include <pthread.h>
 #include <stdio.h>
@@ -18,6 +19,7 @@
 #include <unistd.h>
 #include <signal.h>
 #include "posixtest.h"
+#include "affinity.h"
 
 #define THREAD_NUM  3
 
@@ -77,6 +79,18 @@ int main()
 {
 	int i, rc;
 	struct sigaction act;
+	int pid;
+	int policy;
+	struct sched_param param;
+	pthread_attr_t attr;
+
+	if (getuid() != 0)
+	{
+		fprintf(stderr, "Run this test as root, not as a Regular User\n");
+		return PTS_UNTESTED;
+	}
+
+	set_affinity(0);
 
 	if (pthread_mutex_init(&td.mutex, NULL) != 0) {
 		fprintf(stderr,"Fail to initialize mutex\n");
@@ -87,14 +101,41 @@ int main()
 		return PTS_UNRESOLVED;
 	}
 
+	pid = getpid();
+	param.sched_priority = sched_get_priority_min(SCHED_FIFO);
+	if (sched_setscheduler(pid, SCHED_FIFO, &param) != 0) {
+		fprintf(stderr, "Fail to set main thread scheduler.\n");
+		return PTS_UNRESOLVED;
+	}
+
+	if (pthread_attr_init(&attr) != 0) {
+		fprintf(stderr, "Fail to init child thread attribute.\n");
+		return PTS_UNRESOLVED;
+	}
+	if (pthread_attr_setschedpolicy(&attr, SCHED_FIFO) != 0) {
+		fprintf(stderr, "Fail to set child thread schedule policy attribute.\n");
+		return PTS_UNRESOLVED;
+	}
+	param.sched_priority += 10;
+	if (pthread_attr_setschedparam(&attr, &param) != 0) {
+		fprintf(stderr, "Fail to set child thread schedule priority attribute.\n");
+		return PTS_UNRESOLVED;
+	}
+	if (pthread_attr_setinheritsched(&attr, PTHREAD_EXPLICIT_SCHED) != 0) {
+		fprintf(stderr, "Fail to set child thread inheritace attribute.\n");
+		return PTS_UNRESOLVED;
+	}
+
 	for (i=0; i<THREAD_NUM; i++) {	/* create THREAD_NUM threads */
-	    	if (pthread_create(&thread[i], NULL, thr_func, NULL) != 0) {
+		if (pthread_create(&thread[i], &attr, thr_func, NULL) != 0) {
 			fprintf(stderr,"Fail to create thread[%d]\n", i);
 			exit(PTS_UNRESOLVED);
 		}
 	}
 	while (start_num < THREAD_NUM)	/* waiting for all threads started */
-		usleep(100);
+		sched_yield();
+
+	pthread_attr_destroy(&attr);
 
 	/* Acquire the mutex to make sure that all waiters are currently
 	   blocked on pthread_cond_wait */
@@ -114,7 +155,7 @@ int main()
 		fprintf(stderr,"[Main thread] failed to signal the condition\n");
 		exit(PTS_UNRESOLVED);
 	}
-	sleep(1);
+	sched_yield();
 	if (waken_num <= 0) {
 		fprintf(stderr,"[Main thread] but no waiters were wakened\n");
                 printf("Test FAILED\n");
@@ -142,7 +183,7 @@ int main()
 			fprintf(stderr,"Main failed to signal the condition\n");
 			exit(PTS_UNRESOLVED);
 		}
-		usleep(100);
+		sched_yield();
 	}
 
 	if (i >= THREAD_NUM) {
@@ -158,6 +199,10 @@ int main()
 			exit(PTS_UNRESOLVED);
 		}
 	}
+
+	pthread_mutex_destroy(&td.mutex);
+	pthread_cond_destroy(&td.cond);;
+
 	printf("Test PASSED\n");
 	return PTS_PASS;
-}
\ No newline at end of file
+}
-- 
1.7.5.4


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] pthread_cond_signal/1-1: use sched_yield to schedule child threads

[chrubis]

Hi!
> This case fails on mips board routerstation randomly. The root cause is
> that when main thread call usleep(100) after signal the child threads,
> no child thread is scheduled to run.  So the case fails.
> 
> I update it to set main thread with a lower priority and child threads
> with a higher one, then when sched_yield is called in main thread, one
> of the child threads will be scheduled and run.
> 
> Signed-off-by: Kang Kai <kai.kang@windriver.com>
> ---
>  .../interfaces/pthread_cond_signal/1-1.c           |   55 ++++++++++++++++++--
>  1 files changed, 50 insertions(+), 5 deletions(-)
> 
> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c
> index bf55b31..51c0cfb 100644
> --- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c
> @@ -11,6 +11,7 @@
>   */
>  
>  #define _XOPEN_SOURCE 600
> +#define _GNU_SOURCE

This is open posix testsuite keep that in mind so inserting _GNU_SOURCE
is not allowed here.

>  #include <pthread.h>
>  #include <stdio.h>
> @@ -18,6 +19,7 @@
>  #include <unistd.h>
>  #include <signal.h>
>  #include "posixtest.h"
> +#include "affinity.h"
>  
>  #define THREAD_NUM  3
>  
> @@ -77,6 +79,18 @@ int main()
>  {
>  	int i, rc;
>  	struct sigaction act;
> +	int pid;
> +	int policy;
> +	struct sched_param param;
> +	pthread_attr_t attr;
> +
> +	if (getuid() != 0)
> +	{
> +		fprintf(stderr, "Run this test as root, not as a Regular User\n");
> +		return PTS_UNTESTED;
> +	}

Wrong coding style. The opening brace should be at the end of the if
statement. Please use checkpatch.pl on your patches.

And also I think that the check is not 100% correct, the POSIX says that
schedulling priorities and so may be changed "if calling process has
permission to...", there are capabilities and other mechanisms to check
for that. The rest of the testcases seems to deal with this just by
printing a hint after first call has that may need the priviledge
failed, for example:

"foo failed with -1: are you root?"

> +	set_affinity(0);

I setting affinity really needed here? (I'm just asking before it wasn't
described in the commit description).

>  	if (pthread_mutex_init(&td.mutex, NULL) != 0) {
>  		fprintf(stderr,"Fail to initialize mutex\n");
> @@ -87,14 +101,41 @@ int main()
>  		return PTS_UNRESOLVED;
>  	}
>  
> +	pid = getpid();
> +	param.sched_priority = sched_get_priority_min(SCHED_FIFO);
> +	if (sched_setscheduler(pid, SCHED_FIFO, &param) != 0) {
> +		fprintf(stderr, "Fail to set main thread scheduler.\n");
> +		return PTS_UNRESOLVED;
> +	}
> +
> +	if (pthread_attr_init(&attr) != 0) {
> +		fprintf(stderr, "Fail to init child thread attribute.\n");
> +		return PTS_UNRESOLVED;
> +	}
> +	if (pthread_attr_setschedpolicy(&attr, SCHED_FIFO) != 0) {
> +		fprintf(stderr, "Fail to set child thread schedule policy attribute.\n");
> +		return PTS_UNRESOLVED;
> +	}
> +	param.sched_priority += 10;
> +	if (pthread_attr_setschedparam(&attr, &param) != 0) {
> +		fprintf(stderr, "Fail to set child thread schedule priority attribute.\n");
> +		return PTS_UNRESOLVED;
> +	}
> +	if (pthread_attr_setinheritsched(&attr, PTHREAD_EXPLICIT_SCHED) != 0) {
> +		fprintf(stderr, "Fail to set child thread inheritace attribute.\n");
> +		return PTS_UNRESOLVED;
> +	}
> +
>  	for (i=0; i<THREAD_NUM; i++) {	/* create THREAD_NUM threads */
> -	    	if (pthread_create(&thread[i], NULL, thr_func, NULL) != 0) {
> +		if (pthread_create(&thread[i], &attr, thr_func, NULL) != 0) {
>  			fprintf(stderr,"Fail to create thread[%d]\n", i);
>  			exit(PTS_UNRESOLVED);
>  		}
>  	}
>  	while (start_num < THREAD_NUM)	/* waiting for all threads started */
> -		usleep(100);
> +		sched_yield();

Hmm, I've been burned by:

while (xxx)
	sched_yield();

code before. In certain situations this translates just to bussy loops,
so most of the time small sleep or both sleep and sched_yield may be
better.

> +	pthread_attr_destroy(&attr);
>  
>  	/* Acquire the mutex to make sure that all waiters are currently
>  	   blocked on pthread_cond_wait */
> @@ -114,7 +155,7 @@ int main()
>  		fprintf(stderr,"[Main thread] failed to signal the condition\n");
>  		exit(PTS_UNRESOLVED);
>  	}
> -	sleep(1);
> +	sched_yield();
>  	if (waken_num <= 0) {
>  		fprintf(stderr,"[Main thread] but no waiters were wakened\n");
>                  printf("Test FAILED\n");
> @@ -142,7 +183,7 @@ int main()
>  			fprintf(stderr,"Main failed to signal the condition\n");
>  			exit(PTS_UNRESOLVED);
>  		}
> -		usleep(100);
> +		sched_yield();
>  	}
>  
>  	if (i >= THREAD_NUM) {
> @@ -158,6 +199,10 @@ int main()
>  			exit(PTS_UNRESOLVED);
>  		}
>  	}
> +
> +	pthread_mutex_destroy(&td.mutex);
> +	pthread_cond_destroy(&td.cond);;
> +
>  	printf("Test PASSED\n");
>  	return PTS_PASS;
> -}
> \ No newline at end of file
> +}

And as far as I remeber the original problem was that some of the
threads failed to wake up because the while loop that wakes them was too
tight. Does inserting sched_yield() there help?

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] pthread_cond_signal/1-1: use sched_yield to schedule child threads

[chrubis]

Hi!
> >>This case fails on mips board routerstation randomly. The root cause is
> >>that when main thread call usleep(100) after signal the child threads,
> >>no child thread is scheduled to run.  So the case fails.
> >>
> >>I update it to set main thread with a lower priority and child threads
> >>with a higher one, then when sched_yield is called in main thread, one
> >>of the child threads will be scheduled and run.
> >>
> >>Signed-off-by: Kang Kai<kai.kang@windriver.com>
> >>---
> >>  .../interfaces/pthread_cond_signal/1-1.c           |   55 ++++++++++++++++++--
> >>  1 files changed, 50 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c
> >>index bf55b31..51c0cfb 100644
> >>--- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c
> >>+++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_cond_signal/1-1.c
> >>@@ -11,6 +11,7 @@
> >>   */
> >>
> >>  #define _XOPEN_SOURCE 600
> >>+#define _GNU_SOURCE
> >This is open posix testsuite keep that in mind so inserting _GNU_SOURCE
> >is not allowed here.
> 
> _GNU_SOURCE is needed by set_affinity().
> I once thought multi-processor will break the sync between main
> thread and children threads. Maybe I am wrong that set_affinity() is
> not needed.

Ah, then we are missing the _GNU_SOUCE in the affinity.h.

> >And as far as I remeber the original problem was that some of the
> >threads failed to wake up because the while loop that wakes them was too
> >tight. Does inserting sched_yield() there help?
> >
> 
> Only insert sched_yield() can NOT make sure the main thread really
> sleep. In my test increase usleep to 1000 works.
> I still hope the last patch which increase the usleep value can be
> accept. If can't, I'll send a V2.

Hmm, complicated. What about calling sched_yield() and then doing the
usleep with slightly increased time?

Ideal solution would of course be synchronization of the main thread
with the children so that the main thread would wait until the some
child has been scheduled, but that would require more complex code.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list