Nir Soffer has posted comments on this change.

Change subject: rwlock: Add simpler RWLock
......................................................................


Patch Set 4:

(2 comments)

https://gerrit.ovirt.org/#/c/42908/4/lib/vdsm/rwlock.py
File lib/vdsm/rwlock.py:

Line 31:         self._waiters = []
Line 32:         self._readers = set()
Line 33:         self._writer = None
Line 34: 
Line 35:     def acquireWrite(self):
> Can we have pep8 method names?
I'm tried to keep compatibility with misc.RWLock, so this can version can 
replace the old one.

But it seems that nobody is calling the old code acquireRead and acquireWrite - 
only shared and exclusive context managers are used in the current code. So I 
will change the api to pep8 style.
Line 36:         with self._lock:
Line 37:             if self._writer or self._readers or self._waiters:
Line 38:                 self._wait(True)
Line 39:             self._writer = threading.current_thread()


Line 73:                 self._lock.acquire()
Line 74:         finally:
Line 75:             self._waiters.remove(waiter)
Line 76: 
Line 77:     def _wakeup_waiter(self):
> Does this wake up all queued readers after write unlock?
This wakes up exactly one waiter, the one that got first in the queue.

If there are multiple readers, each will wake up the next. But it looks like 
that a reader will wake up the next reader when releasing the read lock, 
instead of when taking the read lock. I will add another test for this.
Line 78:         if self._readers and self._waiters[0].wants_write:
Line 79:             return
Line 80:         self._waiters[0].wakeup()
Line 81: 


-- 
To view, visit https://gerrit.ovirt.org/42908
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2466c137c89598772fb46347eb02195916883cac
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Ala Hino <ah...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuzn...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to