Commit 50689771 authored by Chris Wilson's avatar Chris Wilson
Browse files

drm/i915: Only close vma we open 

The history of i915_vma_close() is confusing, as is its use. As the
lifetime of the i915_vma is currently bounded by the object it is
attached to, we needed a means of identify when a vma was no longer in
use by userspace (via the user's fd). This is further complicated by
that only ppgtt vma should be closed at the user's behest, as the ggtt
were always shared.

Now that we attach the vma to a lut on the user's context, the open
count does indicate how many unique and open context/vm are referencing
this vma from the user. As such, we can and should just use the
open_count to track when the vma is still in use by userspace.

It's a poor man's replacement for reference counting.

Closes: https://gitlab.freedesktop.org/drm/intel/issues/1193


Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarAndi Shyti <andi.shyti@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200422190558.30509-1-chris@chris-wilson.co.uk
parent b4892e44

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

+1 −3
Original line number Diff line number Diff line
@@ -130,8 +130,6 @@ static void lut_close(struct i915_gem_context *ctx) 
		if (&lut->obj_link != &obj->lut_list) {

			i915_lut_handle_free(lut);

			radix_tree_iter_delete(&ctx->handles_vma, &iter, slot);

			if (atomic_dec_and_test(&vma->open_count) &&

			    !i915_vma_is_ggtt(vma))

			i915_vma_close(vma);

			i915_gem_object_put(obj);

		}

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

+1 −1
Original line number Diff line number Diff line
@@ -830,7 +830,7 @@ static int __eb_add_lut(struct i915_execbuffer *eb, 
	return 0;



err:

	atomic_dec(&vma->open_count);

	i915_vma_close(vma);

	i915_vma_put(vma);

	i915_lut_handle_free(lut);

	return err;

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

+1 −3
Original line number Diff line number Diff line
@@ -135,8 +135,6 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file) 
		if (vma) {

			GEM_BUG_ON(vma->obj != obj);

			GEM_BUG_ON(!atomic_read(&vma->open_count));

			if (atomic_dec_and_test(&vma->open_count) &&

			    !i915_vma_is_ggtt(vma))

			i915_vma_close(vma);

		}

		mutex_unlock(&ctx->mutex);

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

+21 −62
Original line number Diff line number Diff line
@@ -421,7 +421,7 @@ static int igt_mock_exhaust_device_supported_pages(void *arg) 


			err = i915_vma_pin(vma, 0, 0, PIN_USER);

			if (err)

				goto out_close;

				goto out_put;



			err = igt_check_page_sizes(vma);
@@ -432,8 +432,6 @@ static int igt_mock_exhaust_device_supported_pages(void *arg) 
			}



			i915_vma_unpin(vma);

			i915_vma_close(vma);



			i915_gem_object_put(obj);



			if (err)
@@ -443,8 +441,6 @@ static int igt_mock_exhaust_device_supported_pages(void *arg) 


	goto out_device;



out_close:

	i915_vma_close(vma);

out_put:

	i915_gem_object_put(obj);

out_device:
@@ -492,7 +488,7 @@ static int igt_mock_memory_region_huge_pages(void *arg) 


			err = i915_vma_pin(vma, 0, 0, PIN_USER);

			if (err)

				goto out_close;

				goto out_put;



			err = igt_check_page_sizes(vma);

			if (err)
@@ -515,8 +511,6 @@ static int igt_mock_memory_region_huge_pages(void *arg) 
			}



			i915_vma_unpin(vma);

			i915_vma_close(vma);



			__i915_gem_object_put_pages(obj);

			i915_gem_object_put(obj);

		}
@@ -526,8 +520,6 @@ static int igt_mock_memory_region_huge_pages(void *arg) 


out_unpin:

	i915_vma_unpin(vma);

out_close:

	i915_vma_close(vma);

out_put:

	i915_gem_object_put(obj);

out_region:
@@ -587,10 +579,8 @@ static int igt_mock_ppgtt_misaligned_dma(void *arg) 
		}



		err = i915_vma_pin(vma, 0, 0, flags);

		if (err) {

			i915_vma_close(vma);

		if (err)

			goto out_unpin;

		}





		err = igt_check_page_sizes(vma);
