• Daniel Vetter's avatar
    drm/i915: Keep gem ctx->vm alive until the final put · 8cf97637
    Daniel Vetter authored
    The comment added in
    
        commit b81dde71
        Author: Chris Wilson <chris@chris-wilson.co.uk>
        Date:   Tue May 21 22:11:29 2019 +0100
    
            drm/i915: Allow userspace to clone contexts on creation
    
    and moved in
    
        commit 27dbae8f
        Author: Chris Wilson <chris@chris-wilson.co.uk>
        Date:   Wed Nov 6 09:13:12 2019 +0000
    
            drm/i915/gem: Safely acquire the ctx->vm when copying
    
    suggested that i915_address_space were at least intended to be managed
    through SLAB_TYPESAFE_BY_RCU:
    
                    * This ppgtt may have be reallocated between
                    * the read and the kref, and reassigned to a third
                    * context. In order to avoid inadvertent sharing
                    * of this ppgtt with that third context (and not
                    * src), we have to confirm that we have the same
                    * ppgtt after passing through the strong memory
                    * barrier implied by a successful
                    * kref_get_unless_zero().
    
    But extensive git history search has not brough any such reuse to
    light.
    
    What has come to light though is that ever since
    
    commit 2850748e
    Author: Chris Wilson <chris@chris-wilson.co.uk>
    Date:   Fri Oct 4 14:39:58 2019 +0100
    
        drm/i915: Pull i915_vma_pin under the vm->mutex
    
    (yes this commit is earlier) the final i915_vma_put call has been
    moved from i915_gem_context_free (now called _release) to
    context_close, which means it's not actually safe anymore to access
    the ctx->vm pointer without lock helds, because it might disappear at
    any moment. Note that superficially things all still work, because the
    i915_address_space is RCU protected since
    
        commit b32fa811
        Author: Chris Wilson <chris@chris-wilson.co.uk>
        Date:   Thu Jun 20 19:37:05 2019 +0100
    
            drm/i915/gtt: Defer address space cleanup to an RCU worker
    
    except the very clever macro above (which is designed to protected
    against object reuse due to SLAB_TYPESAFE_BY_RCU or similar tricks)
    results in an endless loop if the refcount of the ctx->vm ever
    permanently drops to 0. Which it totally now can.
    
    Fix that by moving the final i915_vm_put to where it should be.
    
    Note that i915_gem_context is rcu protected, but _only_ the final
    kfree. This means anyone who chases a pointer to a gem ctx solely
    under the protection can pretty only call kref_get_unless_zero(). This
    seems to be pretty much the case, aside from a bunch of cases that
    consult the scheduling information without any further protection.
    Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
    Cc: Jason Ekstrand <jason@jlekstrand.net>
    Cc: Chris Wilson <chris@chris-wilson.co.uk>
    Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
    Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
    Cc: Matthew Brost <matthew.brost@intel.com>
    Cc: Matthew Auld <matthew.auld@intel.com>
    Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
    Cc: "Thomas Hellström" <thomas.hellstrom@intel.com>
    Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
    Cc: Dave Airlie <airlied@redhat.com>
    Fixes: 2850748e ("drm/i915: Pull i915_vma_pin under the vm->mutex")
    Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
    Link: https://patchwork.freedesktop.org/patch/msgid/20210902142057.929669-3-daniel.vetter@ffwll.ch
    8cf97637
i915_gem_context.c 55.7 KB