Nir Soffer has uploaded a new change for review.

Change subject: misc: Simplify DynamicBarrier implementation
......................................................................

misc: Simplify DynamicBarrier implementation

DynamicBarrier uses a threading.Condition. The condition
already contain a lock, used to synchronize enter() and exit().

Within the critical sections in enter() and exit(), only one thread can
run. When a thread call self._cond.wait(), the condition lock is
released atomically and the next thread waiting on the condition lock
continue to run. When a thread return from self.cond.wait(), it get the
condition lock and run, while all other threads are waiting on the
condition lock. This is ensured by threading.Condition, both Python
builtin and pthread_cond when using pthreading.

To keep a boolean state of the barrier, we don't need another lock, a
boolean is just fine.

This patch assume that Condition.wait() returns only when notified. This
is ensured by threading.Condition, but not ensured by
pthreading.Condition, which uses pthread_cond_wait. So this depends on
fixing this issue pthreadng, see:
https://github.com/nirs/pthreading/commit/29af6d12b49ad07eca2de10a72adff540fdfc69b

Change-Id: Ia403cd9f734eaafdce95ba5123e54ec4e4939172
Signed-off-by: Nir Soffer <[email protected]>
---
M vdsm/storage/misc.py
1 file changed, 6 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/03/29003/1

diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py
index 3316de7..fbc6059 100644
--- a/vdsm/storage/misc.py
+++ b/vdsm/storage/misc.py
@@ -652,8 +652,8 @@
 
 class DynamicBarrier(object):
     def __init__(self):
-        self._lock = threading.Lock()
         self._cond = threading.Condition()
+        self._busy = False
 
     def enter(self):
         """
@@ -670,8 +670,9 @@
         >>    dynamicBarrier.exit()
         """
         with self._cond:
-            if self._lock.acquire(False):
+            if not self._busy:
                 # The first thread entered the barrier.
+                self._busy = True
                 return True
 
             self._cond.wait()
@@ -680,8 +681,9 @@
             # when the barrier was entered, and so they cannot use the result
             # obtained by this thread.
 
-            if self._lock.acquire(False):
+            if not self._busy:
                 # The second thread entered the barrier.
+                self._busy = True
                 return True
 
             self._cond.wait()
@@ -695,7 +697,7 @@
 
     def exit(self):
         with self._cond:
-            self._lock.release()
+            self._busy = False
             self._cond.notifyAll()
 
 


-- 
To view, visit http://gerrit.ovirt.org/29003
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia403cd9f734eaafdce95ba5123e54ec4e4939172
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to