Re: [Qemu-devel] [PATCH 1/4] fifolock: create rfifolock_is_locked helper

["Denis V. Lunev" <den@openvz.org>]

On 10/30/2015 06:41 PM, Stefan Hajnoczi wrote:
> On Wed, Oct 28, 2015 at 06:01:02PM +0300, Denis V. Lunev wrote:
>> +int  rfifolock_is_locked(RFifoLock *r);
> Please use bool instead of int.
>
>> diff --git a/util/rfifolock.c b/util/rfifolock.c
>> index afbf748..8ac58cb 100644
>> --- a/util/rfifolock.c
>> +++ b/util/rfifolock.c
>> @@ -48,7 +48,7 @@ void rfifolock_lock(RFifoLock *r)
>>       /* Take a ticket */
>>       unsigned int ticket = r->tail++;
>>   
>> -    if (r->nesting > 0 && qemu_thread_is_self(&r->owner_thread)) {
>> +    if (r->nesting > 0 && rfifolock_is_locked(r)) {
>>           r->tail--; /* put ticket back, we're nesting */
>>       } else {
>>           while (ticket != r->head) {
>> @@ -69,10 +69,15 @@ void rfifolock_unlock(RFifoLock *r)
>>   {
>>       qemu_mutex_lock(&r->lock);
>>       assert(r->nesting > 0);
>> -    assert(qemu_thread_is_self(&r->owner_thread));
>> +    assert(rfifolock_is_locked(r));
>>       if (--r->nesting == 0) {
>>           r->head++;
>>           qemu_cond_broadcast(&r->cond);
>>       }
>>       qemu_mutex_unlock(&r->lock);
>>   }
>> +
>> +int rfifolock_is_locked(RFifoLock *r)
>> +{
>> +    return qemu_thread_is_self(&r->owner_thread);
>> +}
> The function name confused me since "does the current thread hold the
> lock?" != "does anyone currently hold the lock?".
>
> I suggest:
>
> bool rfifolock_held_by_current_thread(RFifoLock *r) {
>      return r->nesting > 0 && qemu_thread_is_self(&r->owner_thread);
> }
>
> Then the r->nesting > 0 testing can also be dropped by callers, which is
> good since rfifolock_is_locked() does not return a meaningful result
> when r->nesting == 0.
what about

rfifolock_is_owner() ?

Den