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 <[email protected]> > --- > 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 <[email protected]>
signature.asc
Description: Digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
