All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission
@ 2019-10-03 21:00 Chris Wilson
  2019-10-03 21:00 ` [PATCH 2/5] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
                   ` (10 more replies)
  0 siblings, 11 replies; 22+ messages in thread
From: Chris Wilson @ 2019-10-03 21:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

If we unwind the active requests, and on resubmission discover that we
intend to preempt the active context with itself, simply skip the ELSP
submission.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 431d3b8c3371..3cfea1758fd2 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1739,11 +1739,26 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 
 	if (submit) {
 		*port = execlists_schedule_in(last, port - execlists->pending);
-		memset(port + 1, 0, (last_port - port) * sizeof(*port));
 		execlists->switch_priority_hint =
 			switch_prio(engine, *execlists->pending);
+
+		/*
+		 * Skip if we ended up with exactly the same set of requests,
+		 * e.g. trying to timeslice a pair of ordered contexts
+		 */
+		if (!memcmp(execlists->active, execlists->pending,
+			    (port - execlists->pending + 1) * sizeof(*port))) {
+			do
+				execlists_schedule_out(fetch_and_zero(port));
+			while (port-- != execlists->pending);
+
+			goto skip_submit;
+		}
+
+		memset(port + 1, 0, (last_port - port) * sizeof(*port));
 		execlists_submit_ports(engine);
 	} else {
+skip_submit:
 		ring_set_paused(engine, 0);
 	}
 }
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/5] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
  2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
@ 2019-10-03 21:00 ` Chris Wilson
  2019-10-04  8:31   ` Tvrtko Ursulin
  2019-10-04 10:11   ` [PATCH] " Chris Wilson
  2019-10-03 21:00 ` [PATCH 3/5] drm/mm: Use helpers for drm_mm_node booleans Chris Wilson
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 22+ messages in thread
From: Chris Wilson @ 2019-10-03 21:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

Make dma_fence_enable_sw_signaling() behave like its
dma_fence_add_callback() and dma_fence_default_wait() counterparts and
perform the test to enable signaling under the fence->lock, along with
the action to do so. This ensure that should an implementation be trying
to flush the cb_list (by signaling) on retirement before freeing the
fence, it can do so in a race-free manner.

See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked
with dma_fence_signal").

v2: Refactor all 3 enable_signaling paths to use a common function.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/dma-buf/dma-fence.c | 78 +++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 43 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 2c136aee3e79..b58528c1cc9d 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -273,6 +273,30 @@ void dma_fence_free(struct dma_fence *fence)
 }
 EXPORT_SYMBOL(dma_fence_free);
 
