[PATCH 17/19] drm/radeon: use rcu waits in some ioctls

Maarten Lankhorst maarten.lankhorst at canonical.com
Fri Aug 1 10:07:55 PDT 2014



On 01-08-14 16:13, Michel Dänzer wrote:
> On 01.08.2014 19:12, Maarten Lankhorst wrote:
>> Hey,
>>
>> On 01-08-14 10:27, Michel Dänzer wrote:
>>> On 01.08.2014 00:34, Maarten Lankhorst wrote:
>>>>
>>>> @@ -357,14 +360,20 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
>>>>  	struct drm_radeon_gem_wait_idle *args = data;
>>>>  	struct drm_gem_object *gobj;
>>>>  	struct radeon_bo *robj;
>>>> -	int r;
>>>> +	int r = 0;
>>>> +	long ret;
>>>>  
>>>>  	gobj = drm_gem_object_lookup(dev, filp, args->handle);
>>>>  	if (gobj == NULL) {
>>>>  		return -ENOENT;
>>>>  	}
>>>>  	robj = gem_to_radeon_bo(gobj);
>>>> -	r = radeon_bo_wait(robj, NULL, false);
>>>> +	ret = reservation_object_wait_timeout_rcu(robj->tbo.resv, true, true, 30 * HZ);
>>>> +	if (ret == 0)
>>>> +		r = -EBUSY;
>>>> +	else if (ret < 0)
>>>> +		r = ret;
>>>> +
>>>>  	/* callback hw specific functions if any */
>>>>  	if (rdev->asic->ioctl_wait_idle)
>>>>  		robj->rdev->asic->ioctl_wait_idle(rdev, robj);
>>>
>>> Heads up, this conflicts with
>>> http://lists.freedesktop.org/archives/dri-devel/2014-August/065255.html
>>> which passes a non-NULL second argument to radeon_bo_wait() to get the
>>> BO's current domain.
>> Ok, I will fix it up and resend it later.
>>
>> Does it matter if I grab the current domain without grabbing the lock
>> here? Because it doesn't matter if it sees the old or new domain, it
>> could have been changed after returning too.
> 
> It should be the domain where the BO is located when the fence we are
> waiting for here signals.
Could we compare domain before and after the rcu wait, and retry waiting if they're different, and the new one is VRAM? (eg eviction happened) That should prevent needing to lock the bo.

~Maarten


More information about the dri-devel mailing list