It's been invariant since
commit
ccbc1b97948ab671335e950271e39766729736c3
Author: Jason Ekstrand <jason@jlekstrand.net>
Date: Thu Jul 8 10:48:30 2021 -0500
drm/i915/gem: Don't allow changing the VM on running contexts (v4)
this just completes the deed. I've tried to split out prep work for
more careful review as much as possible, this is what's left:
- get_ppgtt gets simplified since we don't need to grab a temporary
reference - we can rely on the temporary reference for the gem_ctx
while we inspect the vm. The new vm_id still needs a full
i915_vm_open ofc. This also removes the final caller of context_get_vm_rcu
- A pile of selftests can now just look at ctx->vm instead of
rcu_dereference_protected( , true) or similar things.
- All callers of i915_gem_context_vm also disappear.
- I've changed the hugepage selftest to set scrub_64K without any
locking, because when we inspect that setting we're also not taking
any locks either. It works because it's a selftests that's careful
(single threaded gives you nice ordering) and not a live driver
where races can happen from anywhere.
These can only be split up further if we have some intermediate state
with a bunch more rcu_dereference_protected(ctx->vm, true), just to
shut up lockdep and sparse.
The conversion to __rcu happened in
commit
a4e7ccdac38ec8335d9e4e2656c1a041c77feae1
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date: Fri Oct 4 14:40:09 2019 +0100
drm/i915: Move context management under GEM
Note that we're not breaking the actual bugfix in there: The real
bugfix is pushing the i915_vm_relase onto a separate worker, to avoid
locking inversion issues. The rcu conversion was just thrown in for
entertainment value on top (no vm lookup isn't even close to anything
that's a hotpath where removing the single spinlock can be measured).
v2: Rebase over the change to move the i915_vm_put() into
i915_gem_context_release().
v3: Trivial conflict against repainted shed.
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20210902142057.929669-9-daniel.vetter@ffwll.ch
return ret;
}
-static struct i915_address_space *
-context_get_vm_rcu(struct i915_gem_context *ctx)
-{
- GEM_BUG_ON(!rcu_access_pointer(ctx->vm));
-
- do {
- struct i915_address_space *vm;
-
- /*
- * We do not allow downgrading from full-ppgtt [to a shared
- * global gtt], so ctx->vm cannot become NULL.
- */
- vm = rcu_dereference(ctx->vm);
- if (!kref_get_unless_zero(&vm->ref))
- continue;
-
- /*
- * 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().
- *
- * Once we have acquired the current ppgtt of ctx,
- * we no longer care if it is released from ctx, as
- * it cannot be reallocated elsewhere.
- */
-
- if (vm == rcu_access_pointer(ctx->vm))
- return rcu_pointer_handoff(vm);
-
- i915_vm_put(vm);
- } while (1);
-}
-
static int intel_context_set_gem(struct intel_context *ce,
struct i915_gem_context *ctx,
struct intel_sseu sseu)
if (ctx->syncobj)
drm_syncobj_put(ctx->syncobj);
- vm = i915_gem_context_vm(ctx);
+ vm = ctx->vm;
if (vm)
i915_vm_put(vm);
set_closed_name(ctx);
- vm = i915_gem_context_vm(ctx);
+ vm = ctx->vm;
if (vm) {
/* i915_vm_close drops the final reference, which is a bit too
* early and could result in surprises with concurrent
vm = &ppgtt->vm;
}
if (vm) {
- RCU_INIT_POINTER(ctx->vm, i915_vm_open(vm));
+ ctx->vm = i915_vm_open(vm);
/* i915_vm_open() takes a reference */
i915_vm_put(vm);
if (!i915_gem_context_has_full_ppgtt(ctx))
return -ENODEV;
- rcu_read_lock();
- vm = context_get_vm_rcu(ctx);
- rcu_read_unlock();
- if (!vm)
- return -ENODEV;
+ vm = ctx->vm;
+ GEM_BUG_ON(!vm);
err = xa_alloc(&file_priv->vm_xa, &id, vm, xa_limit_32b, GFP_KERNEL);
if (err)
- goto err_put;
+ return err;
i915_vm_open(vm);
args->value = id;
args->size = 0;
-err_put:
- i915_vm_put(vm);
return err;
}
static inline bool i915_gem_context_has_full_ppgtt(struct i915_gem_context *ctx)
{
- GEM_BUG_ON(!!rcu_access_pointer(ctx->vm) != HAS_FULL_PPGTT(ctx->i915));
+ GEM_BUG_ON(!!ctx->vm != HAS_FULL_PPGTT(ctx->i915));
- return !!rcu_access_pointer(ctx->vm);
+ return !!ctx->vm;
}
static inline struct i915_address_space *
{
struct i915_address_space *vm;
- rcu_read_lock();
- vm = rcu_dereference(ctx->vm);
+ vm = ctx->vm;
if (!vm)
vm = &ctx->i915->ggtt.vm;
vm = i915_vm_get(vm);
- rcu_read_unlock();
return vm;
}
* In other modes, this is a NULL pointer with the expectation that
* the caller uses the shared global GTT.
*/
- struct i915_address_space __rcu *vm;
+ struct i915_address_space *vm;
/**
* @pid: process id of creator
goto out_file;
}
- mutex_lock(&ctx->mutex);
- vm = i915_gem_context_vm(ctx);
+ vm = ctx->vm;
if (vm)
WRITE_ONCE(vm->scrub_64K, true);
- mutex_unlock(&ctx->mutex);
err = i915_subtests(tests, ctx);
#define DW_PER_PAGE (PAGE_SIZE / sizeof(u32))
-static inline struct i915_address_space *ctx_vm(struct i915_gem_context *ctx)
-{
- /* single threaded, private ctx */
- return rcu_dereference_protected(ctx->vm, true);
-}
-
static int live_nop_switch(void *arg)
{
const unsigned int nctx = 1024;
struct i915_gem_context *ctx;
struct intel_context *ce;
- ctx = kernel_context(i915, ctx_vm(parent));
+ ctx = kernel_context(i915, parent->vm);
if (IS_ERR(ctx)) {
err = PTR_ERR(ctx);
goto out_test;
GEM_BUG_ON(IS_ERR(ce));
if (!obj) {
- obj = create_test_object(ctx_vm(parent),
+ obj = create_test_object(parent->vm,
file, &objects);
if (IS_ERR(obj)) {
err = PTR_ERR(obj);
goto out_file;
}
- vm = ctx_vm(ctx) ?: &i915->ggtt.alias->vm;
+ vm = ctx->vm ?: &i915->ggtt.alias->vm;
if (!vm || !vm->has_read_only) {
err = 0;
goto out_file;
GEM_BUG_ON(offset < I915_GTT_PAGE_SIZE);
- err = check_scratch(ctx_vm(ctx), offset);
+ err = check_scratch(ctx->vm, offset);
if (err)
return err;
GEM_BUG_ON(offset < I915_GTT_PAGE_SIZE);
- err = check_scratch(ctx_vm(ctx), offset);
+ err = check_scratch(ctx->vm, offset);
if (err)
return err;
u32 *vaddr;
int err = 0;
- vm = ctx_vm(ctx);
+ vm = ctx->vm;
if (!vm)
return -ENODEV;
}
/* We can only test vm isolation, if the vm are distinct */
- if (ctx_vm(ctx_a) == ctx_vm(ctx_b))
+ if (ctx_a->vm == ctx_b->vm)
goto out_file;
/* Read the initial state of the scratch page */
if (err)
goto out_file;
- vm_total = ctx_vm(ctx_a)->total;
- GEM_BUG_ON(ctx_vm(ctx_b)->total != vm_total);
+ vm_total = ctx_a->vm->total;
+ GEM_BUG_ON(ctx_b->vm->total != vm_total);
count = 0;
num_engines = 0;
TP_fast_assign(
__entry->dev = ctx->i915->drm.primary->index;
__entry->ctx = ctx;
- __entry->vm = rcu_access_pointer(ctx->vm);
+ __entry->vm = ctx->vm;
),
TP_printk("dev=%u, ctx=%p, ctx_vm=%p",
{
bool ok = true;
- if (vma->vm != rcu_access_pointer(ctx->vm)) {
+ if (vma->vm != ctx->vm) {
pr_err("VMA created with wrong VM\n");
ok = false;
}
projects / kernel.git / commitdiff