Re: [Pv-drivers] [PATCH 01/12] VMCI: context implementation.

[Dmitry Torokhov]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/