On Wed, Apr 07, 2010 at 09:17:04AM -0400, Nicolas Pitre wrote: > On Wed, 7 Apr 2010, Shawn Pearce wrote: > > You mentioned avoiding a recursive mutex only because windows > > emulation doesn't have support for it. But that's exactly what we > > need here. Shouldn't windows have a recursive mutex object that can > > just be used inside of the emulation layer when we really need a > > recursive mutex? > > Maybe. That would in fact just mean pushing the double mutex issue into > the pthread emulation instead of having it outside it. This would > impact performances for all mutexes although only one instance of them > currently require a recursive behavior. As I mentioned in another mail in this thread, our mutex implementation on WIN32 already is recursive. It is implemented on top of the CRITICAL_SECTION type, which is recursive. See http://msdn.microsoft.com/en-us/library/ms682530%28VS.85%29.aspx We only need something like the following (on top of Nico's previous patch). Warning: It hasn't even been compile tested on WIN32. diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 65f797f..19e42cf 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1559,7 +1559,7 @@ static pthread_cond_t progress_cond; */ static void init_threaded_search(void) { - pthread_mutex_init(&read_mutex, NULL); + init_recursive_mutex(&read_mutex); pthread_mutex_init(&cache_mutex, NULL); pthread_mutex_init(&progress_mutex, NULL); pthread_cond_init(&progress_cond, NULL); diff --git a/thread-utils.c b/thread-utils.c index 4f9c829..3c8d817 100644 --- a/thread-utils.c +++ b/thread-utils.c @@ -1,4 +1,5 @@ #include "cache.h" +#include <pthread.h> #if defined(hpux) || defined(__hpux) || defined(_hpux) # include <sys/pstat.h> @@ -43,3 +44,24 @@ int online_cpus(void) return 1; } + +int init_recursive_mutex(pthread_mutex_t *m) +{ +#ifdef _WIN32 + /* The mutexes in the WIN32 pthreads emulation layer are + * recursive, so we don't have to do anything extra here. */ + return pthread_mutex_init(m, NULL); +#else + pthread_mutexattr_t a; + int ret; + if (pthread_mutexattr_init(&a)) + die("pthread_mutexattr_init failed: %s", strerror(errno)); + + if (pthread_mutexattr_settype(&a, PTHREAD_MUTEX_RECURSIVE)) + die("pthread_mutexattr_settype failed: %s", strerror(errno)); + + ret = pthread_mutex_init(m, &a); + pthread_mutexattr_destroy(&a); + return ret; +#endif +} diff --git a/thread-utils.h b/thread-utils.h index cce4b77..1727a03 100644 --- a/thread-utils.h +++ b/thread-utils.h @@ -2,5 +2,6 @@ #define THREAD_COMPAT_H extern int online_cpus(void); +extern int init_recursive_mutex(pthread_mutex_t*); #endif /* THREAD_COMPAT_H */ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html