Messages in this thread | | | Date | Mon, 30 Jan 2006 20:58:30 +0100 | From | Eric Dumazet <> | Subject | Re: [PATCH 1/5] pid: Implement task references. |
| |
Greg KH a écrit : > On Mon, Jan 30, 2006 at 06:19:35AM +0100, Eric Dumazet wrote: >> Example of improvement in kref_put() : >> >> [PATCH] kref : Avoid an atomic operation in kref_put() when the last >> reference is dropped. On most platforms, atomic_read() is a plan read of >> the counter and involves no atomic at all. > > No, we wat to decrement and test at the same time, to protect against > any race where someone is incrementing right when we are dropping the > last reference.
Sorry ? Me confused !
What I am saying is :
If a CPUA is doing a kref_put() and refcount == 1, then another CPU *cannot* change the refcount, because only one CPU has a valid reference on the object. (CPUA )
Proof :
If CPUB cannot kref_put() as well because only CPUA owns a refcount. (If two CPUS could kref_put, then counter would become -1 !)
If CPUB is going to do a kref_get() : Not allowed by kref specs (Rule 3) (It would mean that both CPUA/B have a reference)
'The kref_get() does not require a lock, since we already have a valid pointer that we own a refcount for.'
So CPUA and CPUB cannot both own a refcount on a object having refcount=1
If you allow this, then current implementation is buggy as well.
kref->refcount == 1
CPUA :
if (atomic_dec_and_test(&kref->refcount)) { returns TRUE condition [refcount is now 0]
CPUB : atomic_inc(&kref->refcount) [refcount is now 1]
CPUA : release(kref); (object freed)
CPUB : doing some work on object freed by CPUA. *kaboom*
CPUB : kref_put() refcount back to 0 -> object freed twice *kaboom*
> > So, thanks, but I'm not going to apply this. > > greg k-h >
Me confused.
Yes, kref abstraction is good :) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |