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

drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree 

As we want to be able to run inside atomic context for retiring the
i915_active, and we are no longer allowed to abuse mutex_trylock, split
the tree management portion of i915_active.mutex into an irq-safe
spinlock.

References: a0855d24 ("locking/mutex: Complain upon mutex API misuse in IRQ contexts")
References: https://bugs.freedesktop.org/show_bug.cgi?id=111626


Fixes: 274cbf20 ("drm/i915: Push the i915_active.retire into a worker")
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191114172535.1116-1-chris@chris-wilson.co.uk
parent 3fb33cd3

drivers/gpu/drm/i915/i915_active.c

+28 −29
Original line number Diff line number Diff line
@@ -91,14 +91,15 @@ static void debug_active_init(struct i915_active *ref) 


static void debug_active_activate(struct i915_active *ref)

{

	lockdep_assert_held(&ref->mutex);

	spin_lock_irq(&ref->tree_lock);

	if (!atomic_read(&ref->count)) /* before the first inc */

		debug_object_activate(ref, &active_debug_desc);

	spin_unlock_irq(&ref->tree_lock);

}



static void debug_active_deactivate(struct i915_active *ref)

{

	lockdep_assert_held(&ref->mutex);

	lockdep_assert_held(&ref->tree_lock);

	if (!atomic_read(&ref->count)) /* after the last dec */

		debug_object_deactivate(ref, &active_debug_desc);

}
@@ -128,29 +129,22 @@ __active_retire(struct i915_active *ref) 
{

	struct active_node *it, *n;

	struct rb_root root;

	bool retire = false;

	unsigned long flags;



	lockdep_assert_held(&ref->mutex);

	GEM_BUG_ON(i915_active_is_idle(ref));



	/* return the unused nodes to our slabcache -- flushing the allocator */

	if (atomic_dec_and_test(&ref->count)) {

	if (!atomic_dec_and_lock_irqsave(&ref->count, &ref->tree_lock, flags))

		return;



	GEM_BUG_ON(rcu_access_pointer(ref->excl.fence));

	debug_active_deactivate(ref);



	root = ref->tree;

	ref->tree = RB_ROOT;

	ref->cache = NULL;

		retire = true;

	}



	mutex_unlock(&ref->mutex);

	if (!retire)

		return;



	GEM_BUG_ON(rcu_access_pointer(ref->excl.fence));

	rbtree_postorder_for_each_entry_safe(it, n, &root, node) {

		GEM_BUG_ON(i915_active_fence_isset(&it->base));

		kmem_cache_free(global.slab_cache, it);

	}

	spin_unlock_irqrestore(&ref->tree_lock, flags);



	/* After the final retire, the entire struct may be freed */

	if (ref->retire)
@@ -158,6 +152,11 @@ __active_retire(struct i915_active *ref) 


	/* ... except if you wait on it, you must manage your own references! */

	wake_up_var(ref);



	rbtree_postorder_for_each_entry_safe(it, n, &root, node) {

		GEM_BUG_ON(i915_active_fence_isset(&it->base));

		kmem_cache_free(global.slab_cache, it);

	}

}



