Commit e1a7ab4f authored by Thomas Hellström's avatar Thomas Hellström
Browse files

drm/i915: Remove the vm open count 



vms are not getting properly closed. Rather than fixing that,
Remove the vm open count and instead rely on the vm refcount.

The vm open count existed solely to break the strong references the
vmas had on the vms. Now instead make those references weak and
ensure vmas are destroyed when the vm is destroyed.

Unfortunately if the vm destructor and the object destructor both
wants to destroy a vma, that may lead to a race in that the vm
destructor just unbinds the vma and leaves the actual vma destruction
to the object destructor. However in order for the object destructor
to ensure the vma is unbound it needs to grab the vm mutex. In order
to keep the vm mutex alive until the object destructor is done with
it, somewhat hackishly grab a vm_resv refcount that is released late
in the vma destruction process, when the vm mutex is no longer needed.

v2: Address review-comments from Niranjana
- Clarify that the struct i915_address_space::skip_pte_rewrite is a hack
  and should ideally be replaced in an upcoming patch.
- Remove an unneeded continue in clear_vm_list and update comment.

v3:
- Documentation update
- Commit message formatting

Co-developed-by: default avatarNiranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Signed-off-by: default avatarNiranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Signed-off-by: default avatarThomas Hellström <thomas.hellstrom@linux.intel.com>
Reviewed-by: default avatarNiranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
Reviewed-by: default avatarMatthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220304082641.308069-2-thomas.hellstrom@linux.intel.com
parent d028a769

drivers/gpu/drm/i915/display/intel_dpt.c

+1 −1
Original line number Diff line number Diff line
@@ -300,5 +300,5 @@ void intel_dpt_destroy(struct i915_address_space *vm) 
{

	struct i915_dpt *dpt = i915_vm_to_dpt(vm);



	i915_vm_close(&dpt->vm);

	i915_vm_put(&dpt->vm);

}

drivers/gpu/drm/i915/gem/i915_gem_context.c

+6 −23
Original line number Diff line number Diff line
@@ -1466,8 +1466,6 @@ static void set_closed_name(struct i915_gem_context *ctx) 


static void context_close(struct i915_gem_context *ctx)

{

	struct i915_address_space *vm;



	/* Flush any concurrent set_engines() */

	mutex_lock(&ctx->engines_mutex);

	unpin_engines(__context_engines_static(ctx));
@@ -1479,19 +1477,6 @@ static void context_close(struct i915_gem_context *ctx) 


	set_closed_name(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

		 * operations racing with thist ctx close. Keep a full reference

		 * until the end.

		 */

		i915_vm_get(vm);

		i915_vm_close(vm);

	}



	ctx->file_priv = ERR_PTR(-EBADF);



	/*

	 * The LUT uses the VMA as a backpointer to unref the object,

	 * so we need to clear the LUT before we close all the VMA (inside
@@ -1499,6 +1484,8 @@ static void context_close(struct i915_gem_context *ctx) 
	 */

	lut_close(ctx);



	ctx->file_priv = ERR_PTR(-EBADF);



	spin_lock(&ctx->i915->gem.contexts.lock);

	list_del(&ctx->link);

	spin_unlock(&ctx->i915->gem.contexts.lock);
@@ -1597,12 +1584,8 @@ i915_gem_create_context(struct drm_i915_private *i915, 
		}

		vm = &ppgtt->vm;

	}

	if (vm) {

		ctx->vm = i915_vm_open(vm);



		/* i915_vm_open() takes a reference */

		i915_vm_put(vm);

	}

	if (vm)

		ctx->vm = vm;



	mutex_init(&ctx->engines_mutex);

	if (pc->num_user_engines >= 0) {
@@ -1652,7 +1635,7 @@ i915_gem_create_context(struct drm_i915_private *i915, 
	free_engines(e);

err_vm:

	if (ctx->vm)

		i915_vm_close(ctx->vm);

		i915_vm_put(ctx->vm);

err_ctx:

	kfree(ctx);

	return ERR_PTR(err);
@@ -1836,7 +1819,7 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv, 
	if (err)

		return err;



	i915_vm_open(vm);

	i915_vm_get(vm);



	GEM_BUG_ON(id == 0); /* reserved for invalid/unassigned ppgtt */

	args->value = id;

drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c

+6 −0
Original line number Diff line number Diff line
@@ -2691,6 +2691,11 @@ eb_select_engine(struct i915_execbuffer *eb) 
	if (err)

		goto err;



	if (!i915_vm_tryget(ce->vm)) {

		err = -ENOENT;

		goto err;

	}



	eb->context = ce;

	eb->gt = ce->engine->gt;
@@ -2714,6 +2719,7 @@ eb_put_engine(struct i915_execbuffer *eb) 
{

	struct intel_context *child;



	i915_vm_put(eb->context->vm);

	intel_gt_pm_put(eb->gt);

	for_each_child(eb->context, child)

		intel_context_put(child);

drivers/gpu/drm/i915/gem/selftests/mock_context.c

+2 −3
Original line number Diff line number Diff line
@@ -42,8 +42,7 @@ mock_context(struct drm_i915_private *i915, 
		if (!ppgtt)

			goto err_free;



		ctx->vm = i915_vm_open(&ppgtt->vm);

		i915_vm_put(&ppgtt->vm);

		ctx->vm = &ppgtt->vm;

	}



	mutex_init(&ctx->engines_mutex);
@@ -59,7 +58,7 @@ mock_context(struct drm_i915_private *i915, 


err_vm:

	if (ctx->vm)

		i915_vm_close(ctx->vm);

		i915_vm_put(ctx->vm);

err_free:

	kfree(ctx);

	return NULL;

drivers/gpu/drm/i915/gt/gen6_ppgtt.c

+1 −1
Original line number Diff line number Diff line
@@ -322,7 +322,7 @@ int gen6_ppgtt_pin(struct i915_ppgtt *base, struct i915_gem_ww_ctx *ww) 
	struct gen6_ppgtt *ppgtt = to_gen6_ppgtt(base);

	int err;



	GEM_BUG_ON(!atomic_read(&ppgtt->base.vm.open));

	GEM_BUG_ON(!kref_read(&ppgtt->base.vm.ref));



	/*

	 * Workaround the limited maximum vma->pin_count and the aliasing_ppgtt