drm/i915: Early rejection of mappable GGTT pin attempts for large bo

Currently, we reject attempting to pin a large bo into the mappable
aperture, but only after trying to create the vma. Under debug kernels,
repeatedly creating and freeing that vma for an oversized bo consumes
one-third of the runtime for pwrite/pread tests as it is spent on
kmalloc/kfree tracking. If we move the rejection to before creating that
vma, we lose some accuracy of checking against the fence_size as opposed
to object size, though the fence can never be smaller than the object.
Note that the vma creation itself will reject an attempt to create a vma
larger than the GTT so we can remove one redundant test.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20171009084401.29090-7-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6f5c0d6da06afde97825097b3e932f3d3c6af15e..b43fae4b83e6f7994a5a233f7219d2dad9271558 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4036,42 +4036,47 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
 
        lockdep_assert_held(&obj->base.dev->struct_mutex);
 
+       if (!view && flags & PIN_MAPPABLE) {
+               /* If the required space is larger than the available
+                * aperture, we will not able to find a slot for the
+                * object and unbinding the object now will be in
+                * vain. Worse, doing so may cause us to ping-pong
+                * the object in and out of the Global GTT and
+                * waste a lot of cycles under the mutex.
+                */
+               if (obj->base.size > dev_priv->ggtt.mappable_end)
+                       return ERR_PTR(-E2BIG);
+
+               /* If NONBLOCK is set the caller is optimistically
+                * trying to cache the full object within the mappable
+                * aperture, and *must* have a fallback in place for
+                * situations where we cannot bind the object. We
+                * can be a little more lax here and use the fallback
+                * more often to avoid costly migrations of ourselves
+                * and other objects within the aperture.
+                *
+                * Half-the-aperture is used as a simple heuristic.
+                * More interesting would to do search for a free
+                * block prior to making the commitment to unbind.
+                * That caters for the self-harm case, and with a
+                * little more heuristics (e.g. NOFAULT, NOEVICT)
+                * we could try to minimise harm to others.
+                */
+               if (flags & PIN_NONBLOCK &&
+                   obj->base.size > dev_priv->ggtt.mappable_end / 2)
+                       return ERR_PTR(-ENOSPC);
+       }
+
        vma = i915_vma_instance(obj, vm, view);
        if (unlikely(IS_ERR(vma)))
                return vma;
 
        if (i915_vma_misplaced(vma, size, alignment, flags)) {
-               if (flags & PIN_NONBLOCK &&
-                   (i915_vma_is_pinned(vma) || i915_vma_is_active(vma)))
-                       return ERR_PTR(-ENOSPC);
+               if (flags & PIN_NONBLOCK) {
+                       if (i915_vma_is_pinned(vma) || i915_vma_is_active(vma))
+                               return ERR_PTR(-ENOSPC);
 
-               if (flags & PIN_MAPPABLE) {
-                       /* If the required space is larger than the available
-                        * aperture, we will not able to find a slot for the
-                        * object and unbinding the object now will be in
-                        * vain. Worse, doing so may cause us to ping-pong
-                        * the object in and out of the Global GTT and
-                        * waste a lot of cycles under the mutex.
-                        */
-                       if (vma->fence_size > dev_priv->ggtt.mappable_end)
-                               return ERR_PTR(-E2BIG);
-
-                       /* If NONBLOCK is set the caller is optimistically
-                        * trying to cache the full object within the mappable
-                        * aperture, and *must* have a fallback in place for
-                        * situations where we cannot bind the object. We
-                        * can be a little more lax here and use the fallback
-                        * more often to avoid costly migrations of ourselves
-                        * and other objects within the aperture.
-                        *
-                        * Half-the-aperture is used as a simple heuristic.
-                        * More interesting would to do search for a free
-                        * block prior to making the commitment to unbind.
-                        * That caters for the self-harm case, and with a
-                        * little more heuristics (e.g. NOFAULT, NOEVICT)
-                        * we could try to minimise harm to others.
-                        */
-                       if (flags & PIN_NONBLOCK &&
+                       if (flags & PIN_MAPPABLE &&
                            vma->fence_size > dev_priv->ggtt.mappable_end / 2)
                                return ERR_PTR(-ENOSPC);
                }