On Wed, Jul 19, 2017 at 04:31:49PM +0200, Michal Privoznik wrote:
> 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 <mprivozn@xxxxxxxxxx>
> ---
> src/libvirt_private.syms | 3 +
> src/util/virobject.c | 144 ++++++++++++++++++++++++++++++++++++-----------
> src/util/virobject.h | 16 ++++++
> 3 files changed, 131 insertions(+), 32 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a792e00c8..f9df35583 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2282,6 +2282,7 @@ virNumaSetupMemoryPolicy;
> # util/virobject.h
> virClassForObject;
> virClassForObjectLockable;
> +virClassForObjectRWLockable;
> virClassIsDerivedFrom;
> virClassName;
> virClassNew;
> @@ -2292,8 +2293,10 @@ virObjectListFree;
> virObjectListFreeCount;
> virObjectLock;
> virObjectLockableNew;
> +virObjectLockRead;
> virObjectNew;
> virObjectRef;
> +virObjectRWLockableNew;
> virObjectUnlock;
> virObjectUnref;
>
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index 34805d34a..3e7a0719e 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,20 @@ virClassForObjectLockable(void)
> }
>
>
> +/**
> + * virClassForObjectRWLockable:
> + *
> + * Returns the class instance for the virObjectRWLockable type
> + */
> +virClassPtr
> +virClassForObjectRWLockable(void)
> +{
> + if (virObjectInitialize() < 0)
> + return NULL;
> +
> + return virObjectRWLockableClass;
> +}
> +
There should be two empty lines.
> /**
> * virClassNew:
> * @parent: the parent class
> @@ -237,6 +259,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 +294,15 @@ virObjectLockableDispose(void *anyobj)
> }
>
>
> +static void
> +virObjectRWLockableDispose(void *anyobj)
> +{
> + virObjectRWLockablePtr obj = anyobj;
> +
> + virRWLockDestroy(&obj->lock);
> +}
> +
> +
> /**
> * virObjectUnref:
> * @anyobj: any instance of virObjectPtr
> @@ -309,28 +366,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 +382,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 virObjectRWLockablePtr
s/virObjectRWLockablePtr/virObjectRWLockable/
Reviewed-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
