[Intel-gfx] [PATCH] drm/i915: Do an optimistic is-busy? check first

[Daniel Vetter]

On Thu, Sep 05, 2013 at 03:29:01PM +0100, Chris Wilson wrote:
> The busy-ioctl is frequently called as it is used by clients treating
> buffer objects like fences and polling for completion. These frequent
> calls are especially subject to contention with multiple clients leading
> to a lot of busy-waiting due to mutex_spin_on_owner()). We can, in most
> cases, report whether the bo is idle without touching struct_mutex, and
> only resort to hitting the lock if we need to queue flushes for the bo.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3b942ce..fed0b39 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4068,18 +4068,34 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>  {
>  	struct drm_i915_gem_busy *args = data;
>  	struct drm_i915_gem_object *obj;
> +	struct intel_ring_buffer *ring;
>  	int ret;
>  
> -	ret = i915_mutex_lock_interruptible(dev);
> -	if (ret)
> -		return ret;
> -
>  	obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle));
>  	if (&obj->base == NULL) {
>  		ret = -ENOENT;
> -		goto unlock;
> +		goto out_unlocked;
>  	}
>  
> +	/* Do an optimistic check for activity - we don't care about userspace
> +	 * racing with itself, that is always problematic.
> +	 */
> +	ring = obj->ring;
> +	if (ring && obj->last_read_seqno == ring->outstanding_lazy_seqno)
> +		goto lock_and_flush;

Feels a bit too tricky ... How useful is just an

	if (ACCESS_ONCE(obj->active))
		goto lock_and_flush;

Cheers, Daniel

> +
> +	args->busy = ring ? intel_ring_flag(ring) << 16 | 1 : 0;
> +	ret = 0;
> +
> +out_unlocked:
> +	drm_gem_object_unreference_unlocked(&obj->base);
> +	return ret;
> +
> +lock_and_flush:
> +	ret = i915_mutex_lock_interruptible(dev);
> +	if (ret)
> +		goto out_unlocked;
> +
>  	/* Count all active objects as busy, even if they are currently not used
>  	 * by the gpu. Users of this interface expect objects to eventually
>  	 * become non-busy without any further actions, therefore emit any
> @@ -4094,7 +4110,6 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>  	}
>  
>  	drm_gem_object_unreference(&obj->base);
> -unlock:
>  	mutex_unlock(&dev->struct_mutex);
>  	return ret;
>  }
> -- 
> 1.8.4.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch