From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42899)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <den@openvz.org>) id 1ZsGJb-00078C-Hv
	for qemu-devel@nongnu.org; Fri, 30 Oct 2015 16:30:24 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <den@openvz.org>) id 1ZsGJW-0008RP-Fe
	for qemu-devel@nongnu.org; Fri, 30 Oct 2015 16:30:23 -0400
Received: from relay.parallels.com ([195.214.232.42]:52716)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <den@openvz.org>) id 1ZsGJW-0008R2-7l
	for qemu-devel@nongnu.org; Fri, 30 Oct 2015 16:30:18 -0400
References: <1446044465-19312-1-git-send-email-den@openvz.org>
	<1446044465-19312-2-git-send-email-den@openvz.org>
	<20151030154135.GA16864@stefanha-x1.localdomain>
From: "Denis V. Lunev" <den@openvz.org>
Message-ID: <5633D34C.6000405@openvz.org>
Date: Fri, 30 Oct 2015 23:30:04 +0300
MIME-Version: 1.0
In-Reply-To: <20151030154135.GA16864@stefanha-x1.localdomain>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 1/4] fifolock: create rfifolock_is_locked
	helper
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org, qemu-stable@nongnu.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