[Top][All Lists]

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

Re: [Qemu-stable] [Qemu-devel] [PATCH 1/4] fifolock: create rfifolock_is

From: Denis V. Lunev
Subject: Re: [Qemu-stable] [Qemu-devel] [PATCH 1/4] fifolock: create rfifolock_is_locked helper
Date: Mon, 2 Nov 2015 17:02:18 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

On 11/02/2015 04:55 PM, Paolo Bonzini wrote:

On 02/11/2015 14:39, Denis V. Lunev wrote:
This is thread-safe:

bool owner;

owner = r->nesting > 0 && qemu_thread_is_self(&r->owner_thread);

return owner;
yep, I know.

But I do not want to take the lock for check.
You can use a trylock.  If it fails, it is definitely safe to return false.

IMHO it would be better to

@@ -68,11 +68,16 @@ void rfifolock_lock(RFifoLock *r)
  void rfifolock_unlock(RFifoLock *r)
-    assert(r->nesting > 0);
-    assert(qemu_thread_is_self(&r->owner_thread));
+    assert(rfifolock_is_owner(r));
      if (--r->nesting == 0) {
+        qemu_thread_clear(&r->owner_thread);
+bool rfifolock_is_owner(RFifoLock *r)
+    return r->nesting > 0 && qemu_thread_is_self(&r->owner_thread);

which does not require lock and thread safe.
I think it requires memory barriers though.  But it can be simplified:
if you clear owner_thread, you do not need to check r->nesting in

Clearing owner_thread can be done with a simple memset.
this does not require memory barrier as call to qemu_call_broadcast
will do the job just fine.

The check for ownership is actually enough as:
- current thread could be set in the current thread only and
  cleared in the same current thread only. This is not racy at all :)
- count check is moved here for convenience only by request of
  Stefan, it is not required at all to make a decision with after
  clearing the thread.

OK, I can use memset for sure if it will be accepted :)
This was my first opinion.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]