drm/i915: Keep gem ctx->vm alive until the final put

The comment added in

    commit b81dde719439c8f09bb61e742ed95bfc4b33946b
    Author: Chris Wilson <[email protected]>
    Date:   Tue May 21 22:11:29 2019 +0100

        drm/i915: Allow userspace to clone contexts on creation

and moved in

    commit 27dbae8f36c1c25008b7885fc07c57054b7dfba3
    Author: Chris Wilson <[email protected]>
    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 2850748ef8763ab46958e43a4d1c445f29eeb37d
Author: Chris Wilson <[email protected]>
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 b32fa811156328aea5a3c2ff05cc096490382456
    Author: Chris Wilson <[email protected]>
    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: Maarten Lankhorst <[email protected]>
Cc: Jason Ekstrand <[email protected]>
Cc: Chris Wilson <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Matthew Brost <[email protected]>
Cc: Matthew Auld <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: "Thomas Hellström" <[email protected]>
Cc: Lionel Landwerlin <[email protected]>
Cc: Dave Airlie <[email protected]>
Fixes: 2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex")
Signed-off-by: Daniel Vetter <[email protected]>
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]

0 files changed, 0 insertions, 0 deletions