On Tue, Dec 3, 2013 at 7:26 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Mon, Dec 02, 2013 at 05:36:17PM -0800, Kristian Høgsberg wrote:
>> There's no reason to keep a reference to objects in the name idr. Each
>> handle to an object has a reference to the object and just before we
>> destroy the last handle we take the object out of the name idr. Thus,
>> if an object is in the name idr, there's at least one reference to the
>> object.
>>
>> Or to put it another way, the name idr reference will never keep the
>> object alive. It just looks like it, which is confusing.
>>
>> Signed-off-by: Kristian Høgsberg <krh@xxxxxxxxxxxxx>
>
> I expect this to blow up when you race gem_close ioctl calls with flink
> open. i-g-t/gem_flink_close tests actually have been written specifically
> to exercise these races.
> -Daniel
Can you be more specific about what race you see? The one thing that
could go wrong is that the last handle is delete after we enter
drm_gem_flink_ioctl() and look up the object but before taking the
object_name_lock, but that's handled by checking obj->handle_count
under the lock. Deleting handles and removing the name is always done
under the object_name_lock from
drm_gem_object_handle_unreference_unlocked().
Kristian
>> ---
>> drivers/gpu/drm/drm_gem.c | 15 ---------------
>> 1 file changed, 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>> index 4761ade..3ff39bb 100644
>> --- a/drivers/gpu/drm/drm_gem.c
>> +++ b/drivers/gpu/drm/drm_gem.c
>> @@ -175,11 +175,6 @@ drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp)
>> mutex_unlock(&filp->prime.lock);
>> }
>>
>> -static void drm_gem_object_ref_bug(struct kref *list_kref)
>> -{
>> - BUG();
>> -}
>> -
>> /**
>> * Called after the last handle to the object has been closed
>> *
>> @@ -195,13 +190,6 @@ static void drm_gem_object_handle_free(struct drm_gem_object *obj)
>> if (obj->name) {
>> idr_remove(&dev->object_name_idr, obj->name);
>> obj->name = 0;
>> - /*
>> - * The object name held a reference to this object, drop
>> - * that now.
>> - *
>> - * This cannot be the last reference, since the handle holds one too.
>> - */
>> - kref_put(&obj->refcount, drm_gem_object_ref_bug);
>> }
>> }
>>
>> @@ -602,9 +590,6 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
>> goto err;
>>
>> obj->name = ret;
>> -
>> - /* Allocate a reference for the name table. */
>> - drm_gem_object_reference(obj);
>> }
>>
>> args->name = (uint64_t) obj->name;
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
