Change in vdsm[master]: rwlock: Add simpler RWLock

nsoffer at redhat.com nsoffer at redhat.com
Mon Dec 21 13:58:23 UTC 2015


Nir Soffer has posted comments on this change.

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


Patch Set 22:

(4 comments)

https://gerrit.ovirt.org/#/c/42908/22//COMMIT_MSG
Commit Message:

Line 29: -------------------
Line 30: 
Line 31: writers: 1 readers: 2
Line 32: writes  avg=3381.00 med=3381 min=3381 max=3381
Line 33: reads   avg=3367.00 med=3376 min=3358 max=3376
> what those numbers mean?
average/median/mininum/maximum number of times a lock was taken by readers or by writers during a test. This is the output of the RWLockStressTests results, running with different number of readers and writers.

Higher numbers means the lock is more efficient.
Line 34: 
Line 35: writers: 2 readers: 8
Line 36: writes  avg=945.00 med=945 min=945 max=945
Line 37: reads   avg=937.38 med=938 min=933 max=940


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

Line 85: 
Line 86:     def acquire_write(self):
Line 87:         me = threading.current_thread()
Line 88:         if me is self._writer:
Line 89:             self._holders[me] += 1
> this for the recursive lock I guess
Yes
Line 90:             return
Line 91:         if me in self._holders:
Line 92:             raise RuntimeError("Lock promotion is forbidden")
Line 93:         with self._lock:


Line 107:             if self._writer or self._waiters:
Line 108:                 self._wait(False)
Line 109:             self._holders[me] = 1
Line 110:             if self._waiters:
Line 111:                 self._grant_next_waiter()
> can't it cause a race? set the event while same holder can acquire the lock
Can you describe the flow that may be racy?

At this point, the current thread is holding self._lock. No other thread can access this object, except another reader that already hold this lock.

The waiter list may have other threads waiting to take a read or write lock.

grant_next_waiter will wake up the next reader thread, and it will return from self._wait(), take a read lock and wake up the next waiter.
Line 112: 
Line 113:     def release(self):
Line 114:         me = threading.current_thread()
Line 115:         if me not in self._holders:


Line 108:                 self._wait(False)
Line 109:             self._holders[me] = 1
Line 110:             if self._waiters:
Line 111:                 self._grant_next_waiter()
Line 112: 
> and after this scope you release _lock as its under context. the lock shoul
No, the lock is protecting the rwlock internals. Both readers and writers take this lock for very short time, marking the rwlock as "locked" for reading or writing.

The actual waiting on the lock (e.g. acquire_read() when a writer is holding the rwlock) is implemented using a list of Waiter objects.
Line 113:     def release(self):
Line 114:         me = threading.current_thread()
Line 115:         if me not in self._holders:
Line 116:             raise RuntimeError("Thread %s attempted to release a lock it "


-- 
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: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dmitrykuzn at gmail.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list