Hi Greg,
On Mon, Oct 29, 2012 at 07:10:58PM -0700, Greg KH wrote:
> On Mon, Oct 29, 2012 at 06:03:42PM -0700, George Zhang wrote:
> > +/*
> > + * Releases the VMCI context. If this is the last reference to
> > + * the context it will be deallocated. A context is created with
> > + * a reference count of one, and on destroy, it is removed from
> > + * the context list before its reference count is
> > + * decremented. Thus, if we reach zero, we are sure that nobody
> > + * else are about to increment it (they need the entry in the
> > + * context list for that). This function musn't be called with a
> > + * lock held.
> > + */
> > +void vmci_ctx_release(struct vmci_ctx *context)
> > +{
> > + ASSERT(context);
> > + kref_put(&context->kref, ctx_free_ctx);
> > +}
> > +
>
> Hm, are you _sure_ you should be calling this without a lock held?
> That's usually kref-101, you MUST hold a lock when calling put,
> otherwise you can race a kref_get() call, and all hell can break loose.
>
> Because of this, some saner people (like Al Viro), have suggested that I
> force the kref_put() and kref_get() calls pass in a spinlock just to
> enforce this.
>
> So, tell me what I'm missing here, and why you put the comment here
> saying that it really is supposed to be called without a lock held? How
> is that safe?
>
Contexts are created/registered in vmci_ctx_init_ctx() and unregistered in
vmci_ctx_release_ctx() and these operations are protected by
ctx_list.lock spinlock. Context lookup (vmci_ctx_get) also uses spinlock
to traverse list of registered contexts and then grabs reference to the
[valid] context. The use of kref_put() without additional locking in
vmci_ctx_release() is fine as there is no chance of another thread
bumping count from 0 to 1.
I believe the comment should actually read that the function should not
be called from atomic contexts.
Thanks,
Dmitry
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
