[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/14] Adding recursive locks



On 03/18/2010 10:51 AM, Daniel P. Berrange wrote:
> On Thu, Mar 18, 2010 at 10:41:58AM -0600, Eric Blake wrote:
>> On 03/18/2010 09:15 AM, Stefan Berger wrote:
>>> This patch adds recursive locks necessary due to the processing of
>>> network filter XML that can reference other network filters, including
>>> references that cause looks. Loops in the XML are prevented but their
>>> detection requires recursive locks.
>>
>> ACK.  I had to double-check MSDN to make sure that threads-win32 already
>> creates recursive mutex by default.
> 
> I don't supposed you read if its possible to make it non-recursive, since
> the orginal impl is supposed to be non-recursive !

Unfortunately, I don't see a way:

http://msdn.microsoft.com/en-us/library/ms684266%28VS.85%29.aspx

states:

After a thread obtains ownership of a mutex, it can specify the same
mutex in repeated calls to the wait-functions without blocking its
execution. This prevents a thread from deadlocking itself while waiting
for a mutex that it already owns. To release its ownership under such
circumstances, the thread must call ReleaseMutex once for each time that
the mutex satisfied the conditions of a wait function.

with no mention of any way to parameterize it to be non-recursive.

But what does a true non-recursive mutex buy you?  The only difference
between recursive and true non-recursive is whether you declare that an
attempt to relock a mutex that you already own is a fatal deadlock
error, rather than incrementing a counter for matching unlocks.  It's
just that non-recursive mutexes typically have faster implementations.

For that matter, do we even need the distinction?  Maybe ALL our code
should be using recursive mutexes by default, by changing virMutexInit
to be recursive no matter what, and not worry about introducing
virMutexInitRecursive.  Looking more closely at virMutexInit in the
pthreads version, we use pthread_mutex_init(,NULL), which requests
PTHREAD_MUTEX_DEFAULT.  But POSIX states:

http://www.opengroup.org/onlinepubs/9699919799/functions/pthread_mutexattr_settype.html

PTHREAD_MUTEX_DEFAULT

    Attempting to recursively lock a mutex of this type results in
undefined behavior. Attempting to unlock a mutex of this type which was
not locked by the calling thread results in undefined behavior.
Attempting to unlock a mutex of this type which is not locked results in
undefined behavior. An implementation may map this mutex to one of the
other mutex types.

That is, our current implementation of virMutexInit is NOT a true
non-recursive mutex, so much as a mutex that is unspecified whether it
is recursive or not.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]