[LTP] [PATCH] prio-wake: avoid glibc to kernel sleep race

[LTP] [PATCH] prio-wake: avoid glibc to kernel sleep race

[Darren Hart]

prio-wake: avoid glibc to kernel sleep race

In the unlocked broadcast scenario, there exists a race between when the
running_threads variable reaches rt_threads and when the last worker_thread
blocks in the kernel after a cond_wait(). It is possible for a thread to miss
the broadcast if it fails to sleep before the broadcast is issued.  

The previous code did not guarantee a small window of time to allow the threads
to get to sleep.  It also used an unreasonably large sleep time which
unnecessarily extended the length of the test run time.  This patch ensures some
time is given to the threads to get to sleep and at the same time uses a much
shorter (1000x) sleep period which results in a 50-100x reduction in test run
time. Lastly, two unecessary loops waiting for the threads to complete were
removed, relying on pthread_join() instead to wait for the threads to complete.

Signed-off-by: Darren Hart <dvhltc@us.ibm.com>
Acked-by: Will Schmidt <will_schmidt@vnet.ibm.com>

---
 func/prio-wake/prio-wake.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Index: realtime/func/prio-wake/prio-wake.c
===================================================================
--- realtime.orig/func/prio-wake/prio-wake.c
+++ realtime/func/prio-wake/prio-wake.c
@@ -108,7 +108,10 @@ void *master_thread(void* arg)
 
 	/* make sure children are started */
 	while (running_threads < rt_threads)
-		sleep(1);
+		usleep(1000);
+	/* give the worker threads a chance to get to sleep in the kernel
+	 * in the unlocked broadcast case. */
+	usleep(1000);
 
 	start = rt_gettime() - beginrun;
 
@@ -120,8 +123,6 @@ void *master_thread(void* arg)
 	if (locked_broadcast)
 		rc = pthread_mutex_unlock(&mutex);
 
-	while (running_threads > 0)
-		sleep(1);
 	return NULL;
 }
 
@@ -157,9 +158,6 @@ void *worker_thread(void* arg)
 
 	rc = pthread_mutex_unlock(&mutex);
 
-	/* wait for all threads to quit */
-	while (running_threads > 0)
-		sleep(1);
 	return NULL;
 }
 
-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

------------------------------------------------------------------------------
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] prio-wake: avoid glibc to kernel sleep race

[Darren Hart]

This patch does not appear in current CVS. Please apply.

Thanks,

Darren Hart

Darren Hart wrote:
> prio-wake: avoid glibc to kernel sleep race
> 
> In the unlocked broadcast scenario, there exists a race between when the
> running_threads variable reaches rt_threads and when the last worker_thread
> blocks in the kernel after a cond_wait(). It is possible for a thread to miss
> the broadcast if it fails to sleep before the broadcast is issued.  
> 
> The previous code did not guarantee a small window of time to allow the threads
> to get to sleep.  It also used an unreasonably large sleep time which
> unnecessarily extended the length of the test run time.  This patch ensures some
> time is given to the threads to get to sleep and at the same time uses a much
> shorter (1000x) sleep period which results in a 50-100x reduction in test run
> time. Lastly, two unecessary loops waiting for the threads to complete were
> removed, relying on pthread_join() instead to wait for the threads to complete.
> 
> Signed-off-by: Darren Hart <dvhltc@us.ibm.com>
> Acked-by: Will Schmidt <will_schmidt@vnet.ibm.com>
> 
> ---
>  func/prio-wake/prio-wake.c |   10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> Index: realtime/func/prio-wake/prio-wake.c
> ===================================================================
> --- realtime.orig/func/prio-wake/prio-wake.c
> +++ realtime/func/prio-wake/prio-wake.c
> @@ -108,7 +108,10 @@ void *master_thread(void* arg)
> 
>  	/* make sure children are started */
>  	while (running_threads < rt_threads)
> -		sleep(1);
> +		usleep(1000);
> +	/* give the worker threads a chance to get to sleep in the kernel
> +	 * in the unlocked broadcast case. */
> +	usleep(1000);
> 
>  	start = rt_gettime() - beginrun;
> 
> @@ -120,8 +123,6 @@ void *master_thread(void* arg)
>  	if (locked_broadcast)
>  		rc = pthread_mutex_unlock(&mutex);
> 
> -	while (running_threads > 0)
> -		sleep(1);
>  	return NULL;
>  }
> 
> @@ -157,9 +158,6 @@ void *worker_thread(void* arg)
> 
>  	rc = pthread_mutex_unlock(&mutex);
> 
> -	/* wait for all threads to quit */
> -	while (running_threads > 0)
> -		sleep(1);
>  	return NULL;
>  }
> 


