From 77f4593b094802140281ad55167fe64ef1a986af Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 19 Jul 2017 09:31:50 +0200 Subject: [PATCH] virobject: Introduce virObjectRWLockable Up until now we only had virObjectLockable which uses mutexes for mutually excluding each other in critical section. Well, this is not enough. Future work will require RW locks so we might as well have virObjectRWLockable which is introduced here. Moreover, polymorphism is introduced to our code for the first time. Yay! More specifically, virObjectLock will grab a write lock, virObjectLockRead will grab a read lock then (what a surprise right?). This has great advantage that an object can be made derived from virObjectRWLockable in a single line and still continue functioning properly (mutexes can be viewed as grabbing write locks only). Then just those critical sections that can grab a read lock need fixing. Therefore the resulting change is going to be way smaller. In order to avoid writer starvation, the object initializes RW lock that prefers writers. Signed-off-by: Michal Privoznik Reviewed-by: Pavel Hrdina --- src/libvirt_private.syms | 3 + src/util/virobject.c | 145 ++++++++++++++++++++++++++++++--------- src/util/virobject.h | 16 +++++ 3 files changed, 132 insertions(+), 32 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e58b5ebe32..87753758f9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2283,6 +2283,7 @@ virNumaSetupMemoryPolicy; # util/virobject.h virClassForObject; virClassForObjectLockable; +virClassForObjectRWLockable; virClassIsDerivedFrom; virClassName; virClassNew; @@ -2293,8 +2294,10 @@ virObjectListFree; virObjectListFreeCount; virObjectLock; virObjectLockableNew; +virObjectLockRead; virObjectNew; virObjectRef; +virObjectRWLockableNew; virObjectUnlock; virObjectUnref; diff --git a/src/util/virobject.c b/src/util/virobject.c index 34805d34af..4236abfef7 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -49,8 +49,10 @@ struct _virClass { static virClassPtr virObjectClass; static virClassPtr virObjectLockableClass; +static virClassPtr virObjectRWLockableClass; static void virObjectLockableDispose(void *anyobj); +static void virObjectRWLockableDispose(void *anyobj); static int virObjectOnceInit(void) @@ -67,6 +69,12 @@ virObjectOnceInit(void) virObjectLockableDispose))) return -1; + if (!(virObjectRWLockableClass = virClassNew(virObjectClass, + "virObjectRWLockable", + sizeof(virObjectRWLockable), + virObjectRWLockableDispose))) + return -1; + return 0; } @@ -103,6 +111,21 @@ virClassForObjectLockable(void) } +/** + * virClassForObjectRWLockable: + * + * Returns the class instance for the virObjectRWLockable type + */ +virClassPtr +virClassForObjectRWLockable(void) +{ + if (virObjectInitialize() < 0) + return NULL; + + return virObjectRWLockableClass; +} + + /** * virClassNew: * @parent: the parent class @@ -237,6 +260,32 @@ virObjectLockableNew(virClassPtr klass) } +void * +virObjectRWLockableNew(virClassPtr klass) +{ + virObjectRWLockablePtr obj; + + if (!virClassIsDerivedFrom(klass, virClassForObjectRWLockable())) { + virReportInvalidArg(klass, + _("Class %s must derive from virObjectRWLockable"), + virClassName(klass)); + return NULL; + } + + if (!(obj = virObjectNew(klass))) + return NULL; + + if (virRWLockInitPreferWriter(&obj->lock) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize RW lock")); + virObjectUnref(obj); + return NULL; + } + + return obj; +} + + static void virObjectLockableDispose(void *anyobj) { @@ -246,6 +295,15 @@ virObjectLockableDispose(void *anyobj) } +static void +virObjectRWLockableDispose(void *anyobj) +{ + virObjectRWLockablePtr obj = anyobj; + + virRWLockDestroy(&obj->lock); +} + + /** * virObjectUnref: * @anyobj: any instance of virObjectPtr @@ -309,28 +367,13 @@ virObjectRef(void *anyobj) } -static virObjectLockablePtr -virObjectGetLockableObj(void *anyobj) -{ - virObjectPtr obj; - - if (virObjectIsClass(anyobj, virObjectLockableClass)) - return anyobj; - - obj = anyobj; - VIR_WARN("Object %p (%s) is not a virObjectLockable instance", - anyobj, obj ? obj->klass->name : "(unknown)"); - - return NULL; -} - - /** * virObjectLock: - * @anyobj: any instance of virObjectLockablePtr + * @anyobj: any instance of virObjectLockable or virObjectRWLockable * - * Acquire a lock on @anyobj. The lock must be - * released by virObjectUnlock. + * Acquire a lock on @anyobj. The lock must be released by + * virObjectUnlock. In case the passed object is instance of + * virObjectRWLockable a write lock is acquired. * * The caller is expected to have acquired a reference * on the object before locking it (eg virObjectRef). @@ -340,31 +383,69 @@ virObjectGetLockableObj(void *anyobj) void virObjectLock(void *anyobj) { - virObjectLockablePtr obj = virObjectGetLockableObj(anyobj); + if (virObjectIsClass(anyobj, virObjectLockableClass)) { + virObjectLockablePtr obj = anyobj; + virMutexLock(&obj->lock); + } else if (virObjectIsClass(anyobj, virObjectRWLockableClass)) { + virObjectRWLockablePtr obj = anyobj; + virRWLockWrite(&obj->lock); + } else { + virObjectPtr obj = anyobj; + VIR_WARN("Object %p (%s) is not a virObjectLockable " + "nor virObjectRWLockable instance", + anyobj, obj ? obj->klass->name : "(unknown)"); + } +} - if (!obj) - return; - virMutexLock(&obj->lock); +/** + * virObjectLockRead: + * @anyobj: any instance of virObjectRWLockable + * + * Acquire a read lock on @anyobj. The lock must be + * released by virObjectUnlock. + * + * The caller is expected to have acquired a reference + * on the object before locking it (eg virObjectRef). + * The object must be unlocked before releasing this + * reference. + */ +void +virObjectLockRead(void *anyobj) +{ + if (virObjectIsClass(anyobj, virObjectRWLockableClass)) { + virObjectRWLockablePtr obj = anyobj; + virRWLockRead(&obj->lock); + } else { + virObjectPtr obj = anyobj; + VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance", + anyobj, obj ? obj->klass->name : "(unknown)"); + } } /** * virObjectUnlock: - * @anyobj: any instance of virObjectLockablePtr + * @anyobj: any instance of virObjectLockable or virObjectRWLockable * - * Release a lock on @anyobj. The lock must have been - * acquired by virObjectLock. + * Release a lock on @anyobj. The lock must have been acquired by + * virObjectLock or virObjectLockRead. */ void virObjectUnlock(void *anyobj) { - virObjectLockablePtr obj = virObjectGetLockableObj(anyobj); - - if (!obj) - return; - - virMutexUnlock(&obj->lock); + if (virObjectIsClass(anyobj, virObjectLockableClass)) { + virObjectLockablePtr obj = anyobj; + virMutexUnlock(&obj->lock); + } else if (virObjectIsClass(anyobj, virObjectRWLockableClass)) { + virObjectRWLockablePtr obj = anyobj; + virRWLockUnlock(&obj->lock); + } else { + virObjectPtr obj = anyobj; + VIR_WARN("Object %p (%s) is not a virObjectLockable " + "nor virObjectRWLockable instance", + anyobj, obj ? obj->klass->name : "(unknown)"); + } } diff --git a/src/util/virobject.h b/src/util/virobject.h index f4c292b16c..5985fadb2d 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -34,6 +34,9 @@ typedef virObject *virObjectPtr; typedef struct _virObjectLockable virObjectLockable; typedef virObjectLockable *virObjectLockablePtr; +typedef struct _virObjectRWLockable virObjectRWLockable; +typedef virObjectRWLockable *virObjectRWLockablePtr; + typedef void (*virObjectDisposeCallback)(void *obj); /* Most code should not play with the contents of this struct; however, @@ -59,9 +62,14 @@ struct _virObjectLockable { virMutex lock; }; +struct _virObjectRWLockable { + virObject parent; + virRWLock lock; +}; virClassPtr virClassForObject(void); virClassPtr virClassForObjectLockable(void); +virClassPtr virClassForObjectRWLockable(void); # ifndef VIR_PARENT_REQUIRED # define VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(1) @@ -108,10 +116,18 @@ void * virObjectLockableNew(virClassPtr klass) ATTRIBUTE_NONNULL(1); +void * +virObjectRWLockableNew(virClassPtr klass) + ATTRIBUTE_NONNULL(1); + void virObjectLock(void *lockableobj) ATTRIBUTE_NONNULL(1); +void +virObjectLockRead(void *lockableobj) + ATTRIBUTE_NONNULL(1); + void virObjectUnlock(void *lockableobj) ATTRIBUTE_NONNULL(1);