Quoting Andi Shyti (2020-07-02 23:09:44)
> Hi Chris,
>
> > @@ -1312,11 +1314,11 @@ static int set_ppgtt(struct drm_i915_file_private
> > *file_priv,
> > if (vm == rcu_access_pointer(ctx->vm))
> > goto unlock;
> >
> > + old = __set_ppgtt(ctx, vm);
> > +
> > /* Teardown the existing obj:vma cache, it will have to be rebuilt. */
> > lut_close(ctx);
> >
> > - old = __set_ppgtt(ctx, vm);
> > -
> > /*
> > * We need to flush any requests using the current ppgtt before
> > * we release it as the requests do not hold a reference themselves,
> > @@ -1330,6 +1332,7 @@ static int set_ppgtt(struct drm_i915_file_private
> > *file_priv,
> > if (err) {
> > i915_vm_close(__set_ppgtt(ctx, old));
> > i915_vm_close(old);
> > + lut_close(ctx); /* rebuild the old obj:vma cache */
>
> I don't really understand this but it doesn't hurt
Yeah; another testcase required for racing set-vm against execbuf.
Outcome unknown, all that we have to avoid are explosions. Userspace is
allowed to shoot itself in the foot, but is not allowed to shoot anyone
else.
> > diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> > b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> > index aa0d06cf1903..51b5a3421b40 100644
> > --- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> > +++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> > @@ -23,6 +23,8 @@ mock_context(struct drm_i915_private *i915,
> > INIT_LIST_HEAD(&ctx->link);
> > ctx->i915 = i915;
> >
> > + mutex_init(&ctx->mutex);
> > +
> > spin_lock_init(&ctx->stale.lock);
> > INIT_LIST_HEAD(&ctx->stale.engines);
> >
> > @@ -35,7 +37,7 @@ mock_context(struct drm_i915_private *i915,
> > RCU_INIT_POINTER(ctx->engines, e);
> >
> > INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
> > - mutex_init(&ctx->mutex);
> > + mutex_init(&ctx->lut_mutex);
>
> ...and I don't really understand why moved the first
> init(&ctx->mutex) above, is it just aesthetic?
Yup. The ctx->mutex is the broader one, so I felt it deserved to be
higher. Whereas here we are setting up the lut [handles_vma] so was the
natural spot to place the ctx->lut_mutex; and I wanted some distance
between the pair to keep the confusion at bay.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
