[Intel-gfx] [PATCH 3/3] drm/i915/selftests: Exercise adding requests to a full GGTT

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Thu Oct 12 09:48:07 UTC 2017


On 11/10/2017 15:06, Chris Wilson wrote:
> A bug recently encountered involved the issue where are we were
> submitting requests to different ppGTT, each would pin a segment of the
> GGTT for its logical context and ring. However, this is invisible to
> eviction as we do not tie the context/ring VMA to a request and so do
> not automatically wait upon it them (instead they are marked as pinned,
> prevent eviction entirely). Instead the eviction code must flush those
> contexts by switching to the kernel context. This selftest tries to
> fill the GGTT with contexts to exercise a path where the
> switch-to-kernel-context failed to make forward progress and we fail
> with ENOSPC.
> 
> v2: Make the hole in the filled GGTT explicit.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>   drivers/gpu/drm/i915/selftests/i915_gem_evict.c    | 129 +++++++++++++++++++++
>   .../gpu/drm/i915/selftests/i915_live_selftests.h   |   1 +
>   drivers/gpu/drm/i915/selftests/mock_context.c      |   6 +-
>   3 files changed, 131 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> index 5ea373221f49..b9c1527660a9 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
> @@ -24,6 +24,9 @@
>   
>   #include "../i915_selftest.h"
>   
> +#include "lib_sw_fence.h"
> +#include "mock_context.h"
> +#include "mock_drm.h"
>   #include "mock_gem_device.h"
>   
>   static int populate_ggtt(struct drm_i915_private *i915)
> @@ -325,6 +328,123 @@ static int igt_evict_vm(void *arg)
>   	return err;
>   }
>   
> +static int igt_evict_contexts(void *arg)
> +{
> +	const u64 PRETEND_GGTT_SIZE = 16ull << 20;
> +	struct drm_i915_private *i915 = arg;
> +	struct intel_engine_cs *engine;
> +	enum intel_engine_id id;
> +	struct reserved {
> +		struct drm_mm_node node;
> +		struct reserved *next;
> +	} *reserved = NULL;
> +	struct drm_mm_node hole;
> +	unsigned long count;
> +	int err;
> +
> +	mutex_lock(&i915->drm.struct_mutex);
> +
> +	/* Reserve a block so that we know we have enough to fit a few rq */
> +	memset(&hole, 0, sizeof(hole));
> +	err = i915_gem_gtt_insert(&i915->ggtt.base, &hole,
> +				  PRETEND_GGTT_SIZE, 0, I915_COLOR_UNEVICTABLE,
> +				  0, i915->ggtt.base.total,
> +				  PIN_NOEVICT);
> +	if (err)
> +		goto out_locked;
> +
> +	/* Make the GGTT appear small by filling it with unevictable nodes */
> +	count = 0;
> +	do {
> +		struct reserved *r;
> +
> +		r = kcalloc(1, sizeof(*r), GFP_KERNEL);
> +		if (!r) {
> +			err = -ENOMEM;
> +			goto out_locked;
> +		}
> +
> +		if (i915_gem_gtt_insert(&i915->ggtt.base, &r->node,
> +					1ul << 20, 0, I915_COLOR_UNEVICTABLE,
> +					0, i915->ggtt.base.total,
> +					PIN_NOEVICT)) {
> +			kfree(r);
> +			break;
> +		}
> +
> +		r->next = reserved;
> +		reserved = r;
> +
> +		count++;
> +	} while (1);
> +	drm_mm_remove_node(&hole);
> +	mutex_unlock(&i915->drm.struct_mutex);
> +	pr_info("Filled GGTT with %lu 1MiB nodes\n", count);
> +
> +	/* Overfill the GGTT with context objects and so try to evict one. */
> +	for_each_engine(engine, i915, id) {
> +		struct timed_fence tf;
> +		struct drm_file *file;
> +
> +		file = mock_file(i915);
> +		if (IS_ERR(file))
> +			return PTR_ERR(file);
> +
> +		count = 0;
> +		mutex_lock(&i915->drm.struct_mutex);
> +		timed_fence_init(&tf, round_jiffies_up(jiffies + HZ/2));

Should the timeout be i915.selftests.timeout_jiffies? Hm nor sure, since 
it is per engine as well.

> +		do {
> +			struct drm_i915_gem_request *rq;
> +			struct i915_gem_context *ctx;
> +
> +			ctx = live_context(i915, file);
> +			if (!ctx)
> +				break;
> +
> +			rq = i915_gem_request_alloc(engine, ctx);
> +			if (IS_ERR(rq)) {
> +				pr_err("Unexpected error from request alloc (ctx hw id %u, on %s): %d\n",
> +				       ctx->hw_id, engine->name,
> +				       (int)PTR_ERR(rq));
> +				err = PTR_ERR(rq) > +				break;
> +			}
> +
> +			i915_sw_fence_await_sw_fence_gfp(&rq->submit, &tf.fence,
> +							 GFP_KERNEL);
> +
> +			i915_add_request(rq);
> +			count++;
> +		} while(!i915_sw_fence_done(&tf.fence));
> +		mutex_unlock(&i915->drm.struct_mutex);
> +
> +		timed_fence_fini(&tf);
> +		pr_info("Submitted %lu contexts/requests on %s\n",
> +			count, engine->name);

I worry about no upper bound on the number of requests this will queue 
in HZ/2. What are the typical numbers it reports here? Would it be 
better to try some minimum number of requests (like a multiplier of the 
pretend GGTT size, if we can query or guess the typical LRC/ringbuffer 
sizes per context) and then signal the fence manually?

Regards,

Tvrtko

> +
> +		mock_file_free(i915, file);
> +
> +		if (err)
> +			break;
> +	}
> +
> +	mutex_lock(&i915->drm.struct_mutex);
> +out_locked:
> +	while (reserved) {
> +		struct reserved *next = reserved->next;
> +
> +		drm_mm_remove_node(&reserved->node);
> +		kfree(reserved);
> +
> +		reserved = next;
> +	}
> +	if (drm_mm_node_allocated(&hole))
> +		drm_mm_remove_node(&hole);
> +	mutex_unlock(&i915->drm.struct_mutex);
> +
> +	return err;
> +}
> +
>   int i915_gem_evict_mock_selftests(void)
>   {
>   	static const struct i915_subtest tests[] = {
> @@ -348,3 +468,12 @@ int i915_gem_evict_mock_selftests(void)
>   	drm_dev_unref(&i915->drm);
>   	return err;
>   }
> +
> +int i915_gem_evict_live_selftests(struct drm_i915_private *i915)
> +{
> +	static const struct i915_subtest tests[] = {
> +		SUBTEST(igt_evict_contexts),
> +	};
> +
> +	return i915_subtests(tests, i915);
> +}
> diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> index 64acd7eccc5c..54a73534b37e 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> +++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
> @@ -15,6 +15,7 @@ selftest(objects, i915_gem_object_live_selftests)
>   selftest(dmabuf, i915_gem_dmabuf_live_selftests)
>   selftest(coherency, i915_gem_coherency_live_selftests)
>   selftest(gtt, i915_gem_gtt_live_selftests)
> +selftest(evict, i915_gem_evict_live_selftests)
>   selftest(hugepages, i915_gem_huge_page_live_selftests)
>   selftest(contexts, i915_gem_context_live_selftests)
>   selftest(hangcheck, intel_hangcheck_live_selftests)
> diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c b/drivers/gpu/drm/i915/selftests/mock_context.c
> index 098ce643ad07..bbf80d42e793 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_context.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_context.c
> @@ -73,11 +73,7 @@ mock_context(struct drm_i915_private *i915,
>   
>   void mock_context_close(struct i915_gem_context *ctx)
>   {
> -	i915_gem_context_set_closed(ctx);
> -
> -	i915_ppgtt_close(&ctx->ppgtt->base);
> -
> -	i915_gem_context_put(ctx);
> +	context_close(ctx);
>   }
>   
>   void mock_init_contexts(struct drm_i915_private *i915)
> 


More information about the Intel-gfx mailing list