+static bool __dma_fence_enable_signaling(struct dma_fence *fence)
+{
+	bool was_set;
+
+	lockdep_assert_held(fence->lock);
+
+	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
+				   &fence->flags);
+
+	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+		return false;
+
+	if (!was_set && fence->ops->enable_signaling) {
+		if (!fence->ops->enable_signaling(fence)) {
+			dma_fence_signal_locked(fence);
+			return false;
+		}
+
+		trace_dma_fence_enable_signal(fence);
+	}
+
+	return true;
+}
+
 /**
  * dma_fence_enable_sw_signaling - enable signaling on fence
  * @fence: the fence to enable
@@ -285,19 +309,12 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence)
 {
 	unsigned long flags;
 
-	if (!test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
-			      &fence->flags) &&
-	    !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) &&
-	    fence->ops->enable_signaling) {
-		trace_dma_fence_enable_signal(fence);
-
-		spin_lock_irqsave(fence->lock, flags);
-
-		if (!fence->ops->enable_signaling(fence))
-			dma_fence_signal_locked(fence);
+	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+		return;
 
-		spin_unlock_irqrestore(fence->lock, flags);
-	}
+	spin_lock_irqsave(fence->lock, flags);
+	__dma_fence_enable_signaling(fence);
+	spin_unlock_irqrestore(fence->lock, flags);
 }
 EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
 
@@ -331,7 +348,6 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
 {
 	unsigned long flags;
 	int ret = 0;
-	bool was_set;
 
 	if (WARN_ON(!fence || !func))
 		return -EINVAL;
@@ -343,25 +359,14 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
 
 	spin_lock_irqsave(fence->lock, flags);
 
-	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
-				   &fence->flags);
-
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
-		ret = -ENOENT;
-	else if (!was_set && fence->ops->enable_signaling) {
-		trace_dma_fence_enable_signal(fence);
-
-		if (!fence->ops->enable_signaling(fence)) {
-			dma_fence_signal_locked(fence);
-			ret = -ENOENT;
-		}
-	}
-
-	if (!ret) {
+	if (__dma_fence_enable_signaling(fence)) {
 		cb->func = func;
 		list_add_tail(&cb->node, &fence->cb_list);
-	} else
+	} else {
 		INIT_LIST_HEAD(&cb->node);
+		ret = -ENOENT;
+	}
+
 	spin_unlock_irqrestore(fence->lock, flags);
 
 	return ret;
@@ -461,7 +466,6 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
 	struct default_wait_cb cb;
 	unsigned long flags;
 	signed long ret = timeout ? timeout : 1;
-	bool was_set;
 
 	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
 		return ret;
@@ -473,21 +477,9 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
 		goto out;
 	}
 
-	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
-				   &fence->flags);
-
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+	if (!__dma_fence_enable_signaling(fence))
 		goto out;
 
-	if (!was_set && fence->ops->enable_signaling) {
-		trace_dma_fence_enable_signal(fence);
-
-		if (!fence->ops->enable_signaling(fence)) {
-			dma_fence_signal_locked(fence);
-			goto out;
-		}
-	}
-
 	if (!timeout) {
 		ret = 0;
 		goto out;
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/5] drm/mm: Use helpers for drm_mm_node booleans
  2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
  2019-10-03 21:00 ` [PATCH 2/5] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
@ 2019-10-03 21:00 ` Chris Wilson
  2019-10-04  8:33   ` Tvrtko Ursulin
  2019-10-03 21:00 ` [PATCH 4/5] drm/mm: Convert drm_mm_node booleans to bitops Chris Wilson
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-10-03 21:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

In preparation for rearranging the booleans into a flags field, ensure
all the current users are using the inline helpers and not directly
accessing the members.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_mm.c                      | 19 ++++++++++++-------
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  4 ++--
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |  2 +-
 drivers/gpu/drm/i915/i915_gem.c               | 12 ++++++------
 drivers/gpu/drm/i915/i915_gem_evict.c         |  2 +-
 drivers/gpu/drm/i915/i915_vma.c               |  2 +-
 drivers/gpu/drm/i915/i915_vma.h               |  4 ++--
 drivers/gpu/drm/selftests/test-drm_mm.c       | 14 +++++++-------
 drivers/gpu/drm/vc4/vc4_crtc.c                |  2 +-
 drivers/gpu/drm/vc4/vc4_hvs.c                 |  2 +-
 drivers/gpu/drm/vc4/vc4_plane.c               |  4 ++--
 11 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 4581c5387372..99312bdc6273 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -174,7 +174,7 @@ static void drm_mm_interval_tree_add_node(struct drm_mm_node *hole_node,
 
 	node->__subtree_last = LAST(node);
 
-	if (hole_node->allocated) {
+	if (drm_mm_node_allocated(hole_node)) {
 		rb = &hole_node->rb;
 		while (rb) {
 			parent = rb_entry(rb, struct drm_mm_node, rb);
@@ -561,6 +561,11 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
 }
 EXPORT_SYMBOL(drm_mm_insert_node_in_range);
 
+static inline bool drm_mm_node_scanned_block(const struct drm_mm_node *node)
+{
+	return node->scanned_block;
+}
+
 /**
  * drm_mm_remove_node - Remove a memory node from the allocator.
  * @node: drm_mm_node to remove
@@ -574,8 +579,8 @@ void drm_mm_remove_node(struct drm_mm_node *node)
 	struct drm_mm *mm = node->mm;
 	struct drm_mm_node *prev_node;
 
-	DRM_MM_BUG_ON(!node->allocated);
-	DRM_MM_BUG_ON(node->scanned_block);
+	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
+	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
 
 	prev_node = list_prev_entry(node, node_list);
 
@@ -605,7 +610,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
 {
 	struct drm_mm *mm = old->mm;
 
-	DRM_MM_BUG_ON(!old->allocated);
+	DRM_MM_BUG_ON(!drm_mm_node_allocated(old));
 
 	*new = *old;
 
@@ -731,8 +736,8 @@ bool drm_mm_scan_add_block(struct drm_mm_scan *scan,
 	u64 adj_start, adj_end;
 
 	DRM_MM_BUG_ON(node->mm != mm);
-	DRM_MM_BUG_ON(!node->allocated);
-	DRM_MM_BUG_ON(node->scanned_block);
+	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
+	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
 	node->scanned_block = true;
 	mm->scan_active++;
 
@@ -818,7 +823,7 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,
 	struct drm_mm_node *prev_node;
 
 	DRM_MM_BUG_ON(node->mm != scan->mm);
-	DRM_MM_BUG_ON(!node->scanned_block);
+	DRM_MM_BUG_ON(!drm_mm_node_scanned_block(node));
 	node->scanned_block = false;
 
 	DRM_MM_BUG_ON(!node->mm->scan_active);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 27dbcb508055..20d8a6297985 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -968,7 +968,7 @@ static void reloc_cache_reset(struct reloc_cache *cache)
 		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
 		io_mapping_unmap_atomic((void __iomem *)vaddr);
 
-		if (cache->node.allocated) {
+		if (drm_mm_node_allocated(&cache->node)) {
 			ggtt->vm.clear_range(&ggtt->vm,
 					     cache->node.start,
 					     cache->node.size);
@@ -1061,7 +1061,7 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
 	}
 
 	offset = cache->node.start;
-	if (cache->node.allocated) {
+	if (drm_mm_node_allocated(&cache->node)) {
 		ggtt->vm.insert_page(&ggtt->vm,
 				     i915_gem_object_get_dma_address(obj, page),
 				     offset, I915_CACHE_NONE, 0);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index bb878119f06c..bb4889d2346d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -387,7 +387,7 @@ static u32 uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt)
 {
 	struct drm_mm_node *node = &ggtt->uc_fw;
 
-	GEM_BUG_ON(!node->allocated);
+	GEM_BUG_ON(!drm_mm_node_allocated(node));
 	GEM_BUG_ON(upper_32_bits(node->start));
 	GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1426e506700d..fa8e028ac0b5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -356,7 +356,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
 		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
 		if (ret)
 			goto out_unlock;
-		GEM_BUG_ON(!node.allocated);
+		GEM_BUG_ON(!drm_mm_node_allocated(&node));
 	}
 
 	mutex_unlock(&i915->drm.struct_mutex);
@@ -393,7 +393,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
 		unsigned page_offset = offset_in_page(offset);
 		unsigned page_length = PAGE_SIZE - page_offset;
 		page_length = remain < page_length ? remain : page_length;
-		if (node.allocated) {
+		if (drm_mm_node_allocated(&node)) {
 			ggtt->vm.insert_page(&ggtt->vm,
 					     i915_gem_object_get_dma_address(obj, offset >> PAGE_SHIFT),
 					     node.start, I915_CACHE_NONE, 0);
@@ -415,7 +415,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
 	i915_gem_object_unlock_fence(obj, fence);
 out_unpin:
 	mutex_lock(&i915->drm.struct_mutex);
-	if (node.allocated) {
+	if (drm_mm_node_allocated(&node)) {
 		ggtt->vm.clear_range(&ggtt->vm, node.start, node.size);
 		remove_mappable_node(&node);
 	} else {
@@ -566,7 +566,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
 		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
 		if (ret)
 			goto out_rpm;
-		GEM_BUG_ON(!node.allocated);
+		GEM_BUG_ON(!drm_mm_node_allocated(&node));
 	}
 
 	mutex_unlock(&i915->drm.struct_mutex);
@@ -604,7 +604,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
 		unsigned int page_offset = offset_in_page(offset);
 		unsigned int page_length = PAGE_SIZE - page_offset;
 		page_length = remain < page_length ? remain : page_length;
-		if (node.allocated) {
+		if (drm_mm_node_allocated(&node)) {
 			/* flush the write before we modify the GGTT */
 			intel_gt_flush_ggtt_writes(ggtt->vm.gt);
 			ggtt->vm.insert_page(&ggtt->vm,
@@ -636,7 +636,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
 out_unpin:
 	mutex_lock(&i915->drm.struct_mutex);
 	intel_gt_flush_ggtt_writes(ggtt->vm.gt);
-	if (node.allocated) {
+	if (drm_mm_node_allocated(&node)) {
 		ggtt->vm.clear_range(&ggtt->vm, node.start, node.size);
 		remove_mappable_node(&node);
 	} else {
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index e76c9da9992d..8c1e04f402bc 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -299,7 +299,7 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
 			break;
 		}
 
-		GEM_BUG_ON(!node->allocated);
+		GEM_BUG_ON(!drm_mm_node_allocated(node));
 		vma = container_of(node, typeof(*vma), node);
 
 		/* If we are using coloring to insert guard pages between
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 9d5b0f87c210..68c34b1a20e4 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -795,7 +795,7 @@ void i915_vma_reopen(struct i915_vma *vma)
 
 static void __i915_vma_destroy(struct i915_vma *vma)
 {
-	GEM_BUG_ON(vma->node.allocated);
+	GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
 	GEM_BUG_ON(vma->fence);
 
 	mutex_lock(&vma->vm->mutex);
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 8bcb5812c446..e49b199f7de7 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -228,7 +228,7 @@ static inline bool i915_vma_is_closed(const struct i915_vma *vma)
 static inline u32 i915_ggtt_offset(const struct i915_vma *vma)
 {
 	GEM_BUG_ON(!i915_vma_is_ggtt(vma));
-	GEM_BUG_ON(!vma->node.allocated);
+	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 	GEM_BUG_ON(upper_32_bits(vma->node.start));
 	GEM_BUG_ON(upper_32_bits(vma->node.start + vma->node.size - 1));
 	return lower_32_bits(vma->node.start);
@@ -390,7 +390,7 @@ static inline bool i915_vma_is_bound(const struct i915_vma *vma,
 static inline bool i915_node_color_differs(const struct drm_mm_node *node,
 					   unsigned long color)
 {
-	return node->allocated && node->color != color;
+	return drm_mm_node_allocated(node) && node->color != color;
 }
 
 /**
diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c
index 388f9844f4ba..9aabe82dcd3a 100644
--- a/drivers/gpu/drm/selftests/test-drm_mm.c
+++ b/drivers/gpu/drm/selftests/test-drm_mm.c
@@ -854,7 +854,7 @@ static bool assert_contiguous_in_range(struct drm_mm *mm,
 
 	if (start > 0) {
 		node = __drm_mm_interval_first(mm, 0, start - 1);
-		if (node->allocated) {
+		if (drm_mm_node_allocated(node)) {
 			pr_err("node before start: node=%llx+%llu, start=%llx\n",
 			       node->start, node->size, start);
 			return false;
@@ -863,7 +863,7 @@ static bool assert_contiguous_in_range(struct drm_mm *mm,
 
 	if (end < U64_MAX) {
 		node = __drm_mm_interval_first(mm, end, U64_MAX);
-		if (node->allocated) {
+		if (drm_mm_node_allocated(node)) {
 			pr_err("node after end: node=%llx+%llu, end=%llx\n",
 			       node->start, node->size, end);
 			return false;
@@ -1156,12 +1156,12 @@ static void show_holes(const struct drm_mm *mm, int count)
 		struct drm_mm_node *next = list_next_entry(hole, node_list);
 		const char *node1 = NULL, *node2 = NULL;
 
-		if (hole->allocated)
+		if (drm_mm_node_allocated(hole))
 			node1 = kasprintf(GFP_KERNEL,
 					  "[%llx + %lld, color=%ld], ",
 					  hole->start, hole->size, hole->color);
 
-		if (next->allocated)
+		if (drm_mm_node_allocated(next))
 			node2 = kasprintf(GFP_KERNEL,
 					  ", [%llx + %lld, color=%ld]",
 					  next->start, next->size, next->color);
@@ -1900,18 +1900,18 @@ static void separate_adjacent_colors(const struct drm_mm_node *node,
 				     u64 *start,
 				     u64 *end)
 {
-	if (node->allocated && node->color != color)
+	if (drm_mm_node_allocated(node) && node->color != color)
 		++*start;
 
 	node = list_next_entry(node, node_list);
-	if (node->allocated && node->color != color)
+	if (drm_mm_node_allocated(node) && node->color != color)
 		--*end;
 }
 
 static bool colors_abutt(const struct drm_mm_node *node)
 {
 	if (!drm_mm_hole_follows(node) &&
-	    list_next_entry(node, node_list)->allocated) {
+	    drm_mm_node_allocated(list_next_entry(node, node_list))) {
 		pr_err("colors abutt; %ld [%llx + %llx] is next to %ld [%llx + %llx]!\n",
 		       node->color, node->start, node->size,
 		       list_next_entry(node, node_list)->color,
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index f1f0a7c87771..b00e20f5ce05 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -994,7 +994,7 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
 	struct vc4_dev *vc4 = to_vc4_dev(crtc->dev);
 	struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state);
 
-	if (vc4_state->mm.allocated) {
+	if (drm_mm_node_allocated(&vc4_state->mm)) {
 		unsigned long flags;
 
 		spin_lock_irqsave(&vc4->hvs->mm_lock, flags);
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 9936b15d0bf1..5a43659da319 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -315,7 +315,7 @@ static void vc4_hvs_unbind(struct device *dev, struct device *master,
 	struct drm_device *drm = dev_get_drvdata(master);
 	struct vc4_dev *vc4 = drm->dev_private;
 
-	if (vc4->hvs->mitchell_netravali_filter.allocated)
+	if (drm_mm_node_allocated(&vc4->hvs->mitchell_netravali_filter))
 		drm_mm_remove_node(&vc4->hvs->mitchell_netravali_filter);
 
 	drm_mm_takedown(&vc4->hvs->dlist_mm);
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 5e5f90810aca..4934127f0d76 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -178,7 +178,7 @@ static void vc4_plane_destroy_state(struct drm_plane *plane,
 	struct vc4_dev *vc4 = to_vc4_dev(plane->dev);
 	struct vc4_plane_state *vc4_state = to_vc4_plane_state(state);
 
-	if (vc4_state->lbm.allocated) {
+	if (drm_mm_node_allocated(&vc4_state->lbm)) {
 		unsigned long irqflags;
 
 		spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
@@ -557,7 +557,7 @@ static int vc4_plane_allocate_lbm(struct drm_plane_state *state)
 	/* Allocate the LBM memory that the HVS will use for temporary
 	 * storage due to our scaling/format conversion.
 	 */
-	if (!vc4_state->lbm.allocated) {
+	if (!drm_mm_node_allocated(&vc4_state->lbm)) {
 		int ret;
 
 		spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/5] drm/mm: Convert drm_mm_node booleans to bitops
  2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
  2019-10-03 21:00 ` [PATCH 2/5] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
  2019-10-03 21:00 ` [PATCH 3/5] drm/mm: Use helpers for drm_mm_node booleans Chris Wilson
@ 2019-10-03 21:00 ` Chris Wilson
  2019-10-04  8:34   ` Tvrtko Ursulin
  2019-10-03 21:01 ` [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node() Chris Wilson
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-10-03 21:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

A straightforward conversion of assignment and checking of the boolean
state flags (allocated, scanned) into non-atomic bitops. The caller
remains responsible for all locking around the drm_mm and its nodes.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_mm.c                       | 18 +++++++++---------
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c |  2 +-
 drivers/gpu/drm/i915/i915_gem.c                |  4 ++--
 include/drm/drm_mm.h                           |  7 ++++---
 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 99312bdc6273..a9cab5e53731 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -426,7 +426,7 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node)
 
 	list_add(&node->node_list, &hole->node_list);
 	drm_mm_interval_tree_add_node(hole, node);
-	node->allocated = true;
+	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 	node->hole_size = 0;
 
 	rm_hole(hole);
@@ -545,7 +545,7 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
 
 		list_add(&node->node_list, &hole->node_list);
 		drm_mm_interval_tree_add_node(hole, node);
-		node->allocated = true;
+		__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 
 		rm_hole(hole);
 		if (adj_start > hole_start)
@@ -563,7 +563,7 @@ EXPORT_SYMBOL(drm_mm_insert_node_in_range);
 
 static inline bool drm_mm_node_scanned_block(const struct drm_mm_node *node)
 {
-	return node->scanned_block;
+	return test_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
 }
 
 /**
@@ -589,7 +589,7 @@ void drm_mm_remove_node(struct drm_mm_node *node)
 
 	drm_mm_interval_tree_remove(node, &mm->interval_tree);
 	list_del(&node->node_list);
-	node->allocated = false;
+	__clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 
 	if (drm_mm_hole_follows(prev_node))
 		rm_hole(prev_node);
@@ -627,8 +627,8 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
 				&mm->holes_addr);
 	}
 
-	old->allocated = false;
-	new->allocated = true;
+	__clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
+	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &new->flags);
 }
 EXPORT_SYMBOL(drm_mm_replace_node);
 
@@ -738,7 +738,7 @@ bool drm_mm_scan_add_block(struct drm_mm_scan *scan,
 	DRM_MM_BUG_ON(node->mm != mm);
 	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
 	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
-	node->scanned_block = true;
+	__set_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
 	mm->scan_active++;
 
 	/* Remove this block from the node_list so that we enlarge the hole
@@ -824,7 +824,7 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,
 
 	DRM_MM_BUG_ON(node->mm != scan->mm);
 	DRM_MM_BUG_ON(!drm_mm_node_scanned_block(node));
-	node->scanned_block = false;
+	__clear_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
 
 	DRM_MM_BUG_ON(!node->mm->scan_active);
 	node->mm->scan_active--;
@@ -922,7 +922,7 @@ void drm_mm_init(struct drm_mm *mm, u64 start, u64 size)
 
 	/* Clever trick to avoid a special case in the free hole tracking. */
 	INIT_LIST_HEAD(&mm->head_node.node_list);
-	mm->head_node.allocated = false;
+	mm->head_node.flags = 0;
 	mm->head_node.mm = mm;
 	mm->head_node.start = start + size;
 	mm->head_node.size = -size;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 20d8a6297985..c049199a1df5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -906,7 +906,7 @@ static void reloc_cache_init(struct reloc_cache *cache,
 	cache->use_64bit_reloc = HAS_64BIT_RELOC(i915);
 	cache->has_fence = cache->gen < 4;
 	cache->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;
-	cache->node.allocated = false;
+	cache->node.flags = 0;
 	cache->ce = NULL;
 	cache->rq = NULL;
 	cache->rq_size = 0;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fa8e028ac0b5..7046067f70c1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -351,7 +351,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
 					       PIN_NOEVICT);
 	if (!IS_ERR(vma)) {
 		node.start = i915_ggtt_offset(vma);
-		node.allocated = false;
+		node.flags = 0;
 	} else {
 		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
 		if (ret)
@@ -561,7 +561,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
 					       PIN_NOEVICT);
 	if (!IS_ERR(vma)) {
 		node.start = i915_ggtt_offset(vma);
-		node.allocated = false;
+		node.flags = 0;
 	} else {
 		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
 		if (ret)
diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
index 2c3bbb43c7d1..d7939c054259 100644
--- a/include/drm/drm_mm.h
+++ b/include/drm/drm_mm.h
@@ -168,8 +168,9 @@ struct drm_mm_node {
 	struct rb_node rb_hole_addr;
 	u64 __subtree_last;
 	u64 hole_size;
-	bool allocated : 1;
-	bool scanned_block : 1;
+	unsigned long flags;
+#define DRM_MM_NODE_ALLOCATED_BIT	0
+#define DRM_MM_NODE_SCANNED_BIT		1
 #ifdef CONFIG_DRM_DEBUG_MM
 	depot_stack_handle_t stack;
 #endif
@@ -253,7 +254,7 @@ struct drm_mm_scan {
  */
 static inline bool drm_mm_node_allocated(const struct drm_mm_node *node)
 {
-	return node->allocated;
+	return test_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 }
 
 /**
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()
  2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
                   ` (2 preceding siblings ...)
  2019-10-03 21:00 ` [PATCH 4/5] drm/mm: Convert drm_mm_node booleans to bitops Chris Wilson
@ 2019-10-03 21:01 ` Chris Wilson
  2019-10-04  9:15   ` Tvrtko Ursulin
  2019-10-03 22:38 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission Patchwork
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-10-03 21:01 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

A few callers need to serialise the destruction of their drm_mm_node and
ensure it is removed from the drm_mm before freeing. However, to be
completely sure that any access from another thread is complete before
we free the struct, we require the RELEASE semantics of
clear_bit_unlock().

This allows the conditional locking such as

Thread A				Thread B
    mutex_lock(mm_lock);                if (drm_mm_node_allocated(node)) {
    drm_mm_node_remove(node);               mutex_lock(mm_lock);
    mutex_unlock(mm_lock);                  drm_mm_node_remove(node);
                                            mutex_unlock(mm_lock);
                                         }
                                         kfree(node);

to serialise correctly without any lingering accesses from A to the
freed node. Allocation / insertion of the node is assumed never to race
with removal or eviction scanning.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_mm.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index a9cab5e53731..2a6e34663146 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -424,9 +424,9 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node)
 
 	node->mm = mm;
 
+	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 	list_add(&node->node_list, &hole->node_list);
 	drm_mm_interval_tree_add_node(hole, node);
-	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 	node->hole_size = 0;
 
 	rm_hole(hole);
@@ -543,9 +543,9 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
 		node->color = color;
 		node->hole_size = 0;
 
+		__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 		list_add(&node->node_list, &hole->node_list);
 		drm_mm_interval_tree_add_node(hole, node);
-		__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 
 		rm_hole(hole);
 		if (adj_start > hole_start)
@@ -589,11 +589,12 @@ void drm_mm_remove_node(struct drm_mm_node *node)
 
 	drm_mm_interval_tree_remove(node, &mm->interval_tree);
 	list_del(&node->node_list);
-	__clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 
 	if (drm_mm_hole_follows(prev_node))
 		rm_hole(prev_node);
 	add_hole(prev_node);
+
+	clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
 }
 EXPORT_SYMBOL(drm_mm_remove_node);
 
@@ -614,6 +615,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
 
 	*new = *old;
 
+	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &new->flags);
 	list_replace(&old->node_list, &new->node_list);
 	rb_replace_node_cached(&old->rb, &new->rb, &mm->interval_tree);
 
@@ -627,8 +629,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
 				&mm->holes_addr);
 	}
 
-	__clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
-	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &new->flags);
+	clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
 }
 EXPORT_SYMBOL(drm_mm_replace_node);
 
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission
  2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
                   ` (3 preceding siblings ...)
  2019-10-03 21:01 ` [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node() Chris Wilson
@ 2019-10-03 22:38 ` Patchwork
  2019-10-03 22:58 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-03 22:38 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915/execlists: Skip redundant resubmission
URL   : https://patchwork.freedesktop.org/series/67566/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
8852b84cf9e7 drm/i915/execlists: Skip redundant resubmission
b34677b475f8 dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
-:14: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked with dma_fence_signal")'
#14: 
See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked

total: 1 errors, 0 warnings, 0 checks, 120 lines checked
554fc3753c90 drm/mm: Use helpers for drm_mm_node booleans
84f0e5825415 drm/mm: Convert drm_mm_node booleans to bitops
45a8b0a2ec91 drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission
  2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
                   ` (4 preceding siblings ...)
  2019-10-03 22:38 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission Patchwork
@ 2019-10-03 22:58 ` Patchwork
  2019-10-04 10:08 ` [PATCH 1/5] " Tvrtko Ursulin
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-03 22:58 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915/execlists: Skip redundant resubmission
URL   : https://patchwork.freedesktop.org/series/67566/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7000 -> Patchwork_14659
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/index.html

Known issues
------------

  Here are the changes found in Patchwork_14659 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_basic@bad-close:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-icl-u3/igt@gem_basic@bad-close.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/fi-icl-u3/igt@gem_basic@bad-close.html

  * igt@gem_ctx_switch@legacy-render:
    - fi-icl-u3:          [PASS][3] -> [INCOMPLETE][4] ([fdo#107713] / [fdo#111381])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-icl-u3/igt@gem_ctx_switch@legacy-render.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/fi-icl-u3/igt@gem_ctx_switch@legacy-render.html

  * igt@gem_ctx_switch@rcs0:
    - fi-icl-u2:          [PASS][5] -> [INCOMPLETE][6] ([fdo#107713])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-icl-u2/igt@gem_ctx_switch@rcs0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/fi-icl-u2/igt@gem_ctx_switch@rcs0.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - {fi-tgl-u}:         [INCOMPLETE][7] ([fdo#111735]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-tgl-u/igt@gem_ctx_create@basic-files.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/fi-tgl-u/igt@gem_ctx_create@basic-files.html
    - {fi-icl-dsi}:       [INCOMPLETE][9] ([fdo#107713] / [fdo#109100]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-icl-dsi/igt@gem_ctx_create@basic-files.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/fi-icl-dsi/igt@gem_ctx_create@basic-files.html

  * igt@i915_selftest@live_execlists:
    - {fi-icl-guc}:       [INCOMPLETE][11] ([fdo#107713]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/fi-icl-guc/igt@i915_selftest@live_execlists.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/fi-icl-guc/igt@i915_selftest@live_execlists.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381
  [fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735


Participating hosts (52 -> 44)
------------------------------

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7000 -> Patchwork_14659

  CI-20190529: 20190529
  CI_DRM_7000: a6af6b11a94cbffff1c70dbab04ef1e13d79e4ae @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5211: 1601e1571eb0f29a06b64494040b3ea7859a650f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14659: 45a8b0a2ec910228fa0ab99fdce0073fbfb8d9bc @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

45a8b0a2ec91 drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()
84f0e5825415 drm/mm: Convert drm_mm_node booleans to bitops
554fc3753c90 drm/mm: Use helpers for drm_mm_node booleans
b34677b475f8 dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
8852b84cf9e7 drm/i915/execlists: Skip redundant resubmission

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/5] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
  2019-10-03 21:00 ` [PATCH 2/5] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
@ 2019-10-04  8:31   ` Tvrtko Ursulin
  2019-10-04 10:11   ` [PATCH] " Chris Wilson
  1 sibling, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-10-04  8:31 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: dri-devel


On 03/10/2019 22:00, Chris Wilson wrote:
> Make dma_fence_enable_sw_signaling() behave like its
> dma_fence_add_callback() and dma_fence_default_wait() counterparts and
> perform the test to enable signaling under the fence->lock, along with
> the action to do so. This ensure that should an implementation be trying
> to flush the cb_list (by signaling) on retirement before freeing the
> fence, it can do so in a race-free manner.
> 
> See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked
> with dma_fence_signal").
> 
> v2: Refactor all 3 enable_signaling paths to use a common function.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/dma-buf/dma-fence.c | 78 +++++++++++++++++--------------------
>   1 file changed, 35 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 2c136aee3e79..b58528c1cc9d 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -273,6 +273,30 @@ void dma_fence_free(struct dma_fence *fence)
>   }
>   EXPORT_SYMBOL(dma_fence_free);
>   
> +static bool __dma_fence_enable_signaling(struct dma_fence *fence)
> +{
> +	bool was_set;
> +
> +	lockdep_assert_held(fence->lock);
> +
> +	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> +				   &fence->flags);
> +
> +	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +		return false;
> +
> +	if (!was_set && fence->ops->enable_signaling) {
> +		if (!fence->ops->enable_signaling(fence)) {
> +			dma_fence_signal_locked(fence);
> +			return false;
> +		}
> +
> +		trace_dma_fence_enable_signal(fence);

Tracepoint used to come before the enable_signaling call so probably 
best to keep it like that.

> +	}
> +
> +	return true;
> +}
> +
>   /**
>    * dma_fence_enable_sw_signaling - enable signaling on fence
>    * @fence: the fence to enable
> @@ -285,19 +309,12 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence)
>   {
>   	unsigned long flags;
>   
> -	if (!test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> -			      &fence->flags) &&
> -	    !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) &&
> -	    fence->ops->enable_signaling) {
> -		trace_dma_fence_enable_signal(fence);
> -
> -		spin_lock_irqsave(fence->lock, flags);
> -
> -		if (!fence->ops->enable_signaling(fence))
> -			dma_fence_signal_locked(fence);
> +	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +		return;
>   
> -		spin_unlock_irqrestore(fence->lock, flags);
> -	}
> +	spin_lock_irqsave(fence->lock, flags);
> +	__dma_fence_enable_signaling(fence);
> +	spin_unlock_irqrestore(fence->lock, flags);
>   }
>   EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
>   
> @@ -331,7 +348,6 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
>   {
>   	unsigned long flags;
>   	int ret = 0;
> -	bool was_set;
>   
>   	if (WARN_ON(!fence || !func))
>   		return -EINVAL;
> @@ -343,25 +359,14 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
>   
>   	spin_lock_irqsave(fence->lock, flags);
>   
> -	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> -				   &fence->flags);
> -
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> -		ret = -ENOENT;
> -	else if (!was_set && fence->ops->enable_signaling) {
> -		trace_dma_fence_enable_signal(fence);
> -
> -		if (!fence->ops->enable_signaling(fence)) {
> -			dma_fence_signal_locked(fence);
> -			ret = -ENOENT;
> -		}
> -	}
> -
> -	if (!ret) {
> +	if (__dma_fence_enable_signaling(fence)) {
>   		cb->func = func;
>   		list_add_tail(&cb->node, &fence->cb_list);
> -	} else
> +	} else {
>   		INIT_LIST_HEAD(&cb->node);
> +		ret = -ENOENT;
> +	}
> +
>   	spin_unlock_irqrestore(fence->lock, flags);
>   
>   	return ret;
> @@ -461,7 +466,6 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
>   	struct default_wait_cb cb;
>   	unsigned long flags;
>   	signed long ret = timeout ? timeout : 1;
> -	bool was_set;
>   
>   	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>   		return ret;
> @@ -473,21 +477,9 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
>   		goto out;
>   	}
>   
> -	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> -				   &fence->flags);
> -
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +	if (!__dma_fence_enable_signaling(fence))
>   		goto out;
>   
> -	if (!was_set && fence->ops->enable_signaling) {
> -		trace_dma_fence_enable_signal(fence);
> -
> -		if (!fence->ops->enable_signaling(fence)) {
> -			dma_fence_signal_locked(fence);
> -			goto out;
> -		}
> -	}
> -
>   	if (!timeout) {
>   		ret = 0;
>   		goto out;
> 

The rest looks correct to me.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/5] drm/mm: Use helpers for drm_mm_node booleans
  2019-10-03 21:00 ` [PATCH 3/5] drm/mm: Use helpers for drm_mm_node booleans Chris Wilson
@ 2019-10-04  8:33   ` Tvrtko Ursulin
  0 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-10-04  8:33 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: dri-devel


On 03/10/2019 22:00, Chris Wilson wrote:
> In preparation for rearranging the booleans into a flags field, ensure
> all the current users are using the inline helpers and not directly
> accessing the members.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/drm_mm.c                      | 19 ++++++++++++-------
>   .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  4 ++--
>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |  2 +-
>   drivers/gpu/drm/i915/i915_gem.c               | 12 ++++++------
>   drivers/gpu/drm/i915/i915_gem_evict.c         |  2 +-
>   drivers/gpu/drm/i915/i915_vma.c               |  2 +-
>   drivers/gpu/drm/i915/i915_vma.h               |  4 ++--
>   drivers/gpu/drm/selftests/test-drm_mm.c       | 14 +++++++-------
>   drivers/gpu/drm/vc4/vc4_crtc.c                |  2 +-
>   drivers/gpu/drm/vc4/vc4_hvs.c                 |  2 +-
>   drivers/gpu/drm/vc4/vc4_plane.c               |  4 ++--
>   11 files changed, 36 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 4581c5387372..99312bdc6273 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -174,7 +174,7 @@ static void drm_mm_interval_tree_add_node(struct drm_mm_node *hole_node,
>   
>   	node->__subtree_last = LAST(node);
>   
> -	if (hole_node->allocated) {
> +	if (drm_mm_node_allocated(hole_node)) {
>   		rb = &hole_node->rb;
>   		while (rb) {
>   			parent = rb_entry(rb, struct drm_mm_node, rb);
> @@ -561,6 +561,11 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
>   }
>   EXPORT_SYMBOL(drm_mm_insert_node_in_range);
>   
> +static inline bool drm_mm_node_scanned_block(const struct drm_mm_node *node)
> +{
> +	return node->scanned_block;
> +}
> +
>   /**
>    * drm_mm_remove_node - Remove a memory node from the allocator.
>    * @node: drm_mm_node to remove
> @@ -574,8 +579,8 @@ void drm_mm_remove_node(struct drm_mm_node *node)
>   	struct drm_mm *mm = node->mm;
>   	struct drm_mm_node *prev_node;
>   
> -	DRM_MM_BUG_ON(!node->allocated);
> -	DRM_MM_BUG_ON(node->scanned_block);
> +	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
> +	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
>   
>   	prev_node = list_prev_entry(node, node_list);
>   
> @@ -605,7 +610,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
>   {
>   	struct drm_mm *mm = old->mm;
>   
> -	DRM_MM_BUG_ON(!old->allocated);
> +	DRM_MM_BUG_ON(!drm_mm_node_allocated(old));
>   
>   	*new = *old;
>   
> @@ -731,8 +736,8 @@ bool drm_mm_scan_add_block(struct drm_mm_scan *scan,
>   	u64 adj_start, adj_end;
>   
>   	DRM_MM_BUG_ON(node->mm != mm);
> -	DRM_MM_BUG_ON(!node->allocated);
> -	DRM_MM_BUG_ON(node->scanned_block);
> +	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
> +	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
>   	node->scanned_block = true;
>   	mm->scan_active++;
>   
> @@ -818,7 +823,7 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,
>   	struct drm_mm_node *prev_node;
>   
>   	DRM_MM_BUG_ON(node->mm != scan->mm);
> -	DRM_MM_BUG_ON(!node->scanned_block);
> +	DRM_MM_BUG_ON(!drm_mm_node_scanned_block(node));
>   	node->scanned_block = false;
>   
>   	DRM_MM_BUG_ON(!node->mm->scan_active);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 27dbcb508055..20d8a6297985 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -968,7 +968,7 @@ static void reloc_cache_reset(struct reloc_cache *cache)
>   		intel_gt_flush_ggtt_writes(ggtt->vm.gt);
>   		io_mapping_unmap_atomic((void __iomem *)vaddr);
>   
> -		if (cache->node.allocated) {
> +		if (drm_mm_node_allocated(&cache->node)) {
>   			ggtt->vm.clear_range(&ggtt->vm,
>   					     cache->node.start,
>   					     cache->node.size);
> @@ -1061,7 +1061,7 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
>   	}
>   
>   	offset = cache->node.start;
> -	if (cache->node.allocated) {
> +	if (drm_mm_node_allocated(&cache->node)) {
>   		ggtt->vm.insert_page(&ggtt->vm,
>   				     i915_gem_object_get_dma_address(obj, page),
>   				     offset, I915_CACHE_NONE, 0);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index bb878119f06c..bb4889d2346d 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -387,7 +387,7 @@ static u32 uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt)
>   {
>   	struct drm_mm_node *node = &ggtt->uc_fw;
>   
> -	GEM_BUG_ON(!node->allocated);
> +	GEM_BUG_ON(!drm_mm_node_allocated(node));
>   	GEM_BUG_ON(upper_32_bits(node->start));
>   	GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
>   
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1426e506700d..fa8e028ac0b5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -356,7 +356,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
>   		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
>   		if (ret)
>   			goto out_unlock;
> -		GEM_BUG_ON(!node.allocated);
> +		GEM_BUG_ON(!drm_mm_node_allocated(&node));
>   	}
>   
>   	mutex_unlock(&i915->drm.struct_mutex);
> @@ -393,7 +393,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
>   		unsigned page_offset = offset_in_page(offset);
>   		unsigned page_length = PAGE_SIZE - page_offset;
>   		page_length = remain < page_length ? remain : page_length;
> -		if (node.allocated) {
> +		if (drm_mm_node_allocated(&node)) {
>   			ggtt->vm.insert_page(&ggtt->vm,
>   					     i915_gem_object_get_dma_address(obj, offset >> PAGE_SHIFT),
>   					     node.start, I915_CACHE_NONE, 0);
> @@ -415,7 +415,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
>   	i915_gem_object_unlock_fence(obj, fence);
>   out_unpin:
>   	mutex_lock(&i915->drm.struct_mutex);
> -	if (node.allocated) {
> +	if (drm_mm_node_allocated(&node)) {
>   		ggtt->vm.clear_range(&ggtt->vm, node.start, node.size);
>   		remove_mappable_node(&node);
>   	} else {
> @@ -566,7 +566,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
>   		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
>   		if (ret)
>   			goto out_rpm;
> -		GEM_BUG_ON(!node.allocated);
> +		GEM_BUG_ON(!drm_mm_node_allocated(&node));
>   	}
>   
>   	mutex_unlock(&i915->drm.struct_mutex);
> @@ -604,7 +604,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
>   		unsigned int page_offset = offset_in_page(offset);
>   		unsigned int page_length = PAGE_SIZE - page_offset;
>   		page_length = remain < page_length ? remain : page_length;
> -		if (node.allocated) {
> +		if (drm_mm_node_allocated(&node)) {
>   			/* flush the write before we modify the GGTT */
>   			intel_gt_flush_ggtt_writes(ggtt->vm.gt);
>   			ggtt->vm.insert_page(&ggtt->vm,
> @@ -636,7 +636,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
>   out_unpin:
>   	mutex_lock(&i915->drm.struct_mutex);
>   	intel_gt_flush_ggtt_writes(ggtt->vm.gt);
> -	if (node.allocated) {
> +	if (drm_mm_node_allocated(&node)) {
>   		ggtt->vm.clear_range(&ggtt->vm, node.start, node.size);
>   		remove_mappable_node(&node);
>   	} else {
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index e76c9da9992d..8c1e04f402bc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -299,7 +299,7 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
>   			break;
>   		}
>   
> -		GEM_BUG_ON(!node->allocated);
> +		GEM_BUG_ON(!drm_mm_node_allocated(node));
>   		vma = container_of(node, typeof(*vma), node);
>   
>   		/* If we are using coloring to insert guard pages between
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 9d5b0f87c210..68c34b1a20e4 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -795,7 +795,7 @@ void i915_vma_reopen(struct i915_vma *vma)
>   
>   static void __i915_vma_destroy(struct i915_vma *vma)
>   {
> -	GEM_BUG_ON(vma->node.allocated);
> +	GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
>   	GEM_BUG_ON(vma->fence);
>   
>   	mutex_lock(&vma->vm->mutex);
> diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
> index 8bcb5812c446..e49b199f7de7 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -228,7 +228,7 @@ static inline bool i915_vma_is_closed(const struct i915_vma *vma)
>   static inline u32 i915_ggtt_offset(const struct i915_vma *vma)
>   {
>   	GEM_BUG_ON(!i915_vma_is_ggtt(vma));
> -	GEM_BUG_ON(!vma->node.allocated);
> +	GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
>   	GEM_BUG_ON(upper_32_bits(vma->node.start));
>   	GEM_BUG_ON(upper_32_bits(vma->node.start + vma->node.size - 1));
>   	return lower_32_bits(vma->node.start);
> @@ -390,7 +390,7 @@ static inline bool i915_vma_is_bound(const struct i915_vma *vma,
>   static inline bool i915_node_color_differs(const struct drm_mm_node *node,
>   					   unsigned long color)
>   {
> -	return node->allocated && node->color != color;
> +	return drm_mm_node_allocated(node) && node->color != color;
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c
> index 388f9844f4ba..9aabe82dcd3a 100644
> --- a/drivers/gpu/drm/selftests/test-drm_mm.c
> +++ b/drivers/gpu/drm/selftests/test-drm_mm.c
> @@ -854,7 +854,7 @@ static bool assert_contiguous_in_range(struct drm_mm *mm,
>   
>   	if (start > 0) {
>   		node = __drm_mm_interval_first(mm, 0, start - 1);
> -		if (node->allocated) {
> +		if (drm_mm_node_allocated(node)) {
>   			pr_err("node before start: node=%llx+%llu, start=%llx\n",
>   			       node->start, node->size, start);
>   			return false;
> @@ -863,7 +863,7 @@ static bool assert_contiguous_in_range(struct drm_mm *mm,
>   
>   	if (end < U64_MAX) {
>   		node = __drm_mm_interval_first(mm, end, U64_MAX);
> -		if (node->allocated) {
> +		if (drm_mm_node_allocated(node)) {
>   			pr_err("node after end: node=%llx+%llu, end=%llx\n",
>   			       node->start, node->size, end);
>   			return false;
> @@ -1156,12 +1156,12 @@ static void show_holes(const struct drm_mm *mm, int count)
>   		struct drm_mm_node *next = list_next_entry(hole, node_list);
>   		const char *node1 = NULL, *node2 = NULL;
>   
> -		if (hole->allocated)
> +		if (drm_mm_node_allocated(hole))
>   			node1 = kasprintf(GFP_KERNEL,
>   					  "[%llx + %lld, color=%ld], ",
>   					  hole->start, hole->size, hole->color);
>   
> -		if (next->allocated)
> +		if (drm_mm_node_allocated(next))
>   			node2 = kasprintf(GFP_KERNEL,
>   					  ", [%llx + %lld, color=%ld]",
>   					  next->start, next->size, next->color);
> @@ -1900,18 +1900,18 @@ static void separate_adjacent_colors(const struct drm_mm_node *node,
>   				     u64 *start,
>   				     u64 *end)
>   {
> -	if (node->allocated && node->color != color)
> +	if (drm_mm_node_allocated(node) && node->color != color)
>   		++*start;
>   
>   	node = list_next_entry(node, node_list);
> -	if (node->allocated && node->color != color)
> +	if (drm_mm_node_allocated(node) && node->color != color)
>   		--*end;
>   }
>   
>   static bool colors_abutt(const struct drm_mm_node *node)
>   {
>   	if (!drm_mm_hole_follows(node) &&
> -	    list_next_entry(node, node_list)->allocated) {
> +	    drm_mm_node_allocated(list_next_entry(node, node_list))) {
>   		pr_err("colors abutt; %ld [%llx + %llx] is next to %ld [%llx + %llx]!\n",
>   		       node->color, node->start, node->size,
>   		       list_next_entry(node, node_list)->color,
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index f1f0a7c87771..b00e20f5ce05 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -994,7 +994,7 @@ static void vc4_crtc_destroy_state(struct drm_crtc *crtc,
>   	struct vc4_dev *vc4 = to_vc4_dev(crtc->dev);
>   	struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state);
>   
> -	if (vc4_state->mm.allocated) {
> +	if (drm_mm_node_allocated(&vc4_state->mm)) {
>   		unsigned long flags;
>   
>   		spin_lock_irqsave(&vc4->hvs->mm_lock, flags);
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index 9936b15d0bf1..5a43659da319 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -315,7 +315,7 @@ static void vc4_hvs_unbind(struct device *dev, struct device *master,
>   	struct drm_device *drm = dev_get_drvdata(master);
>   	struct vc4_dev *vc4 = drm->dev_private;
>   
> -	if (vc4->hvs->mitchell_netravali_filter.allocated)
> +	if (drm_mm_node_allocated(&vc4->hvs->mitchell_netravali_filter))
>   		drm_mm_remove_node(&vc4->hvs->mitchell_netravali_filter);
>   
>   	drm_mm_takedown(&vc4->hvs->dlist_mm);
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 5e5f90810aca..4934127f0d76 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -178,7 +178,7 @@ static void vc4_plane_destroy_state(struct drm_plane *plane,
>   	struct vc4_dev *vc4 = to_vc4_dev(plane->dev);
>   	struct vc4_plane_state *vc4_state = to_vc4_plane_state(state);
>   
> -	if (vc4_state->lbm.allocated) {
> +	if (drm_mm_node_allocated(&vc4_state->lbm)) {
>   		unsigned long irqflags;
>   
>   		spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
> @@ -557,7 +557,7 @@ static int vc4_plane_allocate_lbm(struct drm_plane_state *state)
>   	/* Allocate the LBM memory that the HVS will use for temporary
>   	 * storage due to our scaling/format conversion.
>   	 */
> -	if (!vc4_state->lbm.allocated) {
> +	if (!drm_mm_node_allocated(&vc4_state->lbm)) {
>   		int ret;
>   
>   		spin_lock_irqsave(&vc4->hvs->mm_lock, irqflags);
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/5] drm/mm: Convert drm_mm_node booleans to bitops
  2019-10-03 21:00 ` [PATCH 4/5] drm/mm: Convert drm_mm_node booleans to bitops Chris Wilson
@ 2019-10-04  8:34   ` Tvrtko Ursulin
  0 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-10-04  8:34 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: dri-devel


On 03/10/2019 22:00, Chris Wilson wrote:
> A straightforward conversion of assignment and checking of the boolean
> state flags (allocated, scanned) into non-atomic bitops. The caller
> remains responsible for all locking around the drm_mm and its nodes.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/drm_mm.c                       | 18 +++++++++---------
>   drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c |  2 +-
>   drivers/gpu/drm/i915/i915_gem.c                |  4 ++--
>   include/drm/drm_mm.h                           |  7 ++++---
>   4 files changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 99312bdc6273..a9cab5e53731 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -426,7 +426,7 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node)
>   
>   	list_add(&node->node_list, &hole->node_list);
>   	drm_mm_interval_tree_add_node(hole, node);
> -	node->allocated = true;
> +	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   	node->hole_size = 0;
>   
>   	rm_hole(hole);
> @@ -545,7 +545,7 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
>   
>   		list_add(&node->node_list, &hole->node_list);
>   		drm_mm_interval_tree_add_node(hole, node);
> -		node->allocated = true;
> +		__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   
>   		rm_hole(hole);
>   		if (adj_start > hole_start)
> @@ -563,7 +563,7 @@ EXPORT_SYMBOL(drm_mm_insert_node_in_range);
>   
>   static inline bool drm_mm_node_scanned_block(const struct drm_mm_node *node)
>   {
> -	return node->scanned_block;
> +	return test_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
>   }
>   
>   /**
> @@ -589,7 +589,7 @@ void drm_mm_remove_node(struct drm_mm_node *node)
>   
>   	drm_mm_interval_tree_remove(node, &mm->interval_tree);
>   	list_del(&node->node_list);
> -	node->allocated = false;
> +	__clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   
>   	if (drm_mm_hole_follows(prev_node))
>   		rm_hole(prev_node);
> @@ -627,8 +627,8 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
>   				&mm->holes_addr);
>   	}
>   
> -	old->allocated = false;
> -	new->allocated = true;
> +	__clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
> +	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &new->flags);
>   }
>   EXPORT_SYMBOL(drm_mm_replace_node);
>   
> @@ -738,7 +738,7 @@ bool drm_mm_scan_add_block(struct drm_mm_scan *scan,
>   	DRM_MM_BUG_ON(node->mm != mm);
>   	DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
>   	DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
> -	node->scanned_block = true;
> +	__set_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
>   	mm->scan_active++;
>   
>   	/* Remove this block from the node_list so that we enlarge the hole
> @@ -824,7 +824,7 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,
>   
>   	DRM_MM_BUG_ON(node->mm != scan->mm);
>   	DRM_MM_BUG_ON(!drm_mm_node_scanned_block(node));
> -	node->scanned_block = false;
> +	__clear_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
>   
>   	DRM_MM_BUG_ON(!node->mm->scan_active);
>   	node->mm->scan_active--;
> @@ -922,7 +922,7 @@ void drm_mm_init(struct drm_mm *mm, u64 start, u64 size)
>   
>   	/* Clever trick to avoid a special case in the free hole tracking. */
>   	INIT_LIST_HEAD(&mm->head_node.node_list);
> -	mm->head_node.allocated = false;
> +	mm->head_node.flags = 0;
>   	mm->head_node.mm = mm;
>   	mm->head_node.start = start + size;
>   	mm->head_node.size = -size;
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 20d8a6297985..c049199a1df5 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -906,7 +906,7 @@ static void reloc_cache_init(struct reloc_cache *cache,
>   	cache->use_64bit_reloc = HAS_64BIT_RELOC(i915);
>   	cache->has_fence = cache->gen < 4;
>   	cache->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;
> -	cache->node.allocated = false;
> +	cache->node.flags = 0;
>   	cache->ce = NULL;
>   	cache->rq = NULL;
>   	cache->rq_size = 0;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index fa8e028ac0b5..7046067f70c1 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -351,7 +351,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
>   					       PIN_NOEVICT);
>   	if (!IS_ERR(vma)) {
>   		node.start = i915_ggtt_offset(vma);
> -		node.allocated = false;
> +		node.flags = 0;
>   	} else {
>   		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
>   		if (ret)
> @@ -561,7 +561,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
>   					       PIN_NOEVICT);
>   	if (!IS_ERR(vma)) {
>   		node.start = i915_ggtt_offset(vma);
> -		node.allocated = false;
> +		node.flags = 0;
>   	} else {
>   		ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
>   		if (ret)
> diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
> index 2c3bbb43c7d1..d7939c054259 100644
> --- a/include/drm/drm_mm.h
> +++ b/include/drm/drm_mm.h
> @@ -168,8 +168,9 @@ struct drm_mm_node {
>   	struct rb_node rb_hole_addr;
>   	u64 __subtree_last;
>   	u64 hole_size;
> -	bool allocated : 1;
> -	bool scanned_block : 1;
> +	unsigned long flags;
> +#define DRM_MM_NODE_ALLOCATED_BIT	0
> +#define DRM_MM_NODE_SCANNED_BIT		1
>   #ifdef CONFIG_DRM_DEBUG_MM
>   	depot_stack_handle_t stack;
>   #endif
> @@ -253,7 +254,7 @@ struct drm_mm_scan {
>    */
>   static inline bool drm_mm_node_allocated(const struct drm_mm_node *node)
>   {
> -	return node->allocated;
> +	return test_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   }
>   
>   /**
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()
  2019-10-03 21:01 ` [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node() Chris Wilson
@ 2019-10-04  9:15   ` Tvrtko Ursulin
  2019-10-04 11:07     ` Chris Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-10-04  9:15 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: dri-devel


On 03/10/2019 22:01, Chris Wilson wrote:
> A few callers need to serialise the destruction of their drm_mm_node and
> ensure it is removed from the drm_mm before freeing. However, to be
> completely sure that any access from another thread is complete before
> we free the struct, we require the RELEASE semantics of
> clear_bit_unlock().
> 
> This allows the conditional locking such as
> 
> Thread A				Thread B
>      mutex_lock(mm_lock);                if (drm_mm_node_allocated(node)) {
>      drm_mm_node_remove(node);               mutex_lock(mm_lock);
>      mutex_unlock(mm_lock);                  drm_mm_node_remove(node);
>                                              mutex_unlock(mm_lock);
>                                           }
>                                           kfree(node);

My understanding is that release semantics on node allocated mean 1 -> 0 
transition is guaranteed to be seen only when thread A 
drm_mm_node_remove is fully done with the removal. But if it is in the 
middle of removal, node is still seen as allocated outside and thread B 
can enter the if-body, wait for the lock, and then drm_mm_node_remove 
will attempt a double removal. So I think another drm_mm_node_allocated 
under the lock is needed.

But the fkree part looks correct. There can be no false negatives with 
the release semantics on the allocated bit.

Regards,

Tvrtko

> to serialise correctly without any lingering accesses from A to the
> freed node. Allocation / insertion of the node is assumed never to race
> with removal or eviction scanning.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/drm_mm.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index a9cab5e53731..2a6e34663146 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -424,9 +424,9 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node)
>   
>   	node->mm = mm;
>   
> +	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   	list_add(&node->node_list, &hole->node_list);
>   	drm_mm_interval_tree_add_node(hole, node);
> -	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   	node->hole_size = 0;
>   
>   	rm_hole(hole);
> @@ -543,9 +543,9 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
>   		node->color = color;
>   		node->hole_size = 0;
>   
> +		__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   		list_add(&node->node_list, &hole->node_list);
>   		drm_mm_interval_tree_add_node(hole, node);
> -		__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   
>   		rm_hole(hole);
>   		if (adj_start > hole_start)
> @@ -589,11 +589,12 @@ void drm_mm_remove_node(struct drm_mm_node *node)
>   
>   	drm_mm_interval_tree_remove(node, &mm->interval_tree);
>   	list_del(&node->node_list);
> -	__clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   
>   	if (drm_mm_hole_follows(prev_node))
>   		rm_hole(prev_node);
>   	add_hole(prev_node);
> +
> +	clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
>   }
>   EXPORT_SYMBOL(drm_mm_remove_node);
>   
> @@ -614,6 +615,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
>   
>   	*new = *old;
>   
> +	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &new->flags);
>   	list_replace(&old->node_list, &new->node_list);
>   	rb_replace_node_cached(&old->rb, &new->rb, &mm->interval_tree);
>   
> @@ -627,8 +629,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
>   				&mm->holes_addr);
>   	}
>   
> -	__clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
> -	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &new->flags);
> +	clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
>   }
>   EXPORT_SYMBOL(drm_mm_replace_node);
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission
  2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
                   ` (5 preceding siblings ...)
  2019-10-03 22:58 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-10-04 10:08 ` Tvrtko Ursulin
  2019-10-04 10:25 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2) Patchwork
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-10-04 10:08 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: dri-devel


On 03/10/2019 22:00, Chris Wilson wrote:
> If we unwind the active requests, and on resubmission discover that we
> intend to preempt the active context with itself, simply skip the ELSP
> submission.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/intel_lrc.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 431d3b8c3371..3cfea1758fd2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1739,11 +1739,26 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>   
>   	if (submit) {
>   		*port = execlists_schedule_in(last, port - execlists->pending);
> -		memset(port + 1, 0, (last_port - port) * sizeof(*port));
>   		execlists->switch_priority_hint =
>   			switch_prio(engine, *execlists->pending);
> +
> +		/*
> +		 * Skip if we ended up with exactly the same set of requests,
> +		 * e.g. trying to timeslice a pair of ordered contexts
> +		 */
> +		if (!memcmp(execlists->active, execlists->pending,
> +			    (port - execlists->pending + 1) * sizeof(*port))) {
> +			do
> +				execlists_schedule_out(fetch_and_zero(port));
> +			while (port-- != execlists->pending);
> +
> +			goto skip_submit;
> +		}
> +
> +		memset(port + 1, 0, (last_port - port) * sizeof(*port));
>   		execlists_submit_ports(engine);
>   	} else {
> +skip_submit:
>   		ring_set_paused(engine, 0);
>   	}
>   }
> 

A little bit of tracepoint noise with in/out but looks correct after I 
read the new code. And I wonder if trace.pl still works after last 
couple months of refactors.. :)

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
  2019-10-03 21:00 ` [PATCH 2/5] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
  2019-10-04  8:31   ` Tvrtko Ursulin
@ 2019-10-04 10:11   ` Chris Wilson
  2019-10-04 10:18     ` Tvrtko Ursulin
  1 sibling, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-10-04 10:11 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel, tvrtko.ursulin

Make dma_fence_enable_sw_signaling() behave like its
dma_fence_add_callback() and dma_fence_default_wait() counterparts and
perform the test to enable signaling under the fence->lock, along with
the action to do so. This ensure that should an implementation be trying
to flush the cb_list (by signaling) on retirement before freeing the
fence, it can do so in a race-free manner.

See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked
with dma_fence_signal").

v2: Refactor all 3 enable_signaling paths to use a common function.
v3: Don't argue, just keep the tracepoint in the existing spot.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/dma-buf/dma-fence.c | 78 +++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 43 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 2c136aee3e79..052a41e2451c 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -273,6 +273,30 @@ void dma_fence_free(struct dma_fence *fence)
 }
 EXPORT_SYMBOL(dma_fence_free);
 
+static bool __dma_fence_enable_signaling(struct dma_fence *fence)
+{
+	bool was_set;
+
+	lockdep_assert_held(fence->lock);
+
+	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
+				   &fence->flags);
+
+	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+		return false;
+
+	if (!was_set && fence->ops->enable_signaling) {
+		trace_dma_fence_enable_signal(fence);
+
+		if (!fence->ops->enable_signaling(fence)) {
+			dma_fence_signal_locked(fence);
+			return false;
+		}
+	}
+
+	return true;
+}
+
 /**
  * dma_fence_enable_sw_signaling - enable signaling on fence
  * @fence: the fence to enable
@@ -285,19 +309,12 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence)
 {
 	unsigned long flags;
 
-	if (!test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
-			      &fence->flags) &&
-	    !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) &&
-	    fence->ops->enable_signaling) {
-		trace_dma_fence_enable_signal(fence);
-
-		spin_lock_irqsave(fence->lock, flags);
-
-		if (!fence->ops->enable_signaling(fence))
-			dma_fence_signal_locked(fence);
+	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+		return;
 
-		spin_unlock_irqrestore(fence->lock, flags);
-	}
+	spin_lock_irqsave(fence->lock, flags);
+	__dma_fence_enable_signaling(fence);
+	spin_unlock_irqrestore(fence->lock, flags);
 }
 EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
 
@@ -331,7 +348,6 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
 {
 	unsigned long flags;
 	int ret = 0;
-	bool was_set;
 
 	if (WARN_ON(!fence || !func))
 		return -EINVAL;
@@ -343,25 +359,14 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
 
 	spin_lock_irqsave(fence->lock, flags);
 
-	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
-				   &fence->flags);
-
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
-		ret = -ENOENT;
-	else if (!was_set && fence->ops->enable_signaling) {
-		trace_dma_fence_enable_signal(fence);
-
-		if (!fence->ops->enable_signaling(fence)) {
-			dma_fence_signal_locked(fence);
-			ret = -ENOENT;
-		}
-	}
-
-	if (!ret) {
+	if (__dma_fence_enable_signaling(fence)) {
 		cb->func = func;
 		list_add_tail(&cb->node, &fence->cb_list);
-	} else
+	} else {
 		INIT_LIST_HEAD(&cb->node);
+		ret = -ENOENT;
+	}
+
 	spin_unlock_irqrestore(fence->lock, flags);
 
 	return ret;
@@ -461,7 +466,6 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
 	struct default_wait_cb cb;
 	unsigned long flags;
 	signed long ret = timeout ? timeout : 1;
-	bool was_set;
 
 	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
 		return ret;
@@ -473,21 +477,9 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
 		goto out;
 	}
 
-	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
-				   &fence->flags);
-
-	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+	if (!__dma_fence_enable_signaling(fence))
 		goto out;
 
-	if (!was_set && fence->ops->enable_signaling) {
-		trace_dma_fence_enable_signal(fence);
-
-		if (!fence->ops->enable_signaling(fence)) {
-			dma_fence_signal_locked(fence);
-			goto out;
-		}
-	}
-
 	if (!timeout) {
 		ret = 0;
 		goto out;
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
  2019-10-04 10:11   ` [PATCH] " Chris Wilson
@ 2019-10-04 10:18     ` Tvrtko Ursulin
  0 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-10-04 10:18 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: dri-devel


On 04/10/2019 11:11, Chris Wilson wrote:
> Make dma_fence_enable_sw_signaling() behave like its
> dma_fence_add_callback() and dma_fence_default_wait() counterparts and
> perform the test to enable signaling under the fence->lock, along with
> the action to do so. This ensure that should an implementation be trying
> to flush the cb_list (by signaling) on retirement before freeing the
> fence, it can do so in a race-free manner.
> 
> See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked
> with dma_fence_signal").
> 
> v2: Refactor all 3 enable_signaling paths to use a common function.
> v3: Don't argue, just keep the tracepoint in the existing spot.

I would understand the argument but a) meh and b) why change the ABI, 
kind of.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/dma-buf/dma-fence.c | 78 +++++++++++++++++--------------------
>   1 file changed, 35 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 2c136aee3e79..052a41e2451c 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -273,6 +273,30 @@ void dma_fence_free(struct dma_fence *fence)
>   }
>   EXPORT_SYMBOL(dma_fence_free);
>   
> +static bool __dma_fence_enable_signaling(struct dma_fence *fence)
> +{
> +	bool was_set;
> +
> +	lockdep_assert_held(fence->lock);
> +
> +	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> +				   &fence->flags);
> +
> +	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +		return false;
> +
> +	if (!was_set && fence->ops->enable_signaling) {
> +		trace_dma_fence_enable_signal(fence);
> +
> +		if (!fence->ops->enable_signaling(fence)) {
> +			dma_fence_signal_locked(fence);
> +			return false;
> +		}
> +	}
> +
> +	return true;
> +}
> +
>   /**
>    * dma_fence_enable_sw_signaling - enable signaling on fence
>    * @fence: the fence to enable
> @@ -285,19 +309,12 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence)
>   {
>   	unsigned long flags;
>   
> -	if (!test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> -			      &fence->flags) &&
> -	    !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) &&
> -	    fence->ops->enable_signaling) {
> -		trace_dma_fence_enable_signal(fence);
> -
> -		spin_lock_irqsave(fence->lock, flags);
> -
> -		if (!fence->ops->enable_signaling(fence))
> -			dma_fence_signal_locked(fence);
> +	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +		return;
>   
> -		spin_unlock_irqrestore(fence->lock, flags);
> -	}
> +	spin_lock_irqsave(fence->lock, flags);
> +	__dma_fence_enable_signaling(fence);
> +	spin_unlock_irqrestore(fence->lock, flags);
>   }
>   EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
>   
> @@ -331,7 +348,6 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
>   {
>   	unsigned long flags;
>   	int ret = 0;
> -	bool was_set;
>   
>   	if (WARN_ON(!fence || !func))
>   		return -EINVAL;
> @@ -343,25 +359,14 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
>   
>   	spin_lock_irqsave(fence->lock, flags);
>   
> -	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> -				   &fence->flags);
> -
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> -		ret = -ENOENT;
> -	else if (!was_set && fence->ops->enable_signaling) {
> -		trace_dma_fence_enable_signal(fence);
> -
> -		if (!fence->ops->enable_signaling(fence)) {
> -			dma_fence_signal_locked(fence);
> -			ret = -ENOENT;
> -		}
> -	}
> -
> -	if (!ret) {
> +	if (__dma_fence_enable_signaling(fence)) {
>   		cb->func = func;
>   		list_add_tail(&cb->node, &fence->cb_list);
> -	} else
> +	} else {
>   		INIT_LIST_HEAD(&cb->node);
> +		ret = -ENOENT;
> +	}
> +
>   	spin_unlock_irqrestore(fence->lock, flags);
>   
>   	return ret;
> @@ -461,7 +466,6 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
>   	struct default_wait_cb cb;
>   	unsigned long flags;
>   	signed long ret = timeout ? timeout : 1;
> -	bool was_set;
>   
>   	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
>   		return ret;
> @@ -473,21 +477,9 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, signed long timeout)
>   		goto out;
>   	}
>   
> -	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> -				   &fence->flags);
> -
> -	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> +	if (!__dma_fence_enable_signaling(fence))
>   		goto out;
>   
> -	if (!was_set && fence->ops->enable_signaling) {
> -		trace_dma_fence_enable_signal(fence);
> -
> -		if (!fence->ops->enable_signaling(fence)) {
> -			dma_fence_signal_locked(fence);
> -			goto out;
> -		}
> -	}
> -
>   	if (!timeout) {
>   		ret = 0;
>   		goto out;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2)
  2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
                   ` (6 preceding siblings ...)
  2019-10-04 10:08 ` [PATCH 1/5] " Tvrtko Ursulin
@ 2019-10-04 10:25 ` Patchwork
  2019-10-04 11:36 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-04 10:25 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2)
URL   : https://patchwork.freedesktop.org/series/67566/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
6adbc5589cd8 drm/i915/execlists: Skip redundant resubmission
d1690911537b dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
-:14: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked with dma_fence_signal")'
#14: 
See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked

total: 1 errors, 0 warnings, 0 checks, 120 lines checked
870ebb8b6d24 drm/mm: Use helpers for drm_mm_node booleans
fe0c8958ee33 drm/mm: Convert drm_mm_node booleans to bitops
0ad3aa61926d drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()
  2019-10-04  9:15   ` Tvrtko Ursulin
@ 2019-10-04 11:07     ` Chris Wilson
  2019-10-04 11:17       ` Chris Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-10-04 11:07 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: dri-devel

Quoting Tvrtko Ursulin (2019-10-04 10:15:20)
> 
> On 03/10/2019 22:01, Chris Wilson wrote:
> > A few callers need to serialise the destruction of their drm_mm_node and
> > ensure it is removed from the drm_mm before freeing. However, to be
> > completely sure that any access from another thread is complete before
> > we free the struct, we require the RELEASE semantics of
> > clear_bit_unlock().
> > 
> > This allows the conditional locking such as
> > 
> > Thread A                              Thread B
> >      mutex_lock(mm_lock);                if (drm_mm_node_allocated(node)) {
> >      drm_mm_node_remove(node);               mutex_lock(mm_lock);
> >      mutex_unlock(mm_lock);                  drm_mm_node_remove(node);
> >                                              mutex_unlock(mm_lock);
> >                                           }
> >                                           kfree(node);
> 
> My understanding is that release semantics on node allocated mean 1 -> 0 
> transition is guaranteed to be seen only when thread A 
> drm_mm_node_remove is fully done with the removal. But if it is in the 
> middle of removal, node is still seen as allocated outside and thread B 
> can enter the if-body, wait for the lock, and then drm_mm_node_remove 
> will attempt a double removal. So I think another drm_mm_node_allocated 
> under the lock is needed.

Yes. Check after the lock is indeed required in this scenario. And
drm_mm_node_remove() insists the caller doesn't try a double remove.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()
  2019-10-04 11:07     ` Chris Wilson
@ 2019-10-04 11:17       ` Chris Wilson
  2019-10-04 12:01         ` [Intel-gfx] " Tvrtko Ursulin
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2019-10-04 11:17 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx; +Cc: dri-devel

Quoting Chris Wilson (2019-10-04 12:07:10)
> Quoting Tvrtko Ursulin (2019-10-04 10:15:20)
> > 
> > On 03/10/2019 22:01, Chris Wilson wrote:
> > > A few callers need to serialise the destruction of their drm_mm_node and
> > > ensure it is removed from the drm_mm before freeing. However, to be
> > > completely sure that any access from another thread is complete before
> > > we free the struct, we require the RELEASE semantics of
> > > clear_bit_unlock().
> > > 
> > > This allows the conditional locking such as
> > > 
> > > Thread A                              Thread B
> > >      mutex_lock(mm_lock);                if (drm_mm_node_allocated(node)) {
> > >      drm_mm_node_remove(node);               mutex_lock(mm_lock);
> > >      mutex_unlock(mm_lock);                  drm_mm_node_remove(node);
> > >                                              mutex_unlock(mm_lock);
> > >                                           }
> > >                                           kfree(node);
> > 
> > My understanding is that release semantics on node allocated mean 1 -> 0 
> > transition is guaranteed to be seen only when thread A 
> > drm_mm_node_remove is fully done with the removal. But if it is in the 
> > middle of removal, node is still seen as allocated outside and thread B 
> > can enter the if-body, wait for the lock, and then drm_mm_node_remove 
> > will attempt a double removal. So I think another drm_mm_node_allocated 
> > under the lock is needed.
> 
> Yes. Check after the lock is indeed required in this scenario. And
> drm_mm_node_remove() insists the caller doesn't try a double remove.

I had to go back and double check the vma code, and that's fine.
(We hit this case where one thread is evicting and another thread is
destroying the object. And for us we do the check under the lock inside
__i915_vma_unbind() on the destroy path.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2)
  2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
                   ` (7 preceding siblings ...)
  2019-10-04 10:25 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2) Patchwork
@ 2019-10-04 11:36 ` Patchwork
  2019-10-04 11:53 ` ✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission Patchwork
  2019-10-04 17:01 ` ✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2) Patchwork
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-04 11:36 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2)
URL   : https://patchwork.freedesktop.org/series/67566/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7003 -> Patchwork_14665
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/index.html

Known issues
------------

  Here are the changes found in Patchwork_14665 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_create@basic-files:
    - fi-icl-u3:          [PASS][1] -> [INCOMPLETE][2] ([fdo#107713] / [fdo#109100])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/fi-icl-u3/igt@gem_ctx_create@basic-files.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/fi-icl-u3/igt@gem_ctx_create@basic-files.html

  * igt@gem_ctx_switch@legacy-render:
    - fi-icl-u2:          [PASS][3] -> [INCOMPLETE][4] ([fdo#107713] / [fdo#111381])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/fi-icl-u2/igt@gem_ctx_switch@legacy-render.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/fi-icl-u2/igt@gem_ctx_switch@legacy-render.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [INCOMPLETE][5] ([fdo#107718]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_sync@basic-all:
    - {fi-tgl-u}:         [INCOMPLETE][7] ([fdo#111880]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/fi-tgl-u/igt@gem_sync@basic-all.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/fi-tgl-u/igt@gem_sync@basic-all.html

  
#### Warnings ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][9] ([fdo#111045] / [fdo#111096]) -> [FAIL][10] ([fdo#111407])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111155]: https://bugs.freedesktop.org/show_bug.cgi?id=111155
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [fdo#111880]: https://bugs.freedesktop.org/show_bug.cgi?id=111880


Participating hosts (52 -> 42)
------------------------------

  Missing    (10): fi-ilk-m540 fi-hsw-4200u fi-skl-guc fi-byt-squawks fi-bsw-cyan fi-ilk-650 fi-byt-clapper fi-icl-y fi-icl-dsi fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7003 -> Patchwork_14665

  CI-20190529: 20190529
  CI_DRM_7003: d76b206244f3c13898e65433720bb524c85fb832 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5211: 1601e1571eb0f29a06b64494040b3ea7859a650f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14665: 0ad3aa61926dbd7c99721367e185dadc1a4c407f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0ad3aa61926d drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()
fe0c8958ee33 drm/mm: Convert drm_mm_node booleans to bitops
870ebb8b6d24 drm/mm: Use helpers for drm_mm_node booleans
d1690911537b dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
6adbc5589cd8 drm/i915/execlists: Skip redundant resubmission

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* ✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission
  2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
                   ` (8 preceding siblings ...)
  2019-10-04 11:36 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-10-04 11:53 ` Patchwork
  2019-10-04 17:01 ` ✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2) Patchwork
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-04 11:53 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915/execlists: Skip redundant resubmission
URL   : https://patchwork.freedesktop.org/series/67566/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7000_full -> Patchwork_14659_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14659_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14659_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14659_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_mmap_gtt@hang:
    - shard-kbl:          [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-kbl3/igt@gem_mmap_gtt@hang.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-kbl6/igt@gem_mmap_gtt@hang.html
    - shard-glk:          [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-glk5/igt@gem_mmap_gtt@hang.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-glk9/igt@gem_mmap_gtt@hang.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
    - {shard-tglb}:       [PASS][5] -> [INCOMPLETE][6] +2 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-tglb8/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-tglb3/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html

  
Known issues
------------

  Here are the changes found in Patchwork_14659_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl2/igt@gem_ctx_isolation@bcs0-s3.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-apl4/igt@gem_ctx_isolation@bcs0-s3.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [PASS][9] -> [SKIP][10] ([fdo#110841])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb6/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_schedule@promotion-bsd1:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#109276]) +12 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb1/igt@gem_exec_schedule@promotion-bsd1.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb7/igt@gem_exec_schedule@promotion-bsd1.html

  * igt@gem_exec_schedule@wide-bsd:
    - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#111325]) +3 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb5/igt@gem_exec_schedule@wide-bsd.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb2/igt@gem_exec_schedule@wide-bsd.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-apl:          [PASS][15] -> [DMESG-WARN][16] ([fdo#109385] / [fdo#111870])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl3/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-apl7/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
    - shard-kbl:          [PASS][17] -> [DMESG-WARN][18] ([fdo#111870]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-kbl4/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-kbl3/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
    - shard-iclb:         [PASS][19] -> [DMESG-WARN][20] ([fdo#111870]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb5/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html

  * igt@kms_cursor_legacy@cursora-vs-flipa-atomic-transitions-varying-size:
    - shard-snb:          [PASS][21] -> [SKIP][22] ([fdo#109271])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-snb1/igt@kms_cursor_legacy@cursora-vs-flipa-atomic-transitions-varying-size.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-snb1/igt@kms_cursor_legacy@cursora-vs-flipa-atomic-transitions-varying-size.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
    - shard-iclb:         [PASS][23] -> [FAIL][24] ([fdo#103167]) +3 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#108145])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109642] / [fdo#111068])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb8/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         [PASS][29] -> [SKIP][30] ([fdo#109441]) +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb5/igt@kms_psr@psr2_cursor_blt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][31] -> [FAIL][32] ([fdo#99912])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl1/igt@kms_setmode@basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-apl6/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-kbl:          [PASS][33] -> [INCOMPLETE][34] ([fdo#103665])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-kbl4/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-kbl3/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [DMESG-WARN][35] ([fdo#108566]) -> [PASS][36] +5 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl7/igt@gem_ctx_isolation@rcs0-s3.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-apl8/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_exec_schedule@preempt-bsd:
    - shard-iclb:         [SKIP][37] ([fdo#111325]) -> [PASS][38] +5 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@gem_exec_schedule@preempt-bsd.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb6/igt@gem_exec_schedule@preempt-bsd.html

  * igt@gem_mmap_gtt@hang:
    - shard-apl:          [DMESG-WARN][39] ([fdo#109385]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl3/igt@gem_mmap_gtt@hang.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-apl8/igt@gem_mmap_gtt@hang.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy:
    - shard-glk:          [DMESG-WARN][41] ([fdo#111870]) -> [PASS][42] +2 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-glk2/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-glk3/igt@gem_userptr_blits@map-fixed-invalidate-busy.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-snb:          [DMESG-WARN][43] ([fdo#111870]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-snb4/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-snb6/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
    - shard-skl:          [DMESG-WARN][45] ([fdo#111870]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl4/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-skl1/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@gem_userptr_blits@sync-unmap:
    - shard-hsw:          [DMESG-WARN][47] ([fdo#111870]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-hsw4/igt@gem_userptr_blits@sync-unmap.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-hsw7/igt@gem_userptr_blits@sync-unmap.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-iclb:         [DMESG-WARN][49] ([fdo#111870]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb8/igt@gem_userptr_blits@sync-unmap-cycles.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb6/igt@gem_userptr_blits@sync-unmap-cycles.html

  * {igt@i915_pm_dc@dc6-psr}:
    - shard-iclb:         [FAIL][51] ([fdo#110548]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@i915_pm_dc@dc6-psr.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb5/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_selftest@mock_fence:
    - {shard-tglb}:       [INCOMPLETE][53] ([fdo#111747]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-tglb4/igt@i915_selftest@mock_fence.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-tglb3/igt@i915_selftest@mock_fence.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding:
    - shard-snb:          [INCOMPLETE][55] ([fdo#105411]) -> [PASS][56] +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-snb1/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-snb7/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html

  * igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque:
    - {shard-tglb}:       [DMESG-WARN][57] ([fdo#111600]) -> [PASS][58] +1 similar issue
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-tglb2/igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-tglb8/igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque.html

  * igt@kms_flip@flip-vs-modeset-vs-hang:
    - shard-apl:          [INCOMPLETE][59] ([fdo#103927]) -> [PASS][60] +4 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-apl2/igt@kms_flip@flip-vs-modeset-vs-hang.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-apl7/igt@kms_flip@flip-vs-modeset-vs-hang.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          [INCOMPLETE][61] ([fdo#109507]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl6/igt@kms_flip@flip-vs-suspend.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-skl10/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [FAIL][63] ([fdo#103167]) -> [PASS][64] +3 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
    - {shard-tglb}:       [FAIL][65] ([fdo#103167]) -> [PASS][66] +2 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-tglb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-tglb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][67] ([fdo#108145] / [fdo#110403]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][69] ([fdo#103166]) -> [PASS][70]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb2/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [SKIP][71] ([fdo#109441]) -> [PASS][72]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb5/igt@kms_psr@psr2_sprite_plane_move.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - {shard-tglb}:       [INCOMPLETE][73] ([fdo#111832] / [fdo#111850]) -> [PASS][74]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-tglb4/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-tglb3/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
    - shard-skl:          [INCOMPLETE][75] ([fdo#104108]) -> [PASS][76]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl5/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-skl4/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html

  * igt@perf@polling:
    - shard-skl:          [FAIL][77] ([fdo#110728]) -> [PASS][78]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-skl10/igt@perf@polling.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-skl7/igt@perf@polling.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [SKIP][79] ([fdo#109276]) -> [PASS][80] +20 similar issues
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb7/igt@prime_busy@hang-bsd2.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb1/igt@prime_busy@hang-bsd2.html

  
#### Warnings ####

  * igt@gem_ctx_isolation@vcs1-nonpriv:
    - shard-iclb:         [FAIL][81] ([fdo#111329]) -> [SKIP][82] ([fdo#109276])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb2/igt@gem_ctx_isolation@vcs1-nonpriv.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb8/igt@gem_ctx_isolation@vcs1-nonpriv.html

  * igt@gem_mocs_settings@mocs-reset-dirty-render:
    - shard-iclb:         [INCOMPLETE][83] ([fdo#107713]) -> [SKIP][84] ([fdo#110206])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-iclb1/igt@gem_mocs_settings@mocs-reset-dirty-render.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-iclb8/igt@gem_mocs_settings@mocs-reset-dirty-render.html

  * igt@kms_atomic_transition@5x-modeset-transitions-nonblocking:
    - shard-snb:          [SKIP][85] ([fdo#109271] / [fdo#109278]) -> [SKIP][86] ([fdo#109271])
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7000/shard-snb1/igt@kms_atomic_transition@5x-modeset-transitions-nonblocking.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/shard-snb1/igt@kms_atomic_transition@5x-modeset-transitions-nonblocking.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109385]: https://bugs.freedesktop.org/show_bug.cgi?id=109385
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110206]: https://bugs.freedesktop.org/show_bug.cgi?id=110206
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110548]: https://bugs.freedesktop.org/show_bug.cgi?id=110548
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
  [fdo#111600]: https://bugs.freedesktop.org/show_bug.cgi?id=111600
  [fdo#111646]: https://bugs.freedesktop.org/show_bug.cgi?id=111646
  [fdo#111671]: https://bugs.freedesktop.org/show_bug.cgi?id=111671
  [fdo#111714]: https://bugs.freedesktop.org/show_bug.cgi?id=111714
  [fdo#111736]: https://bugs.freedesktop.org/show_bug.cgi?id=111736
  [fdo#111747]: https://bugs.freedesktop.org/show_bug.cgi?id=111747
  [fdo#111832]: https://bugs.freedesktop.org/show_bug.cgi?id=111832
  [fdo#111850]: https://bugs.freedesktop.org/show_bug.cgi?id=111850
  [fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7000 -> Patchwork_14659

  CI-20190529: 20190529
  CI_DRM_7000: a6af6b11a94cbffff1c70dbab04ef1e13d79e4ae @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5211: 1601e1571eb0f29a06b64494040b3ea7859a650f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14659: 45a8b0a2ec910228fa0ab99fdce0073fbfb8d9bc @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14659/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()
  2019-10-04 11:17       ` Chris Wilson
@ 2019-10-04 12:01         ` Tvrtko Ursulin
  2019-10-09 15:59           ` Daniel Vetter
  0 siblings, 1 reply; 22+ messages in thread
From: Tvrtko Ursulin @ 2019-10-04 12:01 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: dri-devel


On 04/10/2019 12:17, Chris Wilson wrote:
> Quoting Chris Wilson (2019-10-04 12:07:10)
>> Quoting Tvrtko Ursulin (2019-10-04 10:15:20)
>>>
>>> On 03/10/2019 22:01, Chris Wilson wrote:
>>>> A few callers need to serialise the destruction of their drm_mm_node and
>>>> ensure it is removed from the drm_mm before freeing. However, to be
>>>> completely sure that any access from another thread is complete before
>>>> we free the struct, we require the RELEASE semantics of
>>>> clear_bit_unlock().
>>>>
>>>> This allows the conditional locking such as
>>>>
>>>> Thread A                              Thread B
>>>>       mutex_lock(mm_lock);                if (drm_mm_node_allocated(node)) {
>>>>       drm_mm_node_remove(node);               mutex_lock(mm_lock);
>>>>       mutex_unlock(mm_lock);                  drm_mm_node_remove(node);
>>>>                                               mutex_unlock(mm_lock);
>>>>                                            }
>>>>                                            kfree(node);
>>>
>>> My understanding is that release semantics on node allocated mean 1 -> 0
>>> transition is guaranteed to be seen only when thread A
>>> drm_mm_node_remove is fully done with the removal. But if it is in the
>>> middle of removal, node is still seen as allocated outside and thread B
>>> can enter the if-body, wait for the lock, and then drm_mm_node_remove
>>> will attempt a double removal. So I think another drm_mm_node_allocated
>>> under the lock is needed.
>>
>> Yes. Check after the lock is indeed required in this scenario. And
>> drm_mm_node_remove() insists the caller doesn't try a double remove.
> 
> I had to go back and double check the vma code, and that's fine.
> (We hit this case where one thread is evicting and another thread is
> destroying the object. And for us we do the check under the lock inside
> __i915_vma_unbind() on the destroy path.)

So I think if you amend the commit message to contain the repeated check 
under the lock patch looks good to me. With that:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

* ✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2)
  2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
                   ` (9 preceding siblings ...)
  2019-10-04 11:53 ` ✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission Patchwork
@ 2019-10-04 17:01 ` Patchwork
  10 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2019-10-04 17:01 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2)
URL   : https://patchwork.freedesktop.org/series/67566/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7003_full -> Patchwork_14665_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14665_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14665_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14665_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_mmap_gtt@hang:
    - shard-kbl:          [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-kbl1/igt@gem_mmap_gtt@hang.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-kbl2/igt@gem_mmap_gtt@hang.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_mmap_gtt@hang:
    - {shard-tglb}:       NOTRUN -> [DMESG-WARN][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-tglb6/igt@gem_mmap_gtt@hang.html

  
Known issues
------------

  Here are the changes found in Patchwork_14665_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][4] -> [SKIP][5] ([fdo#110854])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb2/igt@gem_exec_balancer@smoke.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb6/igt@gem_exec_balancer@smoke.html

  * igt@gem_exec_schedule@preempt-queue-bsd1:
    - shard-iclb:         [PASS][6] -> [SKIP][7] ([fdo#109276]) +15 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb1/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb7/igt@gem_exec_schedule@preempt-queue-bsd1.html

  * igt@gem_exec_schedule@reorder-wide-bsd:
    - shard-iclb:         [PASS][8] -> [SKIP][9] ([fdo#111325]) +7 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb5/igt@gem_exec_schedule@reorder-wide-bsd.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb1/igt@gem_exec_schedule@reorder-wide-bsd.html

  * igt@gem_userptr_blits@dmabuf-unsync:
    - shard-snb:          [PASS][10] -> [DMESG-WARN][11] ([fdo#111870])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-snb5/igt@gem_userptr_blits@dmabuf-unsync.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-snb1/igt@gem_userptr_blits@dmabuf-unsync.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-apl:          [PASS][12] -> [DMESG-WARN][13] ([fdo#109385] / [fdo#111870]) +1 similar issue
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-apl4/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-apl1/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup:
    - shard-skl:          [PASS][14] -> [DMESG-WARN][15] ([fdo#111870]) +1 similar issue
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-skl3/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-skl10/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
    - shard-iclb:         [PASS][16] -> [DMESG-WARN][17] ([fdo#111870]) +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb5/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb1/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html

  * igt@gem_userptr_blits@sync-unmap:
    - shard-glk:          [PASS][18] -> [DMESG-WARN][19] ([fdo#111870])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-glk1/igt@gem_userptr_blits@sync-unmap.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-glk6/igt@gem_userptr_blits@sync-unmap.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [PASS][20] -> [DMESG-WARN][21] ([fdo#108566]) +2 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-apl1/igt@gem_workarounds@suspend-resume-context.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-apl4/igt@gem_workarounds@suspend-resume-context.html

  * igt@i915_pm_rpm@modeset-stress-extra-wait:
    - shard-glk:          [PASS][22] -> [DMESG-WARN][23] ([fdo#105763] / [fdo#106538])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-glk1/igt@i915_pm_rpm@modeset-stress-extra-wait.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-glk8/igt@i915_pm_rpm@modeset-stress-extra-wait.html

  * igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen:
    - shard-apl:          [PASS][24] -> [INCOMPLETE][25] ([fdo#103927]) +2 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-apl3/igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-apl3/igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
    - shard-hsw:          [PASS][26] -> [FAIL][27] ([fdo#105767])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-hsw7/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-snb:          [PASS][28] -> [INCOMPLETE][29] ([fdo#105411])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-snb5/igt@kms_flip@flip-vs-suspend.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-snb1/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [PASS][30] -> [FAIL][31] ([fdo#103167]) +5 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [PASS][32] -> [FAIL][33] ([fdo#108145])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-skl1/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [PASS][34] -> [SKIP][35] ([fdo#109441])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb6/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@kms_setmode@basic:
    - shard-glk:          [PASS][36] -> [FAIL][37] ([fdo#99912])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-glk3/igt@kms_setmode@basic.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-glk4/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_create@create-clear:
    - shard-hsw:          [INCOMPLETE][38] ([fdo#103540] / [fdo#110401]) -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-hsw5/igt@gem_create@create-clear.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-hsw1/igt@gem_create@create-clear.html

  * igt@gem_exec_schedule@fifo-bsd:
    - shard-iclb:         [SKIP][40] ([fdo#111325]) -> [PASS][41] +2 similar issues
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb4/igt@gem_exec_schedule@fifo-bsd.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb5/igt@gem_exec_schedule@fifo-bsd.html

  * igt@gem_exec_schedule@preempt-queue-bsd2:
    - shard-iclb:         [SKIP][42] ([fdo#109276]) -> [PASS][43] +9 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb8/igt@gem_exec_schedule@preempt-queue-bsd2.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb2/igt@gem_exec_schedule@preempt-queue-bsd2.html

  * igt@gem_mmap_gtt@hang:
    - shard-glk:          [DMESG-WARN][44] -> [PASS][45]
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-glk1/igt@gem_mmap_gtt@hang.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-glk6/igt@gem_mmap_gtt@hang.html

  * igt@gem_partial_pwrite_pread@writes-after-reads-display:
    - {shard-tglb}:       [INCOMPLETE][46] ([fdo#111714]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-tglb8/igt@gem_partial_pwrite_pread@writes-after-reads-display.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-tglb2/igt@gem_partial_pwrite_pread@writes-after-reads-display.html

  * igt@gem_set_tiling_vs_blt@tiled-to-tiled:
    - shard-iclb:         [INCOMPLETE][48] ([fdo#107713]) -> [PASS][49] +1 similar issue
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb7/igt@gem_set_tiling_vs_blt@tiled-to-tiled.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb3/igt@gem_set_tiling_vs_blt@tiled-to-tiled.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-hsw:          [DMESG-WARN][50] ([fdo#111870]) -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-hsw8/igt@gem_userptr_blits@dmabuf-sync.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-hsw8/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy:
    - shard-snb:          [DMESG-WARN][52] ([fdo#111870]) -> [PASS][53]
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-snb1/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-snb5/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
    - shard-glk:          [DMESG-WARN][54] ([fdo#111870]) -> [PASS][55] +1 similar issue
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-glk9/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-glk2/igt@gem_userptr_blits@map-fixed-invalidate-busy.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-kbl:          [DMESG-WARN][56] ([fdo#111870]) -> [PASS][57] +3 similar issues
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-kbl7/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-kbl4/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-iclb:         [DMESG-WARN][58] ([fdo#111870]) -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb5/igt@gem_userptr_blits@sync-unmap-cycles.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb1/igt@gem_userptr_blits@sync-unmap-cycles.html
    - shard-skl:          [DMESG-WARN][60] ([fdo#111870]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-skl3/igt@gem_userptr_blits@sync-unmap-cycles.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-skl10/igt@gem_userptr_blits@sync-unmap-cycles.html

  * {igt@i915_pm_dc@dc6-psr}:
    - shard-skl:          [FAIL][62] ([fdo#110548]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-skl4/igt@i915_pm_dc@dc6-psr.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-skl9/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_big_fb@y-tiled-32bpp-rotate-0:
    - shard-skl:          [INCOMPLETE][64] -> [PASS][65]
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-skl1/igt@kms_big_fb@y-tiled-32bpp-rotate-0.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-skl10/igt@kms_big_fb@y-tiled-32bpp-rotate-0.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding:
    - shard-kbl:          [INCOMPLETE][66] ([fdo#103665]) -> [PASS][67] +1 similar issue
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-kbl7/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-kbl3/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-hsw:          [INCOMPLETE][68] ([fdo#103540]) -> [PASS][69]
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-hsw6/igt@kms_flip@flip-vs-suspend-interruptible.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-hsw2/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-onoff:
    - {shard-tglb}:       [FAIL][70] ([fdo#103167]) -> [PASS][71] +9 similar issues
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-onoff.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-onoff.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         [FAIL][72] ([fdo#103167]) -> [PASS][73] +4 similar issues
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][74] ([fdo#108145]) -> [PASS][75]
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [FAIL][76] ([fdo#103166]) -> [PASS][77]
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb2/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [SKIP][78] ([fdo#109441]) -> [PASS][79] +3 similar issues
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb1/igt@kms_psr@psr2_cursor_render.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb2/igt@kms_psr@psr2_cursor_render.html

  * igt@kms_psr@suspend:
    - {shard-tglb}:       [INCOMPLETE][80] ([fdo#111832]) -> [PASS][81] +3 similar issues
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-tglb4/igt@kms_psr@suspend.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-tglb7/igt@kms_psr@suspend.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-skl:          [INCOMPLETE][82] ([fdo#104108]) -> [PASS][83]
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-skl6/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-skl8/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
    - {shard-tglb}:       [INCOMPLETE][84] -> [PASS][85]
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-tglb1/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-tglb8/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-apl:          [DMESG-WARN][86] ([fdo#108566]) -> [PASS][87]
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-apl8/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-apl6/igt@kms_vblank@pipe-c-ts-continuation-suspend.html

  * igt@prime_vgem@sync-render:
    - {shard-tglb}:       [DMESG-WARN][88] ([fdo#111600]) -> [PASS][89]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-tglb4/igt@prime_vgem@sync-render.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-tglb3/igt@prime_vgem@sync-render.html

  * igt@tools_test@tools_test:
    - shard-hsw:          [SKIP][90] ([fdo#109271]) -> [PASS][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-hsw7/igt@tools_test@tools_test.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-hsw6/igt@tools_test@tools_test.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-isolation-bsd2:
    - shard-iclb:         [SKIP][92] ([fdo#109276]) -> [FAIL][93] ([fdo#111330])
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb5/igt@gem_mocs_settings@mocs-isolation-bsd2.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb1/igt@gem_mocs_settings@mocs-isolation-bsd2.html

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [FAIL][94] ([fdo#111330]) -> [SKIP][95] ([fdo#109276])
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-iclb1/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-iclb7/igt@gem_mocs_settings@mocs-reset-bsd2.html

  * igt@gem_softpin@noreloc-s3:
    - shard-apl:          [INCOMPLETE][96] ([fdo#103927]) -> [DMESG-WARN][97] ([fdo#108566])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-apl7/igt@gem_softpin@noreloc-s3.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-apl4/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@dmabuf-unsync:
    - shard-hsw:          [DMESG-WARN][98] ([fdo#110789] / [fdo#111870]) -> [DMESG-WARN][99] ([fdo#111870])
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-hsw1/igt@gem_userptr_blits@dmabuf-unsync.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-hsw5/igt@gem_userptr_blits@dmabuf-unsync.html

  * igt@gem_userptr_blits@map-fixed-invalidate-busy-gup:
    - shard-snb:          [DMESG-WARN][100] ([fdo#111870]) -> [DMESG-WARN][101] ([fdo#110789] / [fdo#111870])
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-snb4/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-snb4/igt@gem_userptr_blits@map-fixed-invalidate-busy-gup.html

  * igt@gem_userptr_blits@sync-unmap-after-close:
    - shard-hsw:          [DMESG-WARN][102] ([fdo#111870]) -> [DMESG-WARN][103] ([fdo#110789] / [fdo#111870])
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7003/shard-hsw2/igt@gem_userptr_blits@sync-unmap-after-close.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/shard-hsw7/igt@gem_userptr_blits@sync-unmap-after-close.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103166]: https:

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14665/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Intel-gfx] [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()
  2019-10-04 12:01         ` [Intel-gfx] " Tvrtko Ursulin
@ 2019-10-09 15:59           ` Daniel Vetter
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2019-10-09 15:59 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx, dri-devel

On Fri, Oct 04, 2019 at 01:01:36PM +0100, Tvrtko Ursulin wrote:
> 
> On 04/10/2019 12:17, Chris Wilson wrote:
> > Quoting Chris Wilson (2019-10-04 12:07:10)
> > > Quoting Tvrtko Ursulin (2019-10-04 10:15:20)
> > > > 
> > > > On 03/10/2019 22:01, Chris Wilson wrote:
> > > > > A few callers need to serialise the destruction of their drm_mm_node and
> > > > > ensure it is removed from the drm_mm before freeing. However, to be
> > > > > completely sure that any access from another thread is complete before
> > > > > we free the struct, we require the RELEASE semantics of
> > > > > clear_bit_unlock().
> > > > > 
> > > > > This allows the conditional locking such as
> > > > > 
> > > > > Thread A                              Thread B
> > > > >       mutex_lock(mm_lock);                if (drm_mm_node_allocated(node)) {
> > > > >       drm_mm_node_remove(node);               mutex_lock(mm_lock);
> > > > >       mutex_unlock(mm_lock);                  drm_mm_node_remove(node);
> > > > >                                               mutex_unlock(mm_lock);
> > > > >                                            }
> > > > >                                            kfree(node);
> > > > 
> > > > My understanding is that release semantics on node allocated mean 1 -> 0
> > > > transition is guaranteed to be seen only when thread A
> > > > drm_mm_node_remove is fully done with the removal. But if it is in the
> > > > middle of removal, node is still seen as allocated outside and thread B
> > > > can enter the if-body, wait for the lock, and then drm_mm_node_remove
> > > > will attempt a double removal. So I think another drm_mm_node_allocated
> > > > under the lock is needed.
> > > 
> > > Yes. Check after the lock is indeed required in this scenario. And
> > > drm_mm_node_remove() insists the caller doesn't try a double remove.
> > 
> > I had to go back and double check the vma code, and that's fine.
> > (We hit this case where one thread is evicting and another thread is
> > destroying the object. And for us we do the check under the lock inside
> > __i915_vma_unbind() on the destroy path.)
> 
> So I think if you amend the commit message to contain the repeated check
> under the lock patch looks good to me. With that:
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

I think a follow-up patch to update the kerneldoc to mention that we're
guaranteeing this now is missing here (best with the above fixed example).
Plus maybe a oneline code comment for the ALLOCATED_BIT.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2019-10-09 15:59 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-03 21:00 [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission Chris Wilson
2019-10-03 21:00 ` [PATCH 2/5] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling) Chris Wilson
2019-10-04  8:31   ` Tvrtko Ursulin
2019-10-04 10:11   ` [PATCH] " Chris Wilson
2019-10-04 10:18     ` Tvrtko Ursulin
2019-10-03 21:00 ` [PATCH 3/5] drm/mm: Use helpers for drm_mm_node booleans Chris Wilson
2019-10-04  8:33   ` Tvrtko Ursulin
2019-10-03 21:00 ` [PATCH 4/5] drm/mm: Convert drm_mm_node booleans to bitops Chris Wilson
2019-10-04  8:34   ` Tvrtko Ursulin
2019-10-03 21:01 ` [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node() Chris Wilson
2019-10-04  9:15   ` Tvrtko Ursulin
2019-10-04 11:07     ` Chris Wilson
2019-10-04 11:17       ` Chris Wilson
2019-10-04 12:01         ` [Intel-gfx] " Tvrtko Ursulin
2019-10-09 15:59           ` Daniel Vetter
2019-10-03 22:38 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission Patchwork
2019-10-03 22:58 ` ✓ Fi.CI.BAT: success " Patchwork
2019-10-04 10:08 ` [PATCH 1/5] " Tvrtko Ursulin
2019-10-04 10:25 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2) Patchwork
2019-10-04 11:36 ` ✓ Fi.CI.BAT: success " Patchwork
2019-10-04 11:53 ` ✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission Patchwork
2019-10-04 17:01 ` ✗ Fi.CI.IGT: failure for series starting with [1/5] drm/i915/execlists: Skip redundant resubmission (rev2) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.