From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25746 invoked by alias); 19 Feb 2007 16:52:53 -0000 Received: (qmail 25729 invoked by uid 22791); 19 Feb 2007 16:52:52 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 19 Feb 2007 16:52:46 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.1/8.13.1) with ESMTP id l1JGqikl020576 for ; Mon, 19 Feb 2007 11:52:44 -0500 Received: from pobox.toronto.redhat.com (pobox.toronto.redhat.com [172.16.14.4]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id l1JGqc1J029061 for ; Mon, 19 Feb 2007 11:52:39 -0500 Received: from [172.16.14.227] (topaz.toronto.redhat.com [172.16.14.227]) by pobox.toronto.redhat.com (8.12.11.20060308/8.12.11) with ESMTP id l1JGqcoK017146 for ; Mon, 19 Feb 2007 11:52:38 -0500 Message-ID: <45D9D5D6.9000605@redhat.com> Date: Mon, 19 Feb 2007 16:52:00 -0000 From: Dave Brolley User-Agent: Thunderbird 1.5.0.5 (X11/20060719) MIME-Version: 1.0 To: sid@sources.redhat.com Subject: [patch][commit] Single Mutex for sidutil::blocking_component Content-Type: multipart/mixed; boundary="------------030503050307040900080002" X-IsSubscribed: yes Mailing-List: contact sid-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: sid-owner@sourceware.org X-SW-Source: 2007-q1/txt/msg00022.txt.bz2 This is a multi-part message in MIME format. --------------030503050307040900080002 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 647 Hi, I've committed the attached patch which simplifies the implementation of sidutil::blocking_component. The use of two mutex/condition variable pairs for synchronizing the parent and child threads was unnecessary and, if you look at the existing code, you will see that they were always locked/unlocked/blocked in unison. The implementation now uses a single mutex/condition variable pair. The patch also adds checking that a return from pthread_cond_wait was not spontaneous and makes the control_status member volatile for this purpose. Tested extensively using the MeP port which is the only committed port using this class. Dave --------------030503050307040900080002 Content-Type: text/plain; name="sid.blocking_component.ChangeLOg" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="sid.blocking_component.ChangeLOg" Content-length: 368 2007-02-19 Dave Brolley * sidblockingutil.h (sidutil::blocking_component): Remove child_resume_mutex, child_stopped_mutex, chiuld_resume_condition and child_stopped_condition. Use a single mutex/condition pair: child_sync_mutex and child_sync_condition. Handle spontaneous returns from pthread_cond_wait. (control_status): Now volatile. --------------030503050307040900080002 Content-Type: text/plain; name="sid.blocking_component.patch.txt" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="sid.blocking_component.patch.txt" Content-length: 5020 Index: sid/include/sidblockingutil.h =================================================================== RCS file: /cvs/src/src/sid/include/sidblockingutil.h,v retrieving revision 1.3 diff -c -p -r1.3 sidblockingutil.h *** sid/include/sidblockingutil.h 5 Feb 2007 20:28:42 -0000 1.3 --- sid/include/sidblockingutil.h 19 Feb 2007 16:26:33 -0000 *************** namespace sidutil *** 49,57 **** { log (10, "%s: child_init\n", name.c_str ()); assert (child_created); ! // Lock both mutexes ! pthread_mutex_lock (& child_resume_mutex); ! pthread_mutex_lock (& child_stopped_mutex); } protected: --- 49,56 ---- { log (10, "%s: child_init\n", name.c_str ()); assert (child_created); ! // Lock the syncronization mutex ! pthread_mutex_lock (& child_sync_mutex); } protected: *************** namespace sidutil *** 59,73 **** { log (10, "%s: parent_init\n", name.c_str ()); ! // Create mutexes for synchronizing the parent and child threads ! pthread_mutex_init (& child_resume_mutex, NULL); ! pthread_cond_init (& child_resume_condition, NULL); ! pthread_mutex_init (& child_stopped_mutex, NULL); ! pthread_cond_init (& child_stopped_condition, NULL); ! ! // Lock both mutexes ! pthread_mutex_lock (& child_resume_mutex); ! pthread_mutex_lock (& child_stopped_mutex); control_status = ctl_parent; } --- 58,69 ---- { log (10, "%s: parent_init\n", name.c_str ()); ! // Create a mutex for synchronizing the parent and child threads ! pthread_mutex_init (& child_sync_mutex, NULL); ! pthread_cond_init (& child_sync_condition, NULL); ! ! // Lock the mutex ! pthread_mutex_lock (& child_sync_mutex); control_status = ctl_parent; } *************** namespace sidutil *** 121,139 **** // Signal the child to resume assert (control_status != ctl_child_start); control_status = ctl_child_start; ! pthread_cond_signal (& child_resume_condition); ! ! // Unlock the mutex so that the child can gain control ! pthread_mutex_unlock (& child_resume_mutex); // Wait for the return signal from the child ! pthread_cond_wait (& child_stopped_condition, & child_stopped_mutex); ! ! // Reacquire the mutex so that the child can gain control ! pthread_mutex_lock (& child_resume_mutex); - // Check the value of control_status - assert (control_status != ctl_child_start); return control_status; } --- 117,132 ---- // Signal the child to resume assert (control_status != ctl_child_start); control_status = ctl_child_start; ! log (11, "%s: parent signalling the child thread\n", name.c_str ()); ! pthread_cond_signal (& child_sync_condition); // Wait for the return signal from the child ! do { ! log (11, "%s: parent waiting for child thread\n", name.c_str ()); ! pthread_cond_wait (& child_sync_condition, & child_sync_mutex); ! } while (control_status == ctl_child_start); ! log (11, "%s: parent regains control\n", name.c_str ()); return control_status; } *************** namespace sidutil *** 168,185 **** // Signal the parent that we're stopped log (11, "%s: child signalling the parent thread\n", name.c_str ()); ! pthread_cond_signal (& child_stopped_condition); ! ! // Unlock the mutex so that the parent can gain control ! pthread_mutex_unlock (& child_stopped_mutex); // Wait for a signal to resume ! log (11, "%s: child waiting for parent thread\n", name.c_str ()); ! pthread_cond_wait (& child_resume_condition, & child_resume_mutex); ! ! // Reacquire the stopped mutex ! pthread_mutex_lock (& child_stopped_mutex); ! assert (control_status == ctl_child_start); } void set_blockable () --- 161,174 ---- // Signal the parent that we're stopped log (11, "%s: child signalling the parent thread\n", name.c_str ()); ! pthread_cond_signal (& child_sync_condition); // Wait for a signal to resume ! do { ! log (11, "%s: child waiting for parent thread\n", name.c_str ()); ! pthread_cond_wait (& child_sync_condition, & child_sync_mutex); ! } while (control_status != ctl_child_start); ! log (11, "%s: child regains control\n", name.c_str ()); } void set_blockable () *************** namespace sidutil *** 198,208 **** bool child_created; pthread_t child_thread; void *(*child_thread_function)(void *); ! pthread_mutex_t child_resume_mutex; ! pthread_cond_t child_resume_condition; ! pthread_mutex_t child_stopped_mutex; ! pthread_cond_t child_stopped_condition; ! enum { ctl_parent, ctl_child_start, ctl_child_blocked, ctl_child_complete } control_status; --- 187,195 ---- bool child_created; pthread_t child_thread; void *(*child_thread_function)(void *); ! pthread_mutex_t child_sync_mutex; ! pthread_cond_t child_sync_condition; ! volatile enum { ctl_parent, ctl_child_start, ctl_child_blocked, ctl_child_complete } control_status; --------------030503050307040900080002--