@@ -603,10 +593,8 @@ static int igt_mock_ppgtt_misaligned_dma(void *arg) 


		i915_vma_unpin(vma);



		if (err) {

			i915_vma_close(vma);

		if (err)

			goto out_unpin;

		}



		/*

		 * Try all the other valid offsets until the next
@@ -615,16 +603,12 @@ static int igt_mock_ppgtt_misaligned_dma(void *arg) 
		 */

		for (offset = 4096; offset < page_size; offset += 4096) {

			err = i915_vma_unbind(vma);

			if (err) {

				i915_vma_close(vma);

			if (err)

				goto out_unpin;

			}



			err = i915_vma_pin(vma, 0, 0, flags | offset);

			if (err) {

				i915_vma_close(vma);

			if (err)

				goto out_unpin;

			}



			err = igt_check_page_sizes(vma);
@@ -636,10 +620,8 @@ static int igt_mock_ppgtt_misaligned_dma(void *arg) 


			i915_vma_unpin(vma);



			if (err) {

				i915_vma_close(vma);

			if (err)

				goto out_unpin;

			}



			if (igt_timeout(end_time,

					"%s timed out at offset %x with page-size %x\n",
@@ -647,8 +629,6 @@ static int igt_mock_ppgtt_misaligned_dma(void *arg) 
				break;

		}



		i915_vma_close(vma);



		i915_gem_object_unpin_pages(obj);

		__i915_gem_object_put_pages(obj);

		i915_gem_object_put(obj);
@@ -670,12 +650,6 @@ static void close_object_list(struct list_head *objects, 
	struct drm_i915_gem_object *obj, *on;



	list_for_each_entry_safe(obj, on, objects, st_link) {

		struct i915_vma *vma;



		vma = i915_vma_instance(obj, &ppgtt->vm, NULL);

		if (!IS_ERR(vma))

			i915_vma_close(vma);



		list_del(&obj->st_link);

		i915_gem_object_unpin_pages(obj);

		__i915_gem_object_put_pages(obj);
@@ -912,7 +886,7 @@ static int igt_mock_ppgtt_64K(void *arg) 


			err = i915_vma_pin(vma, 0, 0, flags);

			if (err)

				goto out_vma_close;

				goto out_object_unpin;



			err = igt_check_page_sizes(vma);

			if (err)
@@ -945,8 +919,6 @@ static int igt_mock_ppgtt_64K(void *arg) 
			}



			i915_vma_unpin(vma);

			i915_vma_close(vma);



			i915_gem_object_unpin_pages(obj);

			__i915_gem_object_put_pages(obj);

			i915_gem_object_put(obj);
@@ -957,8 +929,6 @@ static int igt_mock_ppgtt_64K(void *arg) 


out_vma_unpin:

	i915_vma_unpin(vma);

out_vma_close:

	i915_vma_close(vma);

out_object_unpin:

	i915_gem_object_unpin_pages(obj);

out_object_put:
@@ -1070,7 +1040,7 @@ static int __igt_write_huge(struct intel_context *ce, 


	err = i915_vma_unbind(vma);

	if (err)

		goto out_vma_close;

		return err;



	err = i915_vma_pin(vma, size, 0, flags | offset);

	if (err) {
@@ -1081,7 +1051,7 @@ static int __igt_write_huge(struct intel_context *ce, 
		if (err == -ENOSPC && i915_is_ggtt(ce->vm))

			err = 0;



		goto out_vma_close;

		return err;

	}



	err = igt_check_page_sizes(vma);
@@ -1102,8 +1072,6 @@ static int __igt_write_huge(struct intel_context *ce, 


out_vma_unpin:

	i915_vma_unpin(vma);

out_vma_close:

	__i915_vma_put(vma);

	return err;

}
@@ -1490,7 +1458,7 @@ static int igt_ppgtt_pin_update(void *arg) 


		err = i915_vma_pin(vma, SZ_2M, 0, flags);

		if (err)

			goto out_close;

			goto out_put;



		if (vma->page_sizes.sg < page_size) {

			pr_info("Unable to allocate page-size %x, finishing test early\n",
@@ -1527,8 +1495,6 @@ static int igt_ppgtt_pin_update(void *arg) 
			goto out_unpin;



		i915_vma_unpin(vma);

		i915_vma_close(vma);



		i915_gem_object_put(obj);

	}
@@ -1546,7 +1512,7 @@ static int igt_ppgtt_pin_update(void *arg) 


	err = i915_vma_pin(vma, 0, 0, flags);

	if (err)

		goto out_close;

		goto out_put;



	/*

	 * Make sure we don't end up with something like where the pde is still
@@ -1576,8 +1542,6 @@ static int igt_ppgtt_pin_update(void *arg) 


out_unpin:

	i915_vma_unpin(vma);

out_close:

	i915_vma_close(vma);

out_put:

	i915_gem_object_put(obj);

out_vm:
@@ -1629,13 +1593,11 @@ static int igt_tmpfs_fallback(void *arg) 


	err = i915_vma_pin(vma, 0, 0, PIN_USER);

	if (err)

		goto out_close;

		goto out_put;



	err = igt_check_page_sizes(vma);



	i915_vma_unpin(vma);

out_close:

	i915_vma_close(vma);

out_put:

	i915_gem_object_put(obj);

out_restore:
@@ -1682,7 +1644,7 @@ static int igt_shrink_thp(void *arg) 


	err = i915_vma_pin(vma, 0, 0, flags);

	if (err)

		goto out_close;

		goto out_put;



	if (obj->mm.page_sizes.phys < I915_GTT_PAGE_SIZE_2M) {

		pr_info("failed to allocate THP, finishing test early\n");
@@ -1706,7 +1668,7 @@ static int igt_shrink_thp(void *arg) 
	i915_gem_context_unlock_engines(ctx);

	i915_vma_unpin(vma);

	if (err)

		goto out_close;

		goto out_put;



	/*

	 * Now that the pages are *unpinned* shrink-all should invoke
@@ -1716,18 +1678,18 @@ static int igt_shrink_thp(void *arg) 
	if (i915_gem_object_has_pages(obj)) {

		pr_err("shrink-all didn't truncate the pages\n");

		err = -EINVAL;

		goto out_close;

		goto out_put;

	}



	if (obj->mm.page_sizes.sg || obj->mm.page_sizes.phys) {

		pr_err("residual page-size bits left\n");

		err = -EINVAL;

		goto out_close;

		goto out_put;

	}



	err = i915_vma_pin(vma, 0, 0, flags);

	if (err)

		goto out_close;

		goto out_put;



	while (n--) {

		err = cpu_check(obj, n, 0xdeadbeaf);
@@ -1737,8 +1699,6 @@ static int igt_shrink_thp(void *arg) 


out_unpin:

	i915_vma_unpin(vma);

out_close:

	i915_vma_close(vma);

out_put:

	i915_gem_object_put(obj);

out_vm:
@@ -1777,21 +1737,20 @@ int i915_gem_huge_page_mock_selftests(void) 
	if (!i915_vm_is_4lvl(&ppgtt->vm)) {

		pr_err("failed to create 48b PPGTT\n");

		err = -EINVAL;

		goto out_close;

		goto out_put;

	}



	/* If we were ever hit this then it's time to mock the 64K scratch */

	if (!i915_vm_has_scratch_64K(&ppgtt->vm)) {

		pr_err("PPGTT missing 64K scratch page\n");

		err = -EINVAL;

		goto out_close;

		goto out_put;

	}



	err = i915_subtests(tests, ppgtt);



out_close:

out_put:

	i915_vm_put(&ppgtt->vm);



out_unlock:

	drm_dev_put(&dev_priv->drm);

	return err;

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

+0 −1
Original line number Diff line number Diff line
@@ -1687,7 +1687,6 @@ static int read_from_scratch(struct i915_gem_context *ctx, 
		goto skip_request;



	i915_vma_unpin(vma);

	i915_vma_close(vma);



	i915_request_add(rq);

CRA Git | Maintained and supported by SUSTech CRA and CCSE