On Wed, Jul 13, 2005 at 10:49:54AM +0100, David Woodhouse wrote:
On Wed, 2005-07-13 at 09:40 +0100, David Woodhouse wrote:
> And _especially_ don't use it for places where the refcount had other
> locking and didn't even need to be atomic.
Actually, Tim, I'll revise that.
The kref stuff seems to be an attempt to provide 'refcounting for
dummies'. However, rather than abstracting the refcounting and making it
safe and easy, it merely hides the potential problems. If you call
kref_put() and kref_get() simultaneously for the same kref while its
count is 1, you can end up calling the 'release()' function from
kref_put() while the kref_get() increments the count back to 1 again.
Yes, and that is well documented.
It's also what 99% of the users of an atomic_t want to use, including
this implementation of it. So, in this case, it is a drop-in
replacement for your reference count variable. I see no such lock being
used in this patch.
To avoid that situation, you still actually need to do your own
locking
of some kind when you use a kref -- even though it might try to tempt
you not to. If you don't, you'll be left with subtle races.
Again, documented as such.
Which means that the 'refcounting for dummies' isn't
quite as stuffed
with transparent goodness as it might seem. And it's far better to
open-code it and actually see what's going on.
Not at all. Please use a kref if you want to do reference counting
logic. If you need to protect yourself from having this kind of locking
issues, then use a lock. You would have to do that anyway.
thanks,
greg k-h