static void
@@ -169,7 +168,6 @@ active_work(struct work_struct *wrk) 
	if (atomic_add_unless(&ref->count, -1, 1))

		return;



	mutex_lock(&ref->mutex);

	__active_retire(ref);

}
@@ -180,9 +178,7 @@ active_retire(struct i915_active *ref) 
	if (atomic_add_unless(&ref->count, -1, 1))

		return;



	/* If we are inside interrupt context (fence signaling), defer */

	if (ref->flags & I915_ACTIVE_RETIRE_SLEEPS ||

	    !mutex_trylock(&ref->mutex)) {

	if (ref->flags & I915_ACTIVE_RETIRE_SLEEPS) {

		queue_work(system_unbound_wq, &ref->work);

		return;

	}
@@ -227,7 +223,7 @@ active_instance(struct i915_active *ref, struct intel_timeline *tl) 
	if (!prealloc)

		return NULL;



	mutex_lock(&ref->mutex);

	spin_lock_irq(&ref->tree_lock);

	GEM_BUG_ON(i915_active_is_idle(ref));



	parent = NULL;
@@ -257,7 +253,7 @@ active_instance(struct i915_active *ref, struct intel_timeline *tl) 


out:

	ref->cache = node;

	mutex_unlock(&ref->mutex);

	spin_unlock_irq(&ref->tree_lock);



	BUILD_BUG_ON(offsetof(typeof(*node), base));

	return &node->base;
@@ -278,8 +274,10 @@ void __i915_active_init(struct i915_active *ref, 
	if (bits & I915_ACTIVE_MAY_SLEEP)

		ref->flags |= I915_ACTIVE_RETIRE_SLEEPS;



	spin_lock_init(&ref->tree_lock);

	ref->tree = RB_ROOT;

	ref->cache = NULL;



	init_llist_head(&ref->preallocated_barriers);

	atomic_set(&ref->count, 0);

	__mutex_init(&ref->mutex, "i915_active", key);
@@ -510,7 +508,7 @@ static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx) 
	if (RB_EMPTY_ROOT(&ref->tree))

		return NULL;



	mutex_lock(&ref->mutex);

	spin_lock_irq(&ref->tree_lock);

	GEM_BUG_ON(i915_active_is_idle(ref));



	/*
@@ -575,7 +573,7 @@ static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx) 
			goto match;

	}



	mutex_unlock(&ref->mutex);

	spin_unlock_irq(&ref->tree_lock);



	return NULL;
@@ -583,7 +581,7 @@ match: 
	rb_erase(p, &ref->tree); /* Hide from waits and sibling allocations */

	if (p == &ref->cache->node)

		ref->cache = NULL;

	mutex_unlock(&ref->mutex);

	spin_unlock_irq(&ref->tree_lock);



	return rb_entry(p, struct active_node, node);

}
@@ -664,6 +662,7 @@ unwind: 
void i915_active_acquire_barrier(struct i915_active *ref)

{

	struct llist_node *pos, *next;

	unsigned long flags;



	GEM_BUG_ON(i915_active_is_idle(ref));
@@ -673,7 +672,7 @@ void i915_active_acquire_barrier(struct i915_active *ref) 
	 * populated by i915_request_add_active_barriers() to point to the

	 * request that will eventually release them.

	 */

	mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING);

	spin_lock_irqsave_nested(&ref->tree_lock, flags, SINGLE_DEPTH_NESTING);

	llist_for_each_safe(pos, next, take_preallocated_barriers(ref)) {

		struct active_node *node = barrier_from_ll(pos);

		struct intel_engine_cs *engine = barrier_to_engine(node);
@@ -699,7 +698,7 @@ void i915_active_acquire_barrier(struct i915_active *ref) 
		llist_add(barrier_to_ll(node), &engine->barrier_tasks);

		intel_engine_pm_put(engine);

	}

	mutex_unlock(&ref->mutex);

	spin_unlock_irqrestore(&ref->tree_lock, flags);

}



void i915_request_add_active_barriers(struct i915_request *rq)

drivers/gpu/drm/i915/i915_active_types.h

+1 −0
Original line number Diff line number Diff line
@@ -48,6 +48,7 @@ struct i915_active { 
	atomic_t count;

	struct mutex mutex;



	spinlock_t tree_lock;

	struct active_node *cache;

	struct rb_root tree;

drivers/gpu/drm/i915/selftests/i915_active.c

+2 −2
Original line number Diff line number Diff line
@@ -277,8 +277,8 @@ void i915_active_unlock_wait(struct i915_active *ref) 
	}



	/* And wait for the retire callback */

	mutex_lock(&ref->mutex);

	mutex_unlock(&ref->mutex);

	spin_lock_irq(&ref->tree_lock);

	spin_unlock_irq(&ref->tree_lock);



	/* ... which may have been on a thread instead */

	flush_work(&ref->work);

CRA Git | Maintained and supported by SUSTech CRA and CCSE