Re: [PATCH] drm: Replace kref with a simple atomic reference count

[Thomas Hellstrom <thomas@shipmail.org>]

On 11/25/2010 11:38 PM, Dave Airlie wrote:
> On Thu, 2010-11-25 at 21:40 +0000, Chris Wilson wrote:
>    
>> For a deferred-free cache of unreferenced bound objects, a simple
>> reference count is required without the baggage of kref.
>>      
> eh?
>
> you've just out of lined kref for no real gain.
>
> the whole point of kref is that its standard and doesn't require
> auditing for the people not inlining it.
>
> The only place I can see the advantage is not taking the struct mutex in
> the free path and really we should just fix the free function to take
> the struct mutex if required and wrap the real free.
>
>    
This last thing is really a generic problem with objects looked up from 
user-space. Consider the lookup path:

read_lock()
lookup_object_name();
kref_get();
read_unlock();

And the destroy path:

vs the desired put path
if (reference_put()) {
write_lock():
remove_name();
write_unlock();
destroy();
}

This is racy, in that the kref_get() can hit a zero refcount. I think an 
ideal thing here would be to add a kref_get_unless_zero() for this 
situation, that returns an error if the refcount was indeed zero. I 
don't think that would violate the kref idea, since we still never 
increase the refcount of a zeroed object, and the user needs to provide 
the necessary synchronization to make sure the object isn't gone while 
still the refcount is zero.

/Thomas






> Dave.
>
>