-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH] prio-wake: avoid glibc to kernel sleep race

[Rishikesh K Rajak]

On Wed, Apr 21, 2010 at 01:23:18PM -0700, Darren Hart wrote:
> This patch does not appear in current CVS. Please apply.
Hi darren,

Committed
5162b710bfc574107d915093c08b198f0a15727e

Thanks.

-Rishi
> 
> Thanks,
> 
> Darren Hart
> 
> Darren Hart wrote:
> > prio-wake: avoid glibc to kernel sleep race
> > 
> > In the unlocked broadcast scenario, there exists a race between when the
> > running_threads variable reaches rt_threads and when the last worker_thread
> > blocks in the kernel after a cond_wait(). It is possible for a thread to miss
> > the broadcast if it fails to sleep before the broadcast is issued.  
> > 
> > The previous code did not guarantee a small window of time to allow the threads
> > to get to sleep.  It also used an unreasonably large sleep time which
> > unnecessarily extended the length of the test run time.  This patch ensures some
> > time is given to the threads to get to sleep and at the same time uses a much
> > shorter (1000x) sleep period which results in a 50-100x reduction in test run
> > time. Lastly, two unecessary loops waiting for the threads to complete were
> > removed, relying on pthread_join() instead to wait for the threads to complete.
> > 
> > Signed-off-by: Darren Hart <dvhltc@us.ibm.com>
> > Acked-by: Will Schmidt <will_schmidt@vnet.ibm.com>
> > 
> > ---
> >  func/prio-wake/prio-wake.c |   10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > Index: realtime/func/prio-wake/prio-wake.c
> > ===================================================================
> > --- realtime.orig/func/prio-wake/prio-wake.c
> > +++ realtime/func/prio-wake/prio-wake.c
> > @@ -108,7 +108,10 @@ void *master_thread(void* arg)
> > 
> >  	/* make sure children are started */
> >  	while (running_threads < rt_threads)
> > -		sleep(1);
> > +		usleep(1000);
> > +	/* give the worker threads a chance to get to sleep in the kernel
> > +	 * in the unlocked broadcast case. */
> > +	usleep(1000);
> > 
> >  	start = rt_gettime() - beginrun;
> > 
> > @@ -120,8 +123,6 @@ void *master_thread(void* arg)
> >  	if (locked_broadcast)
> >  		rc = pthread_mutex_unlock(&mutex);
> > 
> > -	while (running_threads > 0)
> > -		sleep(1);
> >  	return NULL;
> >  }
> > 
> > @@ -157,9 +158,6 @@ void *worker_thread(void* arg)
> > 
> >  	rc = pthread_mutex_unlock(&mutex);
> > 
> > -	/* wait for all threads to quit */
> > -	while (running_threads > 0)
> > -		sleep(1);
> >  	return NULL;
> >  }
> > 
> 
> 
> -- 
> Darren Hart
> IBM Linux Technology Center
> Real-Time Linux Team
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Ltp-list mailing list
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list

-- 
Thanks & Regards
Rishi
LTP Maintainer
IBM, LTC, Bangalore
Please join IRC #ltp @ irc.freenode.net

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list