Re: [PATCH 18/50] drm/i915: Remove intel_context.active_link

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

On 12/04/2019 09:53, Chris Wilson wrote:
> We no longer need to track the active intel_contexts within each engine,
> allowing us to drop a tricky mutex_lock from inside unpin (which may
> occur inside fs_reclaim).
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
>   drivers/gpu/drm/i915/gt/intel_context.c           | 11 +----------
>   drivers/gpu/drm/i915/gt/intel_context_types.h     |  1 -
>   drivers/gpu/drm/i915/i915_debugfs.c               | 13 +++++++++----
>   drivers/gpu/drm/i915/i915_gem_context.c           |  2 --
>   drivers/gpu/drm/i915/i915_gem_context_types.h     |  1 -
>   drivers/gpu/drm/i915/selftests/i915_gem_context.c |  1 -
>   drivers/gpu/drm/i915/selftests/mock_context.c     |  1 -
>   7 files changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> index 5e506e648454..1f1761fc6597 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -49,7 +49,6 @@ int __intel_context_do_pin(struct intel_context *ce)
>   		return -EINTR;
>   
>   	if (likely(!atomic_read(&ce->pin_count))) {
> -		struct i915_gem_context *ctx = ce->gem_context;
>   		intel_wakeref_t wakeref;
>   
>   		err = 0;
> @@ -58,11 +57,7 @@ int __intel_context_do_pin(struct intel_context *ce)
>   		if (err)
>   			goto err;
>   
> -		i915_gem_context_get(ctx);
> -
> -		mutex_lock(&ctx->mutex);
> -		list_add(&ce->active_link, &ctx->active_engines);
> -		mutex_unlock(&ctx->mutex);
> +		i915_gem_context_get(ce->gem_context); /* for ctx->ppgtt */
>   
>   		intel_context_get(ce);
>   		smp_mb__before_atomic(); /* flush pin before it is visible */
> @@ -91,10 +86,6 @@ void intel_context_unpin(struct intel_context *ce)
>   	if (likely(atomic_dec_and_test(&ce->pin_count))) {
>   		ce->ops->unpin(ce);
>   
> -		mutex_lock(&ce->gem_context->mutex);
> -		list_del(&ce->active_link);
> -		mutex_unlock(&ce->gem_context->mutex);
> -
>   		i915_gem_context_put(ce->gem_context);
>   		intel_context_put(ce);
>   	}
> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
> index 3579c2708321..d5a7dbd0daee 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
> @@ -38,7 +38,6 @@ struct intel_context {
>   	struct intel_engine_cs *engine;
>   	struct intel_engine_cs *active;
>   
> -	struct list_head active_link;
>   	struct list_head signal_link;
>   	struct list_head signals;
>   
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 58956b49f392..6c6bd50d87c9 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -34,6 +34,7 @@
>   
>   #include "gt/intel_reset.h"
>   
> +#include "i915_gem_context.h"
>   #include "intel_dp.h"
>   #include "intel_drv.h"
>   #include "intel_fbc.h"
> @@ -396,9 +397,11 @@ static void print_context_stats(struct seq_file *m,
>   	struct i915_gem_context *ctx;
>   
>   	list_for_each_entry(ctx, &i915->contexts.list, link) {
> -		struct intel_context *ce;
> +		struct i915_gem_engines_iter it;
> +
> +		for_each_gem_engine(it, ctx) {

Why no i915_gem_context_lock_engines in debugfs?

Regards,

Tvrtko

> +			struct intel_context *ce = it.context;
>   
> -		list_for_each_entry(ce, &ctx->active_engines, active_link) {
>   			if (ce->state)
>   				per_file_stats(0, ce->state->obj, &kstats);
>   			if (ce->ring)
> @@ -1893,7 +1896,7 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   		return ret;
>   
>   	list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
> -		struct intel_context *ce;
> +		struct i915_gem_engines_iter it;
>   
>   		seq_puts(m, "HW context ");
>   		if (!list_empty(&ctx->hw_id_link))
> @@ -1917,7 +1920,9 @@ static int i915_context_status(struct seq_file *m, void *unused)
>   		seq_putc(m, ctx->remap_slice ? 'R' : 'r');
>   		seq_putc(m, '\n');
>   
> -		list_for_each_entry(ce, &ctx->active_engines, active_link) {
> +		for_each_gem_engine(it, ctx) {
> +			struct intel_context *ce = it.context;
> +
>   			seq_printf(m, "%s: ", ce->engine->name);
>   			if (ce->state)
>   				describe_obj(m, ce->state->obj);
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 99c89dc7533e..889cbc0554e5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -289,7 +289,6 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
>   {
>   	lockdep_assert_held(&ctx->i915->drm.struct_mutex);
>   	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
> -	GEM_BUG_ON(!list_empty(&ctx->active_engines));
>   
>   	release_hw_id(ctx);
>   	i915_ppgtt_put(ctx->ppgtt);
> @@ -416,7 +415,6 @@ __create_context(struct drm_i915_private *dev_priv)
>   	list_add_tail(&ctx->link, &dev_priv->contexts.list);
>   	ctx->i915 = dev_priv;
>   	ctx->sched.priority = I915_USER_PRIORITY(I915_PRIORITY_NORMAL);
> -	INIT_LIST_HEAD(&ctx->active_engines);
>   	mutex_init(&ctx->mutex);
>   
>   	mutex_init(&ctx->engines_mutex);
> diff --git a/drivers/gpu/drm/i915/i915_gem_context_types.h b/drivers/gpu/drm/i915/i915_gem_context_types.h
> index 5d146b7b84ec..33e5a0e36e75 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context_types.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context_types.h
> @@ -158,7 +158,6 @@ struct i915_gem_context {
>   	atomic_t hw_id_pin_count;
>   	struct list_head hw_id_link;
>   
> -	struct list_head active_engines;
>   	struct mutex mutex;
>   
>   	struct i915_sched_attr sched;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> index 7fd224a4ca4c..deedd1898fe5 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
> @@ -1675,7 +1675,6 @@ static int mock_context_barrier(void *arg)
>   		goto out;
>   	}
>   	i915_request_add(rq);
> -	GEM_BUG_ON(list_empty(&ctx->active_engines));
>   
>   	counter = 0;
>   	context_barrier_inject_fault = BIT(RCS0);
> diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c b/drivers/gpu/drm/i915/selftests/mock_context.c
> index 71c750693585..10e67c931ed1 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_context.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_context.c
> @@ -50,7 +50,6 @@ mock_context(struct drm_i915_private *i915,
>   	INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
>   	INIT_LIST_HEAD(&ctx->handles_list);
>   	INIT_LIST_HEAD(&ctx->hw_id_link);
> -	INIT_LIST_HEAD(&ctx->active_engines);
>   	mutex_init(&ctx->mutex);
>   
>   	ret = i915_gem_context_pin_hw_id(ctx);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx