[PATCH] dma-buf: Restore seqlock around dma_resv updates

[PATCH] dma-buf: Restore seqlock around dma_resv updates

[Chris Wilson]

This reverts
67c97fb79a7f ("dma-buf: add reservation_object_fences helper")
dd7a7d1ff2f1 ("drm/i915: use new reservation_object_fences helper")
0e1d8083bddb ("dma-buf: further relax reservation_object_add_shared_fence")
5d344f58da76 ("dma-buf: nuke reservation_object seq number")

The scenario that defeats simply grabbing a set of shared/exclusive
fences and using them blissfully under RCU is that any of those fences
may be reallocated by a SLAB_TYPESAFE_BY_RCU fence slab cache. In this
scenario, while keeping the rcu_read_lock we need to establish that no
fence was changed in the dma_resv after a read (or full) memory barrier.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/dma-buf.c                     |  31 ++++-
 drivers/dma-buf/dma-resv.c                    | 109 ++++++++++++-----
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |   7 +-
 drivers/gpu/drm/i915/gem/i915_gem_busy.c      |  24 ++--
 include/linux/dma-resv.h                      | 113 ++++++++----------
 5 files changed, 175 insertions(+), 109 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index b3400d6524ab..433d91d710e4 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -199,7 +199,7 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
 	struct dma_resv_list *fobj;
 	struct dma_fence *fence_excl;
 	__poll_t events;
-	unsigned shared_count;
+	unsigned shared_count, seq;
 
 	dmabuf = file->private_data;
 	if (!dmabuf || !dmabuf->resv)
@@ -213,8 +213,21 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
 	if (!events)
 		return 0;
 
+retry:
+	seq = read_seqcount_begin(&resv->seq);
 	rcu_read_lock();
-	dma_resv_fences(resv, &fence_excl, &fobj, &shared_count);
+
+	fobj = rcu_dereference(resv->fence);
+	if (fobj)
+		shared_count = fobj->shared_count;
+	else
+		shared_count = 0;
+	fence_excl = rcu_dereference(resv->fence_excl);
+	if (read_seqcount_retry(&resv->seq, seq)) {
+		rcu_read_unlock();
+		goto retry;
+	}
+
 	if (fence_excl && (!(events & EPOLLOUT) || shared_count == 0)) {
 		struct dma_buf_poll_cb_t *dcb = &dmabuf->cb_excl;
 		__poll_t pevents = EPOLLIN;
@@ -1144,6 +1157,7 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused)
 	struct dma_resv *robj;
 	struct dma_resv_list *fobj;
 	struct dma_fence *fence;
+	unsigned seq;
 	int count = 0, attach_count, shared_count, i;
 	size_t size = 0;
 
@@ -1174,9 +1188,16 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused)
 				buf_obj->name ?: "");
 
 		robj = buf_obj->resv;
-		rcu_read_lock();
-		dma_resv_fences(robj, &fence, &fobj, &shared_count);
-		rcu_read_unlock();
+		while (true) {
+			seq = read_seqcount_begin(&robj->seq);
+			rcu_read_lock();
+			fobj = rcu_dereference(robj->fence);
+			shared_count = fobj ? fobj->shared_count : 0;
+			fence = rcu_dereference(robj->fence_excl);
+			if (!read_seqcount_retry(&robj->seq, seq))
+				break;
+			rcu_read_unlock();
+		}
 
 		if (fence)
 			seq_printf(s, "\tExclusive fence: %s %s %ssignalled\n",
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index f5142683c851..42a8f3f11681 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -49,6 +49,12 @@
 DEFINE_WD_CLASS(reservation_ww_class);
 EXPORT_SYMBOL(reservation_ww_class);
 
+struct lock_class_key reservation_seqcount_class;
+EXPORT_SYMBOL(reservation_seqcount_class);
+
+const char reservation_seqcount_string[] = "reservation_seqcount";
+EXPORT_SYMBOL(reservation_seqcount_string);
+
 /**
  * dma_resv_list_alloc - allocate fence list
  * @shared_max: number of fences we need space for
@@ -96,6 +102,9 @@ static void dma_resv_list_free(struct dma_resv_list *list)
 void dma_resv_init(struct dma_resv *obj)
 {
 	ww_mutex_init(&obj->lock, &reservation_ww_class);
+
+	__seqcount_init(&obj->seq, reservation_seqcount_string,
+			&reservation_seqcount_class);
 	RCU_INIT_POINTER(obj->fence, NULL);
 	RCU_INIT_POINTER(obj->fence_excl, NULL);
 }
@@ -225,6 +234,9 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence)
 	fobj = dma_resv_get_list(obj);
 	count = fobj->shared_count;
 
+	preempt_disable();
+	write_seqcount_begin(&obj->seq);
+
 	for (i = 0; i < count; ++i) {
 
 		old = rcu_dereference_protected(fobj->shared[i],
@@ -242,6 +254,9 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence)
 	RCU_INIT_POINTER(fobj->shared[i], fence);
 	/* pointer update must be visible before we extend the shared_count */
 	smp_store_mb(fobj->shared_count, count);
+
+	write_seqcount_end(&obj->seq);
+	preempt_enable();
 	dma_fence_put(old);
 }
 EXPORT_SYMBOL(dma_resv_add_shared_fence);
@@ -269,10 +284,12 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence)
 		dma_fence_get(fence);
 
 	preempt_disable();
-	rcu_assign_pointer(obj->fence_excl, fence);
-	/* pointer update must be visible before we modify the shared_count */
+	write_seqcount_begin(&obj->seq);
+	/* write_seqcount_begin provides the necessary memory barrier */
+	RCU_INIT_POINTER(obj->fence_excl, fence);
 	if (old)
-		smp_store_mb(old->shared_count, 0);
+		old->shared_count = 0;
+	write_seqcount_end(&obj->seq);
 	preempt_enable();
 
 	/* inplace update, no shared fences */
@@ -295,15 +312,17 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
 {
 	struct dma_resv_list *src_list, *dst_list;
 	struct dma_fence *old, *new;
-	unsigned int i, shared_count;
+	unsigned i;
 
 	dma_resv_assert_held(dst);
 
 	rcu_read_lock();
+	src_list = rcu_dereference(src->fence);
 
 retry:
-	dma_resv_fences(src, &new, &src_list, &shared_count);
-	if (shared_count) {
+	if (src_list) {
+		unsigned shared_count = src_list->shared_count;
+
 		rcu_read_unlock();
 
 		dst_list = dma_resv_list_alloc(shared_count);
@@ -311,14 +330,14 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
 			return -ENOMEM;
 
 		rcu_read_lock();
-		dma_resv_fences(src, &new, &src_list, &shared_count);
-		if (!src_list || shared_count > dst_list->shared_max) {
+		src_list = rcu_dereference(src->fence);
+		if (!src_list || src_list->shared_count > shared_count) {
 			kfree(dst_list);
 			goto retry;
 		}
 
 		dst_list->shared_count = 0;
-		for (i = 0; i < shared_count; ++i) {
+		for (i = 0; i < src_list->shared_count; ++i) {
 			struct dma_fence *fence;
 
 			fence = rcu_dereference(src_list->shared[i]);
@@ -328,6 +347,7 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
 
 			if (!dma_fence_get_rcu(fence)) {
 				dma_resv_list_free(dst_list);
+				src_list = rcu_dereference(src->fence);
 				goto retry;
 			}
 
@@ -342,18 +362,18 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
 		dst_list = NULL;
 	}
 
-	if (new && !dma_fence_get_rcu(new)) {
-		dma_resv_list_free(dst_list);
-		goto retry;
-	}
+	new = dma_fence_get_rcu_safe(&src->fence_excl);
 	rcu_read_unlock();
 
 	src_list = dma_resv_get_list(dst);
 	old = dma_resv_get_excl(dst);
 
 	preempt_disable();
-	rcu_assign_pointer(dst->fence_excl, new);
-	rcu_assign_pointer(dst->fence, dst_list);
+	write_seqcount_begin(&dst->seq);
+	/* write_seqcount_begin provides the necessary memory barrier */
+	RCU_INIT_POINTER(dst->fence_excl, new);
+	RCU_INIT_POINTER(dst->fence, dst_list);
+	write_seqcount_end(&dst->seq);
 	preempt_enable();
 
 	dma_resv_list_free(src_list);
@@ -388,18 +408,19 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
 
 	do {
 		struct dma_resv_list *fobj;
-		unsigned int i;
+		unsigned int i, seq;
 		size_t sz = 0;
 
-		i = 0;
+		shared_count = i = 0;
 
 		rcu_read_lock();
-		dma_resv_fences(obj, &fence_excl, &fobj,
-					  &shared_count);
+		seq = read_seqcount_begin(&obj->seq);
 
+		fence_excl = rcu_dereference(obj->fence_excl);
 		if (fence_excl && !dma_fence_get_rcu(fence_excl))
 			goto unlock;
 
+		fobj = rcu_dereference(obj->fence);
 		if (fobj)
 			sz += sizeof(*shared) * fobj->shared_max;
 
@@ -427,6 +448,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
 				break;
 			}
 			shared = nshared;
+			shared_count = fobj ? fobj->shared_count : 0;
 			for (i = 0; i < shared_count; ++i) {
 				shared[i] = rcu_dereference(fobj->shared[i]);
 				if (!dma_fence_get_rcu(shared[i]))
@@ -434,7 +456,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
 			}
 		}
 
-		if (i != shared_count) {
+		if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) {
 			while (i--)
 				dma_fence_put(shared[i]);
 			dma_fence_put(fence_excl);
@@ -478,17 +500,18 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
 			       bool wait_all, bool intr,
 			       unsigned long timeout)
 {
-	struct dma_resv_list *fobj;
 	struct dma_fence *fence;
-	unsigned shared_count;
+	unsigned seq, shared_count;
 	long ret = timeout ? timeout : 1;
 	int i;
 
 retry:
+	shared_count = 0;
+	seq = read_seqcount_begin(&obj->seq);
 	rcu_read_lock();
 	i = -1;
 
-	dma_resv_fences(obj, &fence, &fobj, &shared_count);
+	fence = rcu_dereference(obj->fence_excl);
 	if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
 		if (!dma_fence_get_rcu(fence))
 			goto unlock_retry;
@@ -503,6 +526,11 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
 	}
 
 	if (wait_all) {
+		struct dma_resv_list *fobj = rcu_dereference(obj->fence);
+
+		if (fobj)
+			shared_count = fobj->shared_count;
+
 		for (i = 0; !fence && i < shared_count; ++i) {
 			struct dma_fence *lfence = rcu_dereference(fobj->shared[i]);
 
@@ -525,6 +553,11 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
 
 	rcu_read_unlock();
 	if (fence) {
+		if (read_seqcount_retry(&obj->seq, seq)) {
+			dma_fence_put(fence);
+			goto retry;
+		}
+
 		ret = dma_fence_wait_timeout(fence, intr, ret);
 		dma_fence_put(fence);
 		if (ret > 0 && wait_all && (i + 1 < shared_count))
@@ -567,19 +600,23 @@ static inline int dma_resv_test_signaled_single(struct dma_fence *passed_fence)
  */
 bool dma_resv_test_signaled_rcu(struct dma_resv *obj, bool test_all)
 {
-	struct dma_resv_list *fobj;
-	struct dma_fence *fence_excl;
-	unsigned shared_count;
+	unsigned seq, shared_count;
 	int ret;
 
 	rcu_read_lock();
 retry:
 	ret = true;
+	shared_count = 0;
+	seq = read_seqcount_begin(&obj->seq);
 
-	dma_resv_fences(obj, &fence_excl, &fobj, &shared_count);
 	if (test_all) {
 		unsigned i;
 
+		struct dma_resv_list *fobj = rcu_dereference(obj->fence);
+
+		if (fobj)
+			shared_count = fobj->shared_count;
+
 		for (i = 0; i < shared_count; ++i) {
 			struct dma_fence *fence = rcu_dereference(fobj->shared[i]);
 
@@ -589,14 +626,24 @@ bool dma_resv_test_signaled_rcu(struct dma_resv *obj, bool test_all)
 			else if (!ret)
 				break;
 		}
-	}
 
-	if (!shared_count && fence_excl) {
-		ret = dma_resv_test_signaled_single(fence_excl);
-		if (ret < 0)
+		if (read_seqcount_retry(&obj->seq, seq))
 			goto retry;
 	}
 
+	if (!shared_count) {
+		struct dma_fence *fence_excl = rcu_dereference(obj->fence_excl);
+
+		if (fence_excl) {
+			ret = dma_resv_test_signaled_single(fence_excl);
+			if (ret < 0)
+				goto retry;
+
+			if (read_seqcount_retry(&obj->seq, seq))
+				goto retry;
+		}
+	}
+
 	rcu_read_unlock();
 	return ret;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index bc4ec6b20a87..76e3516484e7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -251,7 +251,12 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo,
 	new->shared_max = old->shared_max;
 	new->shared_count = k;
 
-	rcu_assign_pointer(resv->fence, new);
+	/* Install the new fence list, seqcount provides the barriers */
+	preempt_disable();
+	write_seqcount_begin(&resv->seq);
+	RCU_INIT_POINTER(resv->fence, new);
+	write_seqcount_end(&resv->seq);
+	preempt_enable();
 
 	/* Drop the references to the removed fences or move them to ef_list */
 	for (i = j, k = 0; i < old->shared_count; ++i) {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_busy.c b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
index a2aff1d8290e..3d4f5775a4ba 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
@@ -83,8 +83,7 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
 	struct drm_i915_gem_busy *args = data;
 	struct drm_i915_gem_object *obj;
 	struct dma_resv_list *list;
-	unsigned int i, shared_count;
-	struct dma_fence *excl;
+	unsigned int seq;
 	int err;
 
 	err = -ENOENT;
@@ -110,18 +109,29 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
 	 * to report the overall busyness. This is what the wait-ioctl does.
 	 *
 	 */
-	dma_resv_fences(obj->base.resv, &excl, &list, &shared_count);
+retry:
+	seq = raw_read_seqcount(&obj->base.resv->seq);
 
 	/* Translate the exclusive fence to the READ *and* WRITE engine */
-	args->busy = busy_check_writer(excl);
+	args->busy =
+		busy_check_writer(rcu_dereference(obj->base.resv->fence_excl));
 
 	/* Translate shared fences to READ set of engines */
-	for (i = 0; i < shared_count; ++i) {
-		struct dma_fence *fence = rcu_dereference(list->shared[i]);
+	list = rcu_dereference(obj->base.resv->fence);
+	if (list) {
+		unsigned int shared_count = list->shared_count, i;
 
-		args->busy |= busy_check_reader(fence);
+		for (i = 0; i < shared_count; ++i) {
+			struct dma_fence *fence =
+				rcu_dereference(list->shared[i]);
+
+			args->busy |= busy_check_reader(fence);
+		}
 	}
 
+	if (args->busy && read_seqcount_retry(&obj->base.resv->seq, seq))
+		goto retry;
+
 	err = 0;
 out:
 	rcu_read_unlock();
diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
index 38f2802afabb..ee50d10f052b 100644
--- a/include/linux/dma-resv.h
+++ b/include/linux/dma-resv.h
@@ -46,6 +46,8 @@
 #include <linux/rcupdate.h>
 
 extern struct ww_class reservation_ww_class;
+extern struct lock_class_key reservation_seqcount_class;
+extern const char reservation_seqcount_string[];
 
 /**
  * struct dma_resv_list - a list of shared fences
@@ -69,6 +71,7 @@ struct dma_resv_list {
  */
 struct dma_resv {
 	struct ww_mutex lock;
+	seqcount_t seq;
 
 	struct dma_fence __rcu *fence_excl;
 	struct dma_resv_list __rcu *fence;
@@ -77,24 +80,6 @@ struct dma_resv {
 #define dma_resv_held(obj) lockdep_is_held(&(obj)->lock.base)
 #define dma_resv_assert_held(obj) lockdep_assert_held(&(obj)->lock.base)
 
-/**
- * dma_resv_get_excl - get the reservation object's
- * exclusive fence, with update-side lock held
- * @obj: the reservation object
- *
- * Returns the exclusive fence (if any).  Does NOT take a
- * reference. Writers must hold obj->lock, readers may only
- * hold a RCU read side lock.
- *
- * RETURNS
- * The exclusive fence or NULL
- */
-static inline struct dma_fence *dma_resv_get_excl(struct dma_resv *obj)
-{
-	return rcu_dereference_protected(obj->fence_excl,
-					 dma_resv_held(obj));
-}
-
 /**
  * dma_resv_get_list - get the reservation object's
  * shared fence list, with update-side lock held
@@ -109,53 +94,6 @@ static inline struct dma_resv_list *dma_resv_get_list(struct dma_resv *obj)
 					 dma_resv_held(obj));
 }
 
-/**
- * dma_resv_fences - read consistent fence pointers
- * @obj: reservation object where we get the fences from
- * @excl: pointer for the exclusive fence
- * @list: pointer for the shared fence list
- *
- * Make sure we have a consisten exclusive fence and shared fence list.
- * Must be called with rcu read side lock held.
- */
-static inline void dma_resv_fences(struct dma_resv *obj,
-				   struct dma_fence **excl,
-				   struct dma_resv_list **list,
-				   u32 *shared_count)
-{
-	do {
-		*excl = rcu_dereference(obj->fence_excl);
-		*list = rcu_dereference(obj->fence);
-		*shared_count = *list ? (*list)->shared_count : 0;
-		smp_rmb(); /* See dma_resv_add_excl_fence */
-	} while (rcu_access_pointer(obj->fence_excl) != *excl);
-}
-
-/**
- * dma_resv_get_excl_rcu - get the reservation object's
- * exclusive fence, without lock held.
- * @obj: the reservation object
- *
- * If there is an exclusive fence, this atomically increments it's
- * reference count and returns it.
- *
- * RETURNS
- * The exclusive fence or NULL if none
- */
-static inline struct dma_fence *dma_resv_get_excl_rcu(struct dma_resv *obj)
-{
-	struct dma_fence *fence;
-
-	if (!rcu_access_pointer(obj->fence_excl))
-		return NULL;
-
-	rcu_read_lock();
-	fence = dma_fence_get_rcu_safe(&obj->fence_excl);
-	rcu_read_unlock();
-
-	return fence;
-}
-
 /**
  * dma_resv_lock - lock the reservation object
  * @obj: the reservation object
@@ -290,6 +228,51 @@ static inline void dma_resv_unlock(struct dma_resv *obj)
 	ww_mutex_unlock(&obj->lock);
 }
 
+/**
+ * dma_resv_get_excl - get the reservation object's
+ * exclusive fence, with update-side lock held
+ * @obj: the reservation object
+ *
+ * Returns the exclusive fence (if any).  Does NOT take a
+ * reference. Writers must hold obj->lock, readers may only
+ * hold a RCU read side lock.
+ *
+ * RETURNS
+ * The exclusive fence or NULL
+ */
+static inline struct dma_fence *
+dma_resv_get_excl(struct dma_resv *obj)
+{
+	return rcu_dereference_protected(obj->fence_excl,
+					 dma_resv_held(obj));
+}
+
+/**
+ * dma_resv_get_excl_rcu - get the reservation object's
+ * exclusive fence, without lock held.
+ * @obj: the reservation object
+ *
+ * If there is an exclusive fence, this atomically increments it's
+ * reference count and returns it.
+ *
+ * RETURNS
+ * The exclusive fence or NULL if none
+ */
+static inline struct dma_fence *
+dma_resv_get_excl_rcu(struct dma_resv *obj)
+{
+	struct dma_fence *fence;
+
+	if (!rcu_access_pointer(obj->fence_excl))
+		return NULL;
+
+	rcu_read_lock();
+	fence = dma_fence_get_rcu_safe(&obj->fence_excl);
+	rcu_read_unlock();
+
+	return fence;
+}
+
 void dma_resv_init(struct dma_resv *obj);
 void dma_resv_fini(struct dma_resv *obj);
 int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences);
-- 
2.23.0.rc1

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

Re: [PATCH] dma-buf: Restore seqlock around dma_resv updates

[Chris Wilson]

Quoting Chris Wilson (2019-08-14 19:24:01)
> This reverts
> 67c97fb79a7f ("dma-buf: add reservation_object_fences helper")
> dd7a7d1ff2f1 ("drm/i915: use new reservation_object_fences helper")
> 0e1d8083bddb ("dma-buf: further relax reservation_object_add_shared_fence")
> 5d344f58da76 ("dma-buf: nuke reservation_object seq number")
> 
> The scenario that defeats simply grabbing a set of shared/exclusive
> fences and using them blissfully under RCU is that any of those fences
> may be reallocated by a SLAB_TYPESAFE_BY_RCU fence slab cache. In this
> scenario, while keeping the rcu_read_lock we need to establish that no
> fence was changed in the dma_resv after a read (or full) memory barrier.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

I said I needed to go lie down, that proves it.

Cc: Christian König <christian.koenig@amd.com>

> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/dma-buf/dma-buf.c                     |  31 ++++-
>  drivers/dma-buf/dma-resv.c                    | 109 ++++++++++++-----
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |   7 +-
>  drivers/gpu/drm/i915/gem/i915_gem_busy.c      |  24 ++--
>  include/linux/dma-resv.h                      | 113 ++++++++----------
>  5 files changed, 175 insertions(+), 109 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index b3400d6524ab..433d91d710e4 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -199,7 +199,7 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>         struct dma_resv_list *fobj;
>         struct dma_fence *fence_excl;
>         __poll_t events;
> -       unsigned shared_count;
> +       unsigned shared_count, seq;
>  
>         dmabuf = file->private_data;
>         if (!dmabuf || !dmabuf->resv)
> @@ -213,8 +213,21 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>         if (!events)
>                 return 0;
>  
> +retry:
> +       seq = read_seqcount_begin(&resv->seq);
>         rcu_read_lock();
> -       dma_resv_fences(resv, &fence_excl, &fobj, &shared_count);
> +
> +       fobj = rcu_dereference(resv->fence);
> +       if (fobj)
> +               shared_count = fobj->shared_count;
> +       else
> +               shared_count = 0;
> +       fence_excl = rcu_dereference(resv->fence_excl);
> +       if (read_seqcount_retry(&resv->seq, seq)) {
> +               rcu_read_unlock();
> +               goto retry;
> +       }
> +
>         if (fence_excl && (!(events & EPOLLOUT) || shared_count == 0)) {
>                 struct dma_buf_poll_cb_t *dcb = &dmabuf->cb_excl;
>                 __poll_t pevents = EPOLLIN;
> @@ -1144,6 +1157,7 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused)
>         struct dma_resv *robj;
>         struct dma_resv_list *fobj;
>         struct dma_fence *fence;
> +       unsigned seq;
>         int count = 0, attach_count, shared_count, i;
>         size_t size = 0;
>  
> @@ -1174,9 +1188,16 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused)
>                                 buf_obj->name ?: "");
>  
>                 robj = buf_obj->resv;
> -               rcu_read_lock();
> -               dma_resv_fences(robj, &fence, &fobj, &shared_count);
> -               rcu_read_unlock();
> +               while (true) {
> +                       seq = read_seqcount_begin(&robj->seq);
> +                       rcu_read_lock();
> +                       fobj = rcu_dereference(robj->fence);
> +                       shared_count = fobj ? fobj->shared_count : 0;
> +                       fence = rcu_dereference(robj->fence_excl);
> +                       if (!read_seqcount_retry(&robj->seq, seq))
> +                               break;
> +                       rcu_read_unlock();
> +               }
>  
>                 if (fence)
>                         seq_printf(s, "\tExclusive fence: %s %s %ssignalled\n",
> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> index f5142683c851..42a8f3f11681 100644
> --- a/drivers/dma-buf/dma-resv.c
> +++ b/drivers/dma-buf/dma-resv.c
> @@ -49,6 +49,12 @@
>  DEFINE_WD_CLASS(reservation_ww_class);
>  EXPORT_SYMBOL(reservation_ww_class);
>  
> +struct lock_class_key reservation_seqcount_class;
> +EXPORT_SYMBOL(reservation_seqcount_class);
> +
> +const char reservation_seqcount_string[] = "reservation_seqcount";
> +EXPORT_SYMBOL(reservation_seqcount_string);
> +
>  /**
>   * dma_resv_list_alloc - allocate fence list
>   * @shared_max: number of fences we need space for
> @@ -96,6 +102,9 @@ static void dma_resv_list_free(struct dma_resv_list *list)
>  void dma_resv_init(struct dma_resv *obj)
>  {
>         ww_mutex_init(&obj->lock, &reservation_ww_class);
> +
> +       __seqcount_init(&obj->seq, reservation_seqcount_string,
> +                       &reservation_seqcount_class);
>         RCU_INIT_POINTER(obj->fence, NULL);
>         RCU_INIT_POINTER(obj->fence_excl, NULL);
>  }
> @@ -225,6 +234,9 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence)
>         fobj = dma_resv_get_list(obj);
>         count = fobj->shared_count;
>  
> +       preempt_disable();
> +       write_seqcount_begin(&obj->seq);
> +
>         for (i = 0; i < count; ++i) {
>  
>                 old = rcu_dereference_protected(fobj->shared[i],
> @@ -242,6 +254,9 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence)
>         RCU_INIT_POINTER(fobj->shared[i], fence);
>         /* pointer update must be visible before we extend the shared_count */
>         smp_store_mb(fobj->shared_count, count);
> +
> +       write_seqcount_end(&obj->seq);
> +       preempt_enable();
>         dma_fence_put(old);
>  }
>  EXPORT_SYMBOL(dma_resv_add_shared_fence);
> @@ -269,10 +284,12 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence)
>                 dma_fence_get(fence);
>  
>         preempt_disable();
> -       rcu_assign_pointer(obj->fence_excl, fence);
> -       /* pointer update must be visible before we modify the shared_count */
> +       write_seqcount_begin(&obj->seq);
> +       /* write_seqcount_begin provides the necessary memory barrier */
> +       RCU_INIT_POINTER(obj->fence_excl, fence);
>         if (old)
> -               smp_store_mb(old->shared_count, 0);
> +               old->shared_count = 0;
> +       write_seqcount_end(&obj->seq);
>         preempt_enable();
>  
>         /* inplace update, no shared fences */
> @@ -295,15 +312,17 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
>  {
>         struct dma_resv_list *src_list, *dst_list;
>         struct dma_fence *old, *new;
> -       unsigned int i, shared_count;
> +       unsigned i;
>  
>         dma_resv_assert_held(dst);
>  
>         rcu_read_lock();
> +       src_list = rcu_dereference(src->fence);
>  
>  retry:
> -       dma_resv_fences(src, &new, &src_list, &shared_count);
> -       if (shared_count) {
> +       if (src_list) {
> +               unsigned shared_count = src_list->shared_count;
> +
>                 rcu_read_unlock();
>  
>                 dst_list = dma_resv_list_alloc(shared_count);
> @@ -311,14 +330,14 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
>                         return -ENOMEM;
>  
>                 rcu_read_lock();
> -               dma_resv_fences(src, &new, &src_list, &shared_count);
> -               if (!src_list || shared_count > dst_list->shared_max) {
> +               src_list = rcu_dereference(src->fence);
> +               if (!src_list || src_list->shared_count > shared_count) {
>                         kfree(dst_list);
>                         goto retry;
>                 }
>  
>                 dst_list->shared_count = 0;
> -               for (i = 0; i < shared_count; ++i) {
> +               for (i = 0; i < src_list->shared_count; ++i) {
>                         struct dma_fence *fence;
>  
>                         fence = rcu_dereference(src_list->shared[i]);
> @@ -328,6 +347,7 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
>  
>                         if (!dma_fence_get_rcu(fence)) {
>                                 dma_resv_list_free(dst_list);
> +                               src_list = rcu_dereference(src->fence);
>                                 goto retry;
>                         }
>  
> @@ -342,18 +362,18 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
>                 dst_list = NULL;
>         }
>  
> -       if (new && !dma_fence_get_rcu(new)) {
> -               dma_resv_list_free(dst_list);
> -               goto retry;
> -       }
> +       new = dma_fence_get_rcu_safe(&src->fence_excl);
>         rcu_read_unlock();
>  
>         src_list = dma_resv_get_list(dst);
>         old = dma_resv_get_excl(dst);
>  
>         preempt_disable();
> -       rcu_assign_pointer(dst->fence_excl, new);
> -       rcu_assign_pointer(dst->fence, dst_list);
> +       write_seqcount_begin(&dst->seq);
> +       /* write_seqcount_begin provides the necessary memory barrier */
> +       RCU_INIT_POINTER(dst->fence_excl, new);
> +       RCU_INIT_POINTER(dst->fence, dst_list);
> +       write_seqcount_end(&dst->seq);
>         preempt_enable();
>  
>         dma_resv_list_free(src_list);
> @@ -388,18 +408,19 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
>  
>         do {
>                 struct dma_resv_list *fobj;
> -               unsigned int i;
> +               unsigned int i, seq;
>                 size_t sz = 0;
>  
> -               i = 0;
> +               shared_count = i = 0;
>  
>                 rcu_read_lock();
> -               dma_resv_fences(obj, &fence_excl, &fobj,
> -                                         &shared_count);
> +               seq = read_seqcount_begin(&obj->seq);
>  
> +               fence_excl = rcu_dereference(obj->fence_excl);
>                 if (fence_excl && !dma_fence_get_rcu(fence_excl))
>                         goto unlock;
>  
> +               fobj = rcu_dereference(obj->fence);
>                 if (fobj)
>                         sz += sizeof(*shared) * fobj->shared_max;
>  
> @@ -427,6 +448,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
>                                 break;
>                         }
>                         shared = nshared;
> +                       shared_count = fobj ? fobj->shared_count : 0;
>                         for (i = 0; i < shared_count; ++i) {
>                                 shared[i] = rcu_dereference(fobj->shared[i]);
>                                 if (!dma_fence_get_rcu(shared[i]))
> @@ -434,7 +456,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
>                         }
>                 }
>  
> -               if (i != shared_count) {
> +               if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) {
>                         while (i--)
>                                 dma_fence_put(shared[i]);
>                         dma_fence_put(fence_excl);
> @@ -478,17 +500,18 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
>                                bool wait_all, bool intr,
>                                unsigned long timeout)
>  {
> -       struct dma_resv_list *fobj;
>         struct dma_fence *fence;
> -       unsigned shared_count;
> +       unsigned seq, shared_count;
>         long ret = timeout ? timeout : 1;
>         int i;
>  
>  retry:
> +       shared_count = 0;
> +       seq = read_seqcount_begin(&obj->seq);
>         rcu_read_lock();
>         i = -1;
>  
> -       dma_resv_fences(obj, &fence, &fobj, &shared_count);
> +       fence = rcu_dereference(obj->fence_excl);
>         if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
>                 if (!dma_fence_get_rcu(fence))
>                         goto unlock_retry;
> @@ -503,6 +526,11 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
>         }
>  
>         if (wait_all) {
> +               struct dma_resv_list *fobj = rcu_dereference(obj->fence);
> +
> +               if (fobj)
> +                       shared_count = fobj->shared_count;
> +
>                 for (i = 0; !fence && i < shared_count; ++i) {
>                         struct dma_fence *lfence = rcu_dereference(fobj->shared[i]);
>  
> @@ -525,6 +553,11 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
>  
>         rcu_read_unlock();
>         if (fence) {
> +               if (read_seqcount_retry(&obj->seq, seq)) {
> +                       dma_fence_put(fence);
> +                       goto retry;
> +               }
> +
>                 ret = dma_fence_wait_timeout(fence, intr, ret);
>                 dma_fence_put(fence);
>                 if (ret > 0 && wait_all && (i + 1 < shared_count))
> @@ -567,19 +600,23 @@ static inline int dma_resv_test_signaled_single(struct dma_fence *passed_fence)
>   */
>  bool dma_resv_test_signaled_rcu(struct dma_resv *obj, bool test_all)
>  {
> -       struct dma_resv_list *fobj;
> -       struct dma_fence *fence_excl;
> -       unsigned shared_count;
> +       unsigned seq, shared_count;
>         int ret;
>  
>         rcu_read_lock();
>  retry:
>         ret = true;
> +       shared_count = 0;
> +       seq = read_seqcount_begin(&obj->seq);
>  
> -       dma_resv_fences(obj, &fence_excl, &fobj, &shared_count);
>         if (test_all) {
>                 unsigned i;
>  
> +               struct dma_resv_list *fobj = rcu_dereference(obj->fence);
> +
> +               if (fobj)
> +                       shared_count = fobj->shared_count;
> +
>                 for (i = 0; i < shared_count; ++i) {
>                         struct dma_fence *fence = rcu_dereference(fobj->shared[i]);
>  
> @@ -589,14 +626,24 @@ bool dma_resv_test_signaled_rcu(struct dma_resv *obj, bool test_all)
>                         else if (!ret)
>                                 break;
>                 }
> -       }
>  
> -       if (!shared_count && fence_excl) {
> -               ret = dma_resv_test_signaled_single(fence_excl);
> -               if (ret < 0)
> +               if (read_seqcount_retry(&obj->seq, seq))
>                         goto retry;
>         }
>  
> +       if (!shared_count) {
> +               struct dma_fence *fence_excl = rcu_dereference(obj->fence_excl);
> +
> +               if (fence_excl) {
> +                       ret = dma_resv_test_signaled_single(fence_excl);
> +                       if (ret < 0)
> +                               goto retry;
> +
> +                       if (read_seqcount_retry(&obj->seq, seq))
> +                               goto retry;
> +               }
> +       }
> +
>         rcu_read_unlock();
>         return ret;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index bc4ec6b20a87..76e3516484e7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -251,7 +251,12 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo,
>         new->shared_max = old->shared_max;
>         new->shared_count = k;
>  
> -       rcu_assign_pointer(resv->fence, new);
> +       /* Install the new fence list, seqcount provides the barriers */
> +       preempt_disable();
> +       write_seqcount_begin(&resv->seq);
> +       RCU_INIT_POINTER(resv->fence, new);
> +       write_seqcount_end(&resv->seq);
> +       preempt_enable();
>  
>         /* Drop the references to the removed fences or move them to ef_list */
>         for (i = j, k = 0; i < old->shared_count; ++i) {
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_busy.c b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
> index a2aff1d8290e..3d4f5775a4ba 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
> @@ -83,8 +83,7 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>         struct drm_i915_gem_busy *args = data;
>         struct drm_i915_gem_object *obj;
>         struct dma_resv_list *list;
> -       unsigned int i, shared_count;
> -       struct dma_fence *excl;
> +       unsigned int seq;
>         int err;
>  
>         err = -ENOENT;
> @@ -110,18 +109,29 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>          * to report the overall busyness. This is what the wait-ioctl does.
>          *
>          */
> -       dma_resv_fences(obj->base.resv, &excl, &list, &shared_count);
> +retry:
> +       seq = raw_read_seqcount(&obj->base.resv->seq);
>  
>         /* Translate the exclusive fence to the READ *and* WRITE engine */
> -       args->busy = busy_check_writer(excl);
> +       args->busy =
> +               busy_check_writer(rcu_dereference(obj->base.resv->fence_excl));
>  
>         /* Translate shared fences to READ set of engines */
> -       for (i = 0; i < shared_count; ++i) {
> -               struct dma_fence *fence = rcu_dereference(list->shared[i]);
> +       list = rcu_dereference(obj->base.resv->fence);
> +       if (list) {
> +               unsigned int shared_count = list->shared_count, i;
>  
> -               args->busy |= busy_check_reader(fence);
> +               for (i = 0; i < shared_count; ++i) {
> +                       struct dma_fence *fence =
> +                               rcu_dereference(list->shared[i]);
> +
> +                       args->busy |= busy_check_reader(fence);
> +               }
>         }
>  
> +       if (args->busy && read_seqcount_retry(&obj->base.resv->seq, seq))
> +               goto retry;
> +
>         err = 0;
>  out:
>         rcu_read_unlock();
> diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
> index 38f2802afabb..ee50d10f052b 100644
> --- a/include/linux/dma-resv.h
> +++ b/include/linux/dma-resv.h
> @@ -46,6 +46,8 @@
>  #include <linux/rcupdate.h>
>  
>  extern struct ww_class reservation_ww_class;
> +extern struct lock_class_key reservation_seqcount_class;
> +extern const char reservation_seqcount_string[];
>  
>  /**
>   * struct dma_resv_list - a list of shared fences
> @@ -69,6 +71,7 @@ struct dma_resv_list {
>   */
>  struct dma_resv {
>         struct ww_mutex lock;
> +       seqcount_t seq;
>  
>         struct dma_fence __rcu *fence_excl;
>         struct dma_resv_list __rcu *fence;
> @@ -77,24 +80,6 @@ struct dma_resv {
>  #define dma_resv_held(obj) lockdep_is_held(&(obj)->lock.base)
>  #define dma_resv_assert_held(obj) lockdep_assert_held(&(obj)->lock.base)
>  
> -/**
> - * dma_resv_get_excl - get the reservation object's
> - * exclusive fence, with update-side lock held
> - * @obj: the reservation object
> - *
> - * Returns the exclusive fence (if any).  Does NOT take a
> - * reference. Writers must hold obj->lock, readers may only
> - * hold a RCU read side lock.
> - *
> - * RETURNS
> - * The exclusive fence or NULL
> - */
> -static inline struct dma_fence *dma_resv_get_excl(struct dma_resv *obj)
> -{
> -       return rcu_dereference_protected(obj->fence_excl,
> -                                        dma_resv_held(obj));
> -}
> -
>  /**
>   * dma_resv_get_list - get the reservation object's
>   * shared fence list, with update-side lock held
> @@ -109,53 +94,6 @@ static inline struct dma_resv_list *dma_resv_get_list(struct dma_resv *obj)
>                                          dma_resv_held(obj));
>  }
>  
> -/**
> - * dma_resv_fences - read consistent fence pointers
> - * @obj: reservation object where we get the fences from
> - * @excl: pointer for the exclusive fence
> - * @list: pointer for the shared fence list
> - *
> - * Make sure we have a consisten exclusive fence and shared fence list.
> - * Must be called with rcu read side lock held.
> - */
> -static inline void dma_resv_fences(struct dma_resv *obj,
> -                                  struct dma_fence **excl,
> -                                  struct dma_resv_list **list,
> -                                  u32 *shared_count)
> -{
> -       do {
> -               *excl = rcu_dereference(obj->fence_excl);
> -               *list = rcu_dereference(obj->fence);
> -               *shared_count = *list ? (*list)->shared_count : 0;
> -               smp_rmb(); /* See dma_resv_add_excl_fence */
> -       } while (rcu_access_pointer(obj->fence_excl) != *excl);
> -}
> -
> -/**
> - * dma_resv_get_excl_rcu - get the reservation object's
> - * exclusive fence, without lock held.
> - * @obj: the reservation object
> - *
> - * If there is an exclusive fence, this atomically increments it's
> - * reference count and returns it.
> - *
> - * RETURNS
> - * The exclusive fence or NULL if none
> - */
> -static inline struct dma_fence *dma_resv_get_excl_rcu(struct dma_resv *obj)
> -{
> -       struct dma_fence *fence;
> -
> -       if (!rcu_access_pointer(obj->fence_excl))
> -               return NULL;
> -
> -       rcu_read_lock();
> -       fence = dma_fence_get_rcu_safe(&obj->fence_excl);
> -       rcu_read_unlock();
> -
> -       return fence;
> -}
> -
>  /**
>   * dma_resv_lock - lock the reservation object
>   * @obj: the reservation object
> @@ -290,6 +228,51 @@ static inline void dma_resv_unlock(struct dma_resv *obj)
>         ww_mutex_unlock(&obj->lock);
>  }
>  
> +/**
> + * dma_resv_get_excl - get the reservation object's
> + * exclusive fence, with update-side lock held
> + * @obj: the reservation object
> + *
> + * Returns the exclusive fence (if any).  Does NOT take a
> + * reference. Writers must hold obj->lock, readers may only
> + * hold a RCU read side lock.
> + *
> + * RETURNS
> + * The exclusive fence or NULL
> + */
> +static inline struct dma_fence *
> +dma_resv_get_excl(struct dma_resv *obj)
> +{
> +       return rcu_dereference_protected(obj->fence_excl,
> +                                        dma_resv_held(obj));
> +}
> +
> +/**
> + * dma_resv_get_excl_rcu - get the reservation object's
> + * exclusive fence, without lock held.
> + * @obj: the reservation object
> + *
> + * If there is an exclusive fence, this atomically increments it's
> + * reference count and returns it.
> + *
> + * RETURNS
> + * The exclusive fence or NULL if none
> + */
> +static inline struct dma_fence *
> +dma_resv_get_excl_rcu(struct dma_resv *obj)
> +{
> +       struct dma_fence *fence;
> +
> +       if (!rcu_access_pointer(obj->fence_excl))
> +               return NULL;
> +
> +       rcu_read_lock();
> +       fence = dma_fence_get_rcu_safe(&obj->fence_excl);
> +       rcu_read_unlock();
> +
> +       return fence;
> +}
> +
>  void dma_resv_init(struct dma_resv *obj);
>  void dma_resv_fini(struct dma_resv *obj);
>  int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences);
> -- 
> 2.23.0.rc1
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] dma-buf: Restore seqlock around dma_resv updates

[Daniel Vetter]

On Wed, Aug 14, 2019 at 07:26:44PM +0100, Chris Wilson wrote:
> Quoting Chris Wilson (2019-08-14 19:24:01)
> > This reverts
> > 67c97fb79a7f ("dma-buf: add reservation_object_fences helper")
> > dd7a7d1ff2f1 ("drm/i915: use new reservation_object_fences helper")
> > 0e1d8083bddb ("dma-buf: further relax reservation_object_add_shared_fence")
> > 5d344f58da76 ("dma-buf: nuke reservation_object seq number")

Oh I didn't realize they landed already.

> > The scenario that defeats simply grabbing a set of shared/exclusive
> > fences and using them blissfully under RCU is that any of those fences
> > may be reallocated by a SLAB_TYPESAFE_BY_RCU fence slab cache. In this
> > scenario, while keeping the rcu_read_lock we need to establish that no
> > fence was changed in the dma_resv after a read (or full) memory barrier.

So if I'm reading correctly what Chris is saying here I guess my half
comment in that other thread pointed at a real oversight. Since I still
haven't checked and it's too late for real review not more for now.

> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> 
> I said I needed to go lie down, that proves it.

Yeah I guess we need to wait for Christian to wake up an have a working
brain.
-Daniel

> 
> Cc: Christian König <christian.koenig@amd.com>
> 
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/dma-buf/dma-buf.c                     |  31 ++++-
> >  drivers/dma-buf/dma-resv.c                    | 109 ++++++++++++-----
> >  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |   7 +-
> >  drivers/gpu/drm/i915/gem/i915_gem_busy.c      |  24 ++--
> >  include/linux/dma-resv.h                      | 113 ++++++++----------
> >  5 files changed, 175 insertions(+), 109 deletions(-)
> > 
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index b3400d6524ab..433d91d710e4 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -199,7 +199,7 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
> >         struct dma_resv_list *fobj;
> >         struct dma_fence *fence_excl;
> >         __poll_t events;
> > -       unsigned shared_count;
> > +       unsigned shared_count, seq;
> >  
> >         dmabuf = file->private_data;
> >         if (!dmabuf || !dmabuf->resv)
> > @@ -213,8 +213,21 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
> >         if (!events)
> >                 return 0;
> >  
> > +retry:
> > +       seq = read_seqcount_begin(&resv->seq);
> >         rcu_read_lock();
> > -       dma_resv_fences(resv, &fence_excl, &fobj, &shared_count);
> > +
> > +       fobj = rcu_dereference(resv->fence);
> > +       if (fobj)
> > +               shared_count = fobj->shared_count;
> > +       else
> > +               shared_count = 0;
> > +       fence_excl = rcu_dereference(resv->fence_excl);
> > +       if (read_seqcount_retry(&resv->seq, seq)) {
> > +               rcu_read_unlock();
> > +               goto retry;
> > +       }
> > +
> >         if (fence_excl && (!(events & EPOLLOUT) || shared_count == 0)) {
> >                 struct dma_buf_poll_cb_t *dcb = &dmabuf->cb_excl;
> >                 __poll_t pevents = EPOLLIN;
> > @@ -1144,6 +1157,7 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused)
> >         struct dma_resv *robj;
> >         struct dma_resv_list *fobj;
> >         struct dma_fence *fence;
> > +       unsigned seq;
> >         int count = 0, attach_count, shared_count, i;
> >         size_t size = 0;
> >  
> > @@ -1174,9 +1188,16 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused)
> >                                 buf_obj->name ?: "");
> >  
> >                 robj = buf_obj->resv;
> > -               rcu_read_lock();
> > -               dma_resv_fences(robj, &fence, &fobj, &shared_count);
> > -               rcu_read_unlock();
> > +               while (true) {
> > +                       seq = read_seqcount_begin(&robj->seq);
> > +                       rcu_read_lock();
> > +                       fobj = rcu_dereference(robj->fence);
> > +                       shared_count = fobj ? fobj->shared_count : 0;
> > +                       fence = rcu_dereference(robj->fence_excl);
> > +                       if (!read_seqcount_retry(&robj->seq, seq))
> > +                               break;
> > +                       rcu_read_unlock();
> > +               }
> >  
> >                 if (fence)
> >                         seq_printf(s, "\tExclusive fence: %s %s %ssignalled\n",
> > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> > index f5142683c851..42a8f3f11681 100644
> > --- a/drivers/dma-buf/dma-resv.c
> > +++ b/drivers/dma-buf/dma-resv.c
> > @@ -49,6 +49,12 @@
> >  DEFINE_WD_CLASS(reservation_ww_class);
> >  EXPORT_SYMBOL(reservation_ww_class);
> >  
> > +struct lock_class_key reservation_seqcount_class;
> > +EXPORT_SYMBOL(reservation_seqcount_class);
> > +
> > +const char reservation_seqcount_string[] = "reservation_seqcount";
> > +EXPORT_SYMBOL(reservation_seqcount_string);
> > +
> >  /**
> >   * dma_resv_list_alloc - allocate fence list
> >   * @shared_max: number of fences we need space for
> > @@ -96,6 +102,9 @@ static void dma_resv_list_free(struct dma_resv_list *list)
> >  void dma_resv_init(struct dma_resv *obj)
> >  {
> >         ww_mutex_init(&obj->lock, &reservation_ww_class);
> > +
> > +       __seqcount_init(&obj->seq, reservation_seqcount_string,
> > +                       &reservation_seqcount_class);
> >         RCU_INIT_POINTER(obj->fence, NULL);
> >         RCU_INIT_POINTER(obj->fence_excl, NULL);
> >  }
> > @@ -225,6 +234,9 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence)
> >         fobj = dma_resv_get_list(obj);
> >         count = fobj->shared_count;
> >  
> > +       preempt_disable();
> > +       write_seqcount_begin(&obj->seq);
> > +
> >         for (i = 0; i < count; ++i) {
> >  
> >                 old = rcu_dereference_protected(fobj->shared[i],
> > @@ -242,6 +254,9 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence)
> >         RCU_INIT_POINTER(fobj->shared[i], fence);
> >         /* pointer update must be visible before we extend the shared_count */
> >         smp_store_mb(fobj->shared_count, count);
> > +
> > +       write_seqcount_end(&obj->seq);
> > +       preempt_enable();
> >         dma_fence_put(old);
> >  }
> >  EXPORT_SYMBOL(dma_resv_add_shared_fence);
> > @@ -269,10 +284,12 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence)
> >                 dma_fence_get(fence);
> >  
> >         preempt_disable();
> > -       rcu_assign_pointer(obj->fence_excl, fence);
> > -       /* pointer update must be visible before we modify the shared_count */
> > +       write_seqcount_begin(&obj->seq);
> > +       /* write_seqcount_begin provides the necessary memory barrier */
> > +       RCU_INIT_POINTER(obj->fence_excl, fence);
> >         if (old)
> > -               smp_store_mb(old->shared_count, 0);
> > +               old->shared_count = 0;
> > +       write_seqcount_end(&obj->seq);
> >         preempt_enable();
> >  
> >         /* inplace update, no shared fences */
> > @@ -295,15 +312,17 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
> >  {
> >         struct dma_resv_list *src_list, *dst_list;
> >         struct dma_fence *old, *new;
> > -       unsigned int i, shared_count;
> > +       unsigned i;
> >  
> >         dma_resv_assert_held(dst);
> >  
> >         rcu_read_lock();
> > +       src_list = rcu_dereference(src->fence);
> >  
> >  retry:
> > -       dma_resv_fences(src, &new, &src_list, &shared_count);
> > -       if (shared_count) {
> > +       if (src_list) {
> > +               unsigned shared_count = src_list->shared_count;
> > +
> >                 rcu_read_unlock();
> >  
> >                 dst_list = dma_resv_list_alloc(shared_count);
> > @@ -311,14 +330,14 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
> >                         return -ENOMEM;
> >  
> >                 rcu_read_lock();
> > -               dma_resv_fences(src, &new, &src_list, &shared_count);
> > -               if (!src_list || shared_count > dst_list->shared_max) {
> > +               src_list = rcu_dereference(src->fence);
> > +               if (!src_list || src_list->shared_count > shared_count) {
> >                         kfree(dst_list);
> >                         goto retry;
> >                 }
> >  
> >                 dst_list->shared_count = 0;
> > -               for (i = 0; i < shared_count; ++i) {
> > +               for (i = 0; i < src_list->shared_count; ++i) {
> >                         struct dma_fence *fence;
> >  
> >                         fence = rcu_dereference(src_list->shared[i]);
> > @@ -328,6 +347,7 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
> >  
> >                         if (!dma_fence_get_rcu(fence)) {
> >                                 dma_resv_list_free(dst_list);
> > +                               src_list = rcu_dereference(src->fence);
> >                                 goto retry;
> >                         }
> >  
> > @@ -342,18 +362,18 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
> >                 dst_list = NULL;
> >         }
> >  
> > -       if (new && !dma_fence_get_rcu(new)) {
> > -               dma_resv_list_free(dst_list);
> > -               goto retry;
> > -       }
> > +       new = dma_fence_get_rcu_safe(&src->fence_excl);
> >         rcu_read_unlock();
> >  
> >         src_list = dma_resv_get_list(dst);
> >         old = dma_resv_get_excl(dst);
> >  
> >         preempt_disable();
> > -       rcu_assign_pointer(dst->fence_excl, new);
> > -       rcu_assign_pointer(dst->fence, dst_list);
> > +       write_seqcount_begin(&dst->seq);
> > +       /* write_seqcount_begin provides the necessary memory barrier */
> > +       RCU_INIT_POINTER(dst->fence_excl, new);
> > +       RCU_INIT_POINTER(dst->fence, dst_list);
> > +       write_seqcount_end(&dst->seq);
> >         preempt_enable();
> >  
> >         dma_resv_list_free(src_list);
> > @@ -388,18 +408,19 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
> >  
> >         do {
> >                 struct dma_resv_list *fobj;
> > -               unsigned int i;
> > +               unsigned int i, seq;
> >                 size_t sz = 0;
> >  
> > -               i = 0;
> > +               shared_count = i = 0;
> >  
> >                 rcu_read_lock();
> > -               dma_resv_fences(obj, &fence_excl, &fobj,
> > -                                         &shared_count);
> > +               seq = read_seqcount_begin(&obj->seq);
> >  
> > +               fence_excl = rcu_dereference(obj->fence_excl);
> >                 if (fence_excl && !dma_fence_get_rcu(fence_excl))
> >                         goto unlock;
> >  
> > +               fobj = rcu_dereference(obj->fence);
> >                 if (fobj)
> >                         sz += sizeof(*shared) * fobj->shared_max;
> >  
> > @@ -427,6 +448,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
> >                                 break;
> >                         }
> >                         shared = nshared;
> > +                       shared_count = fobj ? fobj->shared_count : 0;
> >                         for (i = 0; i < shared_count; ++i) {
> >                                 shared[i] = rcu_dereference(fobj->shared[i]);
> >                                 if (!dma_fence_get_rcu(shared[i]))
> > @@ -434,7 +456,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
> >                         }
> >                 }
> >  
> > -               if (i != shared_count) {
> > +               if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) {
> >                         while (i--)
> >                                 dma_fence_put(shared[i]);
> >                         dma_fence_put(fence_excl);
> > @@ -478,17 +500,18 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
> >                                bool wait_all, bool intr,
> >                                unsigned long timeout)
> >  {
> > -       struct dma_resv_list *fobj;
> >         struct dma_fence *fence;
> > -       unsigned shared_count;
> > +       unsigned seq, shared_count;
> >         long ret = timeout ? timeout : 1;
> >         int i;
> >  
> >  retry:
> > +       shared_count = 0;
> > +       seq = read_seqcount_begin(&obj->seq);
> >         rcu_read_lock();
> >         i = -1;
> >  
> > -       dma_resv_fences(obj, &fence, &fobj, &shared_count);
> > +       fence = rcu_dereference(obj->fence_excl);
> >         if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
> >                 if (!dma_fence_get_rcu(fence))
> >                         goto unlock_retry;
> > @@ -503,6 +526,11 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
> >         }
> >  
> >         if (wait_all) {
> > +               struct dma_resv_list *fobj = rcu_dereference(obj->fence);
> > +
> > +               if (fobj)
> > +                       shared_count = fobj->shared_count;
> > +
> >                 for (i = 0; !fence && i < shared_count; ++i) {
> >                         struct dma_fence *lfence = rcu_dereference(fobj->shared[i]);
> >  
> > @@ -525,6 +553,11 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
> >  
> >         rcu_read_unlock();
> >         if (fence) {
> > +               if (read_seqcount_retry(&obj->seq, seq)) {
> > +                       dma_fence_put(fence);
> > +                       goto retry;
> > +               }
> > +
> >                 ret = dma_fence_wait_timeout(fence, intr, ret);
> >                 dma_fence_put(fence);
> >                 if (ret > 0 && wait_all && (i + 1 < shared_count))
> > @@ -567,19 +600,23 @@ static inline int dma_resv_test_signaled_single(struct dma_fence *passed_fence)
> >   */
> >  bool dma_resv_test_signaled_rcu(struct dma_resv *obj, bool test_all)
> >  {
> > -       struct dma_resv_list *fobj;
> > -       struct dma_fence *fence_excl;
> > -       unsigned shared_count;
> > +       unsigned seq, shared_count;
> >         int ret;
> >  
> >         rcu_read_lock();
> >  retry:
> >         ret = true;
> > +       shared_count = 0;
> > +       seq = read_seqcount_begin(&obj->seq);
> >  
> > -       dma_resv_fences(obj, &fence_excl, &fobj, &shared_count);
> >         if (test_all) {
> >                 unsigned i;
> >  
> > +               struct dma_resv_list *fobj = rcu_dereference(obj->fence);
> > +
> > +               if (fobj)
> > +                       shared_count = fobj->shared_count;
> > +
> >                 for (i = 0; i < shared_count; ++i) {
> >                         struct dma_fence *fence = rcu_dereference(fobj->shared[i]);
> >  
> > @@ -589,14 +626,24 @@ bool dma_resv_test_signaled_rcu(struct dma_resv *obj, bool test_all)
> >                         else if (!ret)
> >                                 break;
> >                 }
> > -       }
> >  
> > -       if (!shared_count && fence_excl) {
> > -               ret = dma_resv_test_signaled_single(fence_excl);
> > -               if (ret < 0)
> > +               if (read_seqcount_retry(&obj->seq, seq))
> >                         goto retry;
> >         }
> >  
> > +       if (!shared_count) {
> > +               struct dma_fence *fence_excl = rcu_dereference(obj->fence_excl);
> > +
> > +               if (fence_excl) {
> > +                       ret = dma_resv_test_signaled_single(fence_excl);
> > +                       if (ret < 0)
> > +                               goto retry;
> > +
> > +                       if (read_seqcount_retry(&obj->seq, seq))
> > +                               goto retry;
> > +               }
> > +       }
> > +
> >         rcu_read_unlock();
> >         return ret;
> >  }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index bc4ec6b20a87..76e3516484e7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -251,7 +251,12 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo,
> >         new->shared_max = old->shared_max;
> >         new->shared_count = k;
> >  
> > -       rcu_assign_pointer(resv->fence, new);
> > +       /* Install the new fence list, seqcount provides the barriers */
> > +       preempt_disable();
> > +       write_seqcount_begin(&resv->seq);
> > +       RCU_INIT_POINTER(resv->fence, new);
> > +       write_seqcount_end(&resv->seq);
> > +       preempt_enable();
> >  
> >         /* Drop the references to the removed fences or move them to ef_list */
> >         for (i = j, k = 0; i < old->shared_count; ++i) {
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_busy.c b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
> > index a2aff1d8290e..3d4f5775a4ba 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
> > @@ -83,8 +83,7 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
> >         struct drm_i915_gem_busy *args = data;
> >         struct drm_i915_gem_object *obj;
> >         struct dma_resv_list *list;
> > -       unsigned int i, shared_count;
> > -       struct dma_fence *excl;
> > +       unsigned int seq;
> >         int err;
> >  
> >         err = -ENOENT;
> > @@ -110,18 +109,29 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
> >          * to report the overall busyness. This is what the wait-ioctl does.
> >          *
> >          */
> > -       dma_resv_fences(obj->base.resv, &excl, &list, &shared_count);
> > +retry:
> > +       seq = raw_read_seqcount(&obj->base.resv->seq);
> >  
> >         /* Translate the exclusive fence to the READ *and* WRITE engine */
> > -       args->busy = busy_check_writer(excl);
> > +       args->busy =
> > +               busy_check_writer(rcu_dereference(obj->base.resv->fence_excl));
> >  
> >         /* Translate shared fences to READ set of engines */
> > -       for (i = 0; i < shared_count; ++i) {
> > -               struct dma_fence *fence = rcu_dereference(list->shared[i]);
> > +       list = rcu_dereference(obj->base.resv->fence);
> > +       if (list) {
> > +               unsigned int shared_count = list->shared_count, i;
> >  
> > -               args->busy |= busy_check_reader(fence);
> > +               for (i = 0; i < shared_count; ++i) {
> > +                       struct dma_fence *fence =
> > +                               rcu_dereference(list->shared[i]);
> > +
> > +                       args->busy |= busy_check_reader(fence);
> > +               }
> >         }
> >  
> > +       if (args->busy && read_seqcount_retry(&obj->base.resv->seq, seq))
> > +               goto retry;
> > +
> >         err = 0;
> >  out:
> >         rcu_read_unlock();
> > diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
> > index 38f2802afabb..ee50d10f052b 100644
> > --- a/include/linux/dma-resv.h
> > +++ b/include/linux/dma-resv.h
> > @@ -46,6 +46,8 @@
> >  #include <linux/rcupdate.h>
> >  
> >  extern struct ww_class reservation_ww_class;
> > +extern struct lock_class_key reservation_seqcount_class;
> > +extern const char reservation_seqcount_string[];
> >  
> >  /**
> >   * struct dma_resv_list - a list of shared fences
> > @@ -69,6 +71,7 @@ struct dma_resv_list {
> >   */
> >  struct dma_resv {
> >         struct ww_mutex lock;
> > +       seqcount_t seq;
> >  
> >         struct dma_fence __rcu *fence_excl;
> >         struct dma_resv_list __rcu *fence;
> > @@ -77,24 +80,6 @@ struct dma_resv {
> >  #define dma_resv_held(obj) lockdep_is_held(&(obj)->lock.base)
> >  #define dma_resv_assert_held(obj) lockdep_assert_held(&(obj)->lock.base)
> >  
> > -/**
> > - * dma_resv_get_excl - get the reservation object's
> > - * exclusive fence, with update-side lock held
> > - * @obj: the reservation object
> > - *
> > - * Returns the exclusive fence (if any).  Does NOT take a
> > - * reference. Writers must hold obj->lock, readers may only
> > - * hold a RCU read side lock.
> > - *
> > - * RETURNS
> > - * The exclusive fence or NULL
> > - */
> > -static inline struct dma_fence *dma_resv_get_excl(struct dma_resv *obj)
> > -{
> > -       return rcu_dereference_protected(obj->fence_excl,
> > -                                        dma_resv_held(obj));
> > -}
> > -
> >  /**
> >   * dma_resv_get_list - get the reservation object's
> >   * shared fence list, with update-side lock held
> > @@ -109,53 +94,6 @@ static inline struct dma_resv_list *dma_resv_get_list(struct dma_resv *obj)
> >                                          dma_resv_held(obj));
> >  }
> >  
> > -/**
> > - * dma_resv_fences - read consistent fence pointers
> > - * @obj: reservation object where we get the fences from
> > - * @excl: pointer for the exclusive fence
> > - * @list: pointer for the shared fence list
> > - *
> > - * Make sure we have a consisten exclusive fence and shared fence list.
> > - * Must be called with rcu read side lock held.
> > - */
> > -static inline void dma_resv_fences(struct dma_resv *obj,
> > -                                  struct dma_fence **excl,
> > -                                  struct dma_resv_list **list,
> > -                                  u32 *shared_count)
> > -{
> > -       do {
> > -               *excl = rcu_dereference(obj->fence_excl);
> > -               *list = rcu_dereference(obj->fence);
> > -               *shared_count = *list ? (*list)->shared_count : 0;
> > -               smp_rmb(); /* See dma_resv_add_excl_fence */
> > -       } while (rcu_access_pointer(obj->fence_excl) != *excl);
> > -}
> > -
> > -/**
> > - * dma_resv_get_excl_rcu - get the reservation object's
> > - * exclusive fence, without lock held.
> > - * @obj: the reservation object
> > - *
> > - * If there is an exclusive fence, this atomically increments it's
> > - * reference count and returns it.
> > - *
> > - * RETURNS
> > - * The exclusive fence or NULL if none
> > - */
> > -static inline struct dma_fence *dma_resv_get_excl_rcu(struct dma_resv *obj)
> > -{
> > -       struct dma_fence *fence;
> > -
> > -       if (!rcu_access_pointer(obj->fence_excl))
> > -               return NULL;
> > -
> > -       rcu_read_lock();
> > -       fence = dma_fence_get_rcu_safe(&obj->fence_excl);
> > -       rcu_read_unlock();
> > -
> > -       return fence;
> > -}
> > -
> >  /**
> >   * dma_resv_lock - lock the reservation object
> >   * @obj: the reservation object
> > @@ -290,6 +228,51 @@ static inline void dma_resv_unlock(struct dma_resv *obj)
> >         ww_mutex_unlock(&obj->lock);
> >  }
> >  
> > +/**
> > + * dma_resv_get_excl - get the reservation object's
> > + * exclusive fence, with update-side lock held
> > + * @obj: the reservation object
> > + *
> > + * Returns the exclusive fence (if any).  Does NOT take a
> > + * reference. Writers must hold obj->lock, readers may only
> > + * hold a RCU read side lock.
> > + *
> > + * RETURNS
> > + * The exclusive fence or NULL
> > + */
> > +static inline struct dma_fence *
> > +dma_resv_get_excl(struct dma_resv *obj)
> > +{
> > +       return rcu_dereference_protected(obj->fence_excl,
> > +                                        dma_resv_held(obj));
> > +}
> > +
> > +/**
> > + * dma_resv_get_excl_rcu - get the reservation object's
> > + * exclusive fence, without lock held.
> > + * @obj: the reservation object
> > + *
> > + * If there is an exclusive fence, this atomically increments it's
> > + * reference count and returns it.
> > + *
> > + * RETURNS
> > + * The exclusive fence or NULL if none
> > + */
> > +static inline struct dma_fence *
> > +dma_resv_get_excl_rcu(struct dma_resv *obj)
> > +{
> > +       struct dma_fence *fence;
> > +
> > +       if (!rcu_access_pointer(obj->fence_excl))
> > +               return NULL;
> > +
> > +       rcu_read_lock();
> > +       fence = dma_fence_get_rcu_safe(&obj->fence_excl);
> > +       rcu_read_unlock();
> > +
> > +       return fence;
> > +}
> > +
> >  void dma_resv_init(struct dma_resv *obj);
> >  void dma_resv_fini(struct dma_resv *obj);
> >  int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences);
> > -- 
> > 2.23.0.rc1
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] dma-buf: Restore seqlock around dma_resv updates

[Koenig, Christian]

Am 14.08.19 um 22:07 schrieb Daniel Vetter:
> On Wed, Aug 14, 2019 at 07:26:44PM +0100, Chris Wilson wrote:
>> Quoting Chris Wilson (2019-08-14 19:24:01)
>>> This reverts
>>> 67c97fb79a7f ("dma-buf: add reservation_object_fences helper")
>>> dd7a7d1ff2f1 ("drm/i915: use new reservation_object_fences helper")
>>> 0e1d8083bddb ("dma-buf: further relax reservation_object_add_shared_fence")
>>> 5d344f58da76 ("dma-buf: nuke reservation_object seq number")
> Oh I didn't realize they landed already.
>
>>> The scenario that defeats simply grabbing a set of shared/exclusive
>>> fences and using them blissfully under RCU is that any of those fences
>>> may be reallocated by a SLAB_TYPESAFE_BY_RCU fence slab cache. In this
>>> scenario, while keeping the rcu_read_lock we need to establish that no
>>> fence was changed in the dma_resv after a read (or full) memory barrier.
> So if I'm reading correctly what Chris is saying here I guess my half
> comment in that other thread pointed at a real oversight. Since I still
> haven't checked and it's too late for real review not more for now.

Yeah, the root of the problem is that I didn't realized that fences 
could be reused while in the RCU grace period.

Need to go a step back and try to come up with a complete new approach 
for synchronization.

>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> I said I needed to go lie down, that proves it.
> Yeah I guess we need to wait for Christian to wake up an have a working
> brain.

Well in that case you will need to wait for a few more years for my kids 
to enter school age :)

Cheers,
Christian.

> -Daniel
>

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

Re: [PATCH] dma-buf: Restore seqlock around dma_resv updates

[Koenig, Christian]

Am 14.08.19 um 20:26 schrieb Chris Wilson:
> Quoting Chris Wilson (2019-08-14 19:24:01)
>> This reverts
>> 67c97fb79a7f ("dma-buf: add reservation_object_fences helper")
>> dd7a7d1ff2f1 ("drm/i915: use new reservation_object_fences helper")
>> 0e1d8083bddb ("dma-buf: further relax reservation_object_add_shared_fence")
>> 5d344f58da76 ("dma-buf: nuke reservation_object seq number")
>>
>> The scenario that defeats simply grabbing a set of shared/exclusive
>> fences and using them blissfully under RCU is that any of those fences
>> may be reallocated by a SLAB_TYPESAFE_BY_RCU fence slab cache. In this
>> scenario, while keeping the rcu_read_lock we need to establish that no
>> fence was changed in the dma_resv after a read (or full) memory barrier.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Acked-by: Christian König <christian.koenig@amd.com>

>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> I said I needed to go lie down, that proves it.
>
> Cc: Christian König <christian.koenig@amd.com>
>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>   drivers/dma-buf/dma-buf.c                     |  31 ++++-
>>   drivers/dma-buf/dma-resv.c                    | 109 ++++++++++++-----
>>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |   7 +-
>>   drivers/gpu/drm/i915/gem/i915_gem_busy.c      |  24 ++--
>>   include/linux/dma-resv.h                      | 113 ++++++++----------
>>   5 files changed, 175 insertions(+), 109 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index b3400d6524ab..433d91d710e4 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -199,7 +199,7 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>>          struct dma_resv_list *fobj;
>>          struct dma_fence *fence_excl;
>>          __poll_t events;
>> -       unsigned shared_count;
>> +       unsigned shared_count, seq;
>>   
>>          dmabuf = file->private_data;
>>          if (!dmabuf || !dmabuf->resv)
>> @@ -213,8 +213,21 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
>>          if (!events)
>>                  return 0;
>>   
>> +retry:
>> +       seq = read_seqcount_begin(&resv->seq);
>>          rcu_read_lock();
>> -       dma_resv_fences(resv, &fence_excl, &fobj, &shared_count);
>> +
>> +       fobj = rcu_dereference(resv->fence);
>> +       if (fobj)
>> +               shared_count = fobj->shared_count;
>> +       else
>> +               shared_count = 0;
>> +       fence_excl = rcu_dereference(resv->fence_excl);
>> +       if (read_seqcount_retry(&resv->seq, seq)) {
>> +               rcu_read_unlock();
>> +               goto retry;
>> +       }
>> +
>>          if (fence_excl && (!(events & EPOLLOUT) || shared_count == 0)) {
>>                  struct dma_buf_poll_cb_t *dcb = &dmabuf->cb_excl;
>>                  __poll_t pevents = EPOLLIN;
>> @@ -1144,6 +1157,7 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused)
>>          struct dma_resv *robj;
>>          struct dma_resv_list *fobj;
>>          struct dma_fence *fence;
>> +       unsigned seq;
>>          int count = 0, attach_count, shared_count, i;
>>          size_t size = 0;
>>   
>> @@ -1174,9 +1188,16 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused)
>>                                  buf_obj->name ?: "");
>>   
>>                  robj = buf_obj->resv;
>> -               rcu_read_lock();
>> -               dma_resv_fences(robj, &fence, &fobj, &shared_count);
>> -               rcu_read_unlock();
>> +               while (true) {
>> +                       seq = read_seqcount_begin(&robj->seq);
>> +                       rcu_read_lock();
>> +                       fobj = rcu_dereference(robj->fence);
>> +                       shared_count = fobj ? fobj->shared_count : 0;
>> +                       fence = rcu_dereference(robj->fence_excl);
>> +                       if (!read_seqcount_retry(&robj->seq, seq))
>> +                               break;
>> +                       rcu_read_unlock();
>> +               }
>>   
>>                  if (fence)
>>                          seq_printf(s, "\tExclusive fence: %s %s %ssignalled\n",
>> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
>> index f5142683c851..42a8f3f11681 100644
>> --- a/drivers/dma-buf/dma-resv.c
>> +++ b/drivers/dma-buf/dma-resv.c
>> @@ -49,6 +49,12 @@
>>   DEFINE_WD_CLASS(reservation_ww_class);
>>   EXPORT_SYMBOL(reservation_ww_class);
>>   
>> +struct lock_class_key reservation_seqcount_class;
>> +EXPORT_SYMBOL(reservation_seqcount_class);
>> +
>> +const char reservation_seqcount_string[] = "reservation_seqcount";
>> +EXPORT_SYMBOL(reservation_seqcount_string);
>> +
>>   /**
>>    * dma_resv_list_alloc - allocate fence list
>>    * @shared_max: number of fences we need space for
>> @@ -96,6 +102,9 @@ static void dma_resv_list_free(struct dma_resv_list *list)
>>   void dma_resv_init(struct dma_resv *obj)
>>   {
>>          ww_mutex_init(&obj->lock, &reservation_ww_class);
>> +
>> +       __seqcount_init(&obj->seq, reservation_seqcount_string,
>> +                       &reservation_seqcount_class);
>>          RCU_INIT_POINTER(obj->fence, NULL);
>>          RCU_INIT_POINTER(obj->fence_excl, NULL);
>>   }
>> @@ -225,6 +234,9 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence)
>>          fobj = dma_resv_get_list(obj);
>>          count = fobj->shared_count;
>>   
>> +       preempt_disable();
>> +       write_seqcount_begin(&obj->seq);
>> +
>>          for (i = 0; i < count; ++i) {
>>   
>>                  old = rcu_dereference_protected(fobj->shared[i],
>> @@ -242,6 +254,9 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence)
>>          RCU_INIT_POINTER(fobj->shared[i], fence);
>>          /* pointer update must be visible before we extend the shared_count */
>>          smp_store_mb(fobj->shared_count, count);
>> +
>> +       write_seqcount_end(&obj->seq);
>> +       preempt_enable();
>>          dma_fence_put(old);
>>   }
>>   EXPORT_SYMBOL(dma_resv_add_shared_fence);
>> @@ -269,10 +284,12 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence)
>>                  dma_fence_get(fence);
>>   
>>          preempt_disable();
>> -       rcu_assign_pointer(obj->fence_excl, fence);
>> -       /* pointer update must be visible before we modify the shared_count */
>> +       write_seqcount_begin(&obj->seq);
>> +       /* write_seqcount_begin provides the necessary memory barrier */
>> +       RCU_INIT_POINTER(obj->fence_excl, fence);
>>          if (old)
>> -               smp_store_mb(old->shared_count, 0);
>> +               old->shared_count = 0;
>> +       write_seqcount_end(&obj->seq);
>>          preempt_enable();
>>   
>>          /* inplace update, no shared fences */
>> @@ -295,15 +312,17 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
>>   {
>>          struct dma_resv_list *src_list, *dst_list;
>>          struct dma_fence *old, *new;
>> -       unsigned int i, shared_count;
>> +       unsigned i;
>>   
>>          dma_resv_assert_held(dst);
>>   
>>          rcu_read_lock();
>> +       src_list = rcu_dereference(src->fence);
>>   
>>   retry:
>> -       dma_resv_fences(src, &new, &src_list, &shared_count);
>> -       if (shared_count) {
>> +       if (src_list) {
>> +               unsigned shared_count = src_list->shared_count;
>> +
>>                  rcu_read_unlock();
>>   
>>                  dst_list = dma_resv_list_alloc(shared_count);
>> @@ -311,14 +330,14 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
>>                          return -ENOMEM;
>>   
>>                  rcu_read_lock();
>> -               dma_resv_fences(src, &new, &src_list, &shared_count);
>> -               if (!src_list || shared_count > dst_list->shared_max) {
>> +               src_list = rcu_dereference(src->fence);
>> +               if (!src_list || src_list->shared_count > shared_count) {
>>                          kfree(dst_list);
>>                          goto retry;
>>                  }
>>   
>>                  dst_list->shared_count = 0;
>> -               for (i = 0; i < shared_count; ++i) {
>> +               for (i = 0; i < src_list->shared_count; ++i) {
>>                          struct dma_fence *fence;
>>   
>>                          fence = rcu_dereference(src_list->shared[i]);
>> @@ -328,6 +347,7 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
>>   
>>                          if (!dma_fence_get_rcu(fence)) {
>>                                  dma_resv_list_free(dst_list);
>> +                               src_list = rcu_dereference(src->fence);
>>                                  goto retry;
>>                          }
>>   
>> @@ -342,18 +362,18 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
>>                  dst_list = NULL;
>>          }
>>   
>> -       if (new && !dma_fence_get_rcu(new)) {
>> -               dma_resv_list_free(dst_list);
>> -               goto retry;
>> -       }
>> +       new = dma_fence_get_rcu_safe(&src->fence_excl);
>>          rcu_read_unlock();
>>   
>>          src_list = dma_resv_get_list(dst);
>>          old = dma_resv_get_excl(dst);
>>   
>>          preempt_disable();
>> -       rcu_assign_pointer(dst->fence_excl, new);
>> -       rcu_assign_pointer(dst->fence, dst_list);
>> +       write_seqcount_begin(&dst->seq);
>> +       /* write_seqcount_begin provides the necessary memory barrier */
>> +       RCU_INIT_POINTER(dst->fence_excl, new);
>> +       RCU_INIT_POINTER(dst->fence, dst_list);
>> +       write_seqcount_end(&dst->seq);
>>          preempt_enable();
>>   
>>          dma_resv_list_free(src_list);
>> @@ -388,18 +408,19 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
>>   
>>          do {
>>                  struct dma_resv_list *fobj;
>> -               unsigned int i;
>> +               unsigned int i, seq;
>>                  size_t sz = 0;
>>   
>> -               i = 0;
>> +               shared_count = i = 0;
>>   
>>                  rcu_read_lock();
>> -               dma_resv_fences(obj, &fence_excl, &fobj,
>> -                                         &shared_count);
>> +               seq = read_seqcount_begin(&obj->seq);
>>   
>> +               fence_excl = rcu_dereference(obj->fence_excl);
>>                  if (fence_excl && !dma_fence_get_rcu(fence_excl))
>>                          goto unlock;
>>   
>> +               fobj = rcu_dereference(obj->fence);
>>                  if (fobj)
>>                          sz += sizeof(*shared) * fobj->shared_max;
>>   
>> @@ -427,6 +448,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
>>                                  break;
>>                          }
>>                          shared = nshared;
>> +                       shared_count = fobj ? fobj->shared_count : 0;
>>                          for (i = 0; i < shared_count; ++i) {
>>                                  shared[i] = rcu_dereference(fobj->shared[i]);
>>                                  if (!dma_fence_get_rcu(shared[i]))
>> @@ -434,7 +456,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj,
>>                          }
>>                  }
>>   
>> -               if (i != shared_count) {
>> +               if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) {
>>                          while (i--)
>>                                  dma_fence_put(shared[i]);
>>                          dma_fence_put(fence_excl);
>> @@ -478,17 +500,18 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
>>                                 bool wait_all, bool intr,
>>                                 unsigned long timeout)
>>   {
>> -       struct dma_resv_list *fobj;
>>          struct dma_fence *fence;
>> -       unsigned shared_count;
>> +       unsigned seq, shared_count;
>>          long ret = timeout ? timeout : 1;
>>          int i;
>>   
>>   retry:
>> +       shared_count = 0;
>> +       seq = read_seqcount_begin(&obj->seq);
>>          rcu_read_lock();
>>          i = -1;
>>   
>> -       dma_resv_fences(obj, &fence, &fobj, &shared_count);
>> +       fence = rcu_dereference(obj->fence_excl);
>>          if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) {
>>                  if (!dma_fence_get_rcu(fence))
>>                          goto unlock_retry;
>> @@ -503,6 +526,11 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
>>          }
>>   
>>          if (wait_all) {
>> +               struct dma_resv_list *fobj = rcu_dereference(obj->fence);
>> +
>> +               if (fobj)
>> +                       shared_count = fobj->shared_count;
>> +
>>                  for (i = 0; !fence && i < shared_count; ++i) {
>>                          struct dma_fence *lfence = rcu_dereference(fobj->shared[i]);
>>   
>> @@ -525,6 +553,11 @@ long dma_resv_wait_timeout_rcu(struct dma_resv *obj,
>>   
>>          rcu_read_unlock();
>>          if (fence) {
>> +               if (read_seqcount_retry(&obj->seq, seq)) {
>> +                       dma_fence_put(fence);
>> +                       goto retry;
>> +               }
>> +
>>                  ret = dma_fence_wait_timeout(fence, intr, ret);
>>                  dma_fence_put(fence);
>>                  if (ret > 0 && wait_all && (i + 1 < shared_count))
>> @@ -567,19 +600,23 @@ static inline int dma_resv_test_signaled_single(struct dma_fence *passed_fence)
>>    */
>>   bool dma_resv_test_signaled_rcu(struct dma_resv *obj, bool test_all)
>>   {
>> -       struct dma_resv_list *fobj;
>> -       struct dma_fence *fence_excl;
>> -       unsigned shared_count;
>> +       unsigned seq, shared_count;
>>          int ret;
>>   
>>          rcu_read_lock();
>>   retry:
>>          ret = true;
>> +       shared_count = 0;
>> +       seq = read_seqcount_begin(&obj->seq);
>>   
>> -       dma_resv_fences(obj, &fence_excl, &fobj, &shared_count);
>>          if (test_all) {
>>                  unsigned i;
>>   
>> +               struct dma_resv_list *fobj = rcu_dereference(obj->fence);
>> +
>> +               if (fobj)
>> +                       shared_count = fobj->shared_count;
>> +
>>                  for (i = 0; i < shared_count; ++i) {
>>                          struct dma_fence *fence = rcu_dereference(fobj->shared[i]);
>>   
>> @@ -589,14 +626,24 @@ bool dma_resv_test_signaled_rcu(struct dma_resv *obj, bool test_all)
>>                          else if (!ret)
>>                                  break;
>>                  }
>> -       }
>>   
>> -       if (!shared_count && fence_excl) {
>> -               ret = dma_resv_test_signaled_single(fence_excl);
>> -               if (ret < 0)
>> +               if (read_seqcount_retry(&obj->seq, seq))
>>                          goto retry;
>>          }
>>   
>> +       if (!shared_count) {
>> +               struct dma_fence *fence_excl = rcu_dereference(obj->fence_excl);
>> +
>> +               if (fence_excl) {
>> +                       ret = dma_resv_test_signaled_single(fence_excl);
>> +                       if (ret < 0)
>> +                               goto retry;
>> +
>> +                       if (read_seqcount_retry(&obj->seq, seq))
>> +                               goto retry;
>> +               }
>> +       }
>> +
>>          rcu_read_unlock();
>>          return ret;
>>   }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index bc4ec6b20a87..76e3516484e7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -251,7 +251,12 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo,
>>          new->shared_max = old->shared_max;
>>          new->shared_count = k;
>>   
>> -       rcu_assign_pointer(resv->fence, new);
>> +       /* Install the new fence list, seqcount provides the barriers */
>> +       preempt_disable();
>> +       write_seqcount_begin(&resv->seq);
>> +       RCU_INIT_POINTER(resv->fence, new);
>> +       write_seqcount_end(&resv->seq);
>> +       preempt_enable();
>>   
>>          /* Drop the references to the removed fences or move them to ef_list */
>>          for (i = j, k = 0; i < old->shared_count; ++i) {
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_busy.c b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
>> index a2aff1d8290e..3d4f5775a4ba 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_busy.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_busy.c
>> @@ -83,8 +83,7 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>>          struct drm_i915_gem_busy *args = data;
>>          struct drm_i915_gem_object *obj;
>>          struct dma_resv_list *list;
>> -       unsigned int i, shared_count;
>> -       struct dma_fence *excl;
>> +       unsigned int seq;
>>          int err;
>>   
>>          err = -ENOENT;
>> @@ -110,18 +109,29 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>>           * to report the overall busyness. This is what the wait-ioctl does.
>>           *
>>           */
>> -       dma_resv_fences(obj->base.resv, &excl, &list, &shared_count);
>> +retry:
>> +       seq = raw_read_seqcount(&obj->base.resv->seq);
>>   
>>          /* Translate the exclusive fence to the READ *and* WRITE engine */
>> -       args->busy = busy_check_writer(excl);
>> +       args->busy =
>> +               busy_check_writer(rcu_dereference(obj->base.resv->fence_excl));
>>   
>>          /* Translate shared fences to READ set of engines */
>> -       for (i = 0; i < shared_count; ++i) {
>> -               struct dma_fence *fence = rcu_dereference(list->shared[i]);
>> +       list = rcu_dereference(obj->base.resv->fence);
>> +       if (list) {
>> +               unsigned int shared_count = list->shared_count, i;
>>   
>> -               args->busy |= busy_check_reader(fence);
>> +               for (i = 0; i < shared_count; ++i) {
>> +                       struct dma_fence *fence =
>> +                               rcu_dereference(list->shared[i]);
>> +
>> +                       args->busy |= busy_check_reader(fence);
>> +               }
>>          }
>>   
>> +       if (args->busy && read_seqcount_retry(&obj->base.resv->seq, seq))
>> +               goto retry;
>> +
>>          err = 0;
>>   out:
>>          rcu_read_unlock();
>> diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
>> index 38f2802afabb..ee50d10f052b 100644
>> --- a/include/linux/dma-resv.h
>> +++ b/include/linux/dma-resv.h
>> @@ -46,6 +46,8 @@
>>   #include <linux/rcupdate.h>
>>   
>>   extern struct ww_class reservation_ww_class;
>> +extern struct lock_class_key reservation_seqcount_class;
>> +extern const char reservation_seqcount_string[];
>>   
>>   /**
>>    * struct dma_resv_list - a list of shared fences
>> @@ -69,6 +71,7 @@ struct dma_resv_list {
>>    */
>>   struct dma_resv {
>>          struct ww_mutex lock;
>> +       seqcount_t seq;
>>   
>>          struct dma_fence __rcu *fence_excl;
>>          struct dma_resv_list __rcu *fence;
>> @@ -77,24 +80,6 @@ struct dma_resv {
>>   #define dma_resv_held(obj) lockdep_is_held(&(obj)->lock.base)
>>   #define dma_resv_assert_held(obj) lockdep_assert_held(&(obj)->lock.base)
>>   
>> -/**
>> - * dma_resv_get_excl - get the reservation object's
>> - * exclusive fence, with update-side lock held
>> - * @obj: the reservation object
>> - *
>> - * Returns the exclusive fence (if any).  Does NOT take a
>> - * reference. Writers must hold obj->lock, readers may only
>> - * hold a RCU read side lock.
>> - *
>> - * RETURNS
>> - * The exclusive fence or NULL
>> - */
>> -static inline struct dma_fence *dma_resv_get_excl(struct dma_resv *obj)
>> -{
>> -       return rcu_dereference_protected(obj->fence_excl,
>> -                                        dma_resv_held(obj));
>> -}
>> -
>>   /**
>>    * dma_resv_get_list - get the reservation object's
>>    * shared fence list, with update-side lock held
>> @@ -109,53 +94,6 @@ static inline struct dma_resv_list *dma_resv_get_list(struct dma_resv *obj)
>>                                           dma_resv_held(obj));
>>   }
>>   
>> -/**
>> - * dma_resv_fences - read consistent fence pointers
>> - * @obj: reservation object where we get the fences from
>> - * @excl: pointer for the exclusive fence
>> - * @list: pointer for the shared fence list
>> - *
>> - * Make sure we have a consisten exclusive fence and shared fence list.
>> - * Must be called with rcu read side lock held.
>> - */
>> -static inline void dma_resv_fences(struct dma_resv *obj,
>> -                                  struct dma_fence **excl,
>> -                                  struct dma_resv_list **list,
>> -                                  u32 *shared_count)
>> -{
>> -       do {
>> -               *excl = rcu_dereference(obj->fence_excl);
>> -               *list = rcu_dereference(obj->fence);
>> -               *shared_count = *list ? (*list)->shared_count : 0;
>> -               smp_rmb(); /* See dma_resv_add_excl_fence */
>> -       } while (rcu_access_pointer(obj->fence_excl) != *excl);
>> -}
>> -
>> -/**
>> - * dma_resv_get_excl_rcu - get the reservation object's
>> - * exclusive fence, without lock held.
>> - * @obj: the reservation object
>> - *
>> - * If there is an exclusive fence, this atomically increments it's
>> - * reference count and returns it.
>> - *
>> - * RETURNS
>> - * The exclusive fence or NULL if none
>> - */
>> -static inline struct dma_fence *dma_resv_get_excl_rcu(struct dma_resv *obj)
>> -{
>> -       struct dma_fence *fence;
>> -
>> -       if (!rcu_access_pointer(obj->fence_excl))
>> -               return NULL;
>> -
>> -       rcu_read_lock();
>> -       fence = dma_fence_get_rcu_safe(&obj->fence_excl);
>> -       rcu_read_unlock();
>> -
>> -       return fence;
>> -}
>> -
>>   /**
>>    * dma_resv_lock - lock the reservation object
>>    * @obj: the reservation object
>> @@ -290,6 +228,51 @@ static inline void dma_resv_unlock(struct dma_resv *obj)
>>          ww_mutex_unlock(&obj->lock);
>>   }
>>   
>> +/**
>> + * dma_resv_get_excl - get the reservation object's
>> + * exclusive fence, with update-side lock held
>> + * @obj: the reservation object
>> + *
>> + * Returns the exclusive fence (if any).  Does NOT take a
>> + * reference. Writers must hold obj->lock, readers may only
>> + * hold a RCU read side lock.
>> + *
>> + * RETURNS
>> + * The exclusive fence or NULL
>> + */
>> +static inline struct dma_fence *
>> +dma_resv_get_excl(struct dma_resv *obj)
>> +{
>> +       return rcu_dereference_protected(obj->fence_excl,
>> +                                        dma_resv_held(obj));
>> +}
>> +
>> +/**
>> + * dma_resv_get_excl_rcu - get the reservation object's
>> + * exclusive fence, without lock held.
>> + * @obj: the reservation object
>> + *
>> + * If there is an exclusive fence, this atomically increments it's
>> + * reference count and returns it.
>> + *
>> + * RETURNS
>> + * The exclusive fence or NULL if none
>> + */
>> +static inline struct dma_fence *
>> +dma_resv_get_excl_rcu(struct dma_resv *obj)
>> +{
>> +       struct dma_fence *fence;
>> +
>> +       if (!rcu_access_pointer(obj->fence_excl))
>> +               return NULL;
>> +
>> +       rcu_read_lock();
>> +       fence = dma_fence_get_rcu_safe(&obj->fence_excl);
>> +       rcu_read_unlock();
>> +
>> +       return fence;
>> +}
>> +
>>   void dma_resv_init(struct dma_resv *obj);
>>   void dma_resv_fini(struct dma_resv *obj);
>>   int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences);
>> -- 
>> 2.23.0.rc1
>>

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

✗ Fi.CI.CHECKPATCH: warning for dma-buf: Restore seqlock around dma_resv updates

[Patchwork]

== Series Details ==

Series: dma-buf: Restore seqlock around dma_resv updates
URL   : https://patchwork.freedesktop.org/series/65196/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
cffdefd474fc dma-buf: Restore seqlock around dma_resv updates
-:7: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 67c97fb79a7f ("dma-buf: add reservation_object_fences helper")'
#7: 
67c97fb79a7f ("dma-buf: add reservation_object_fences helper")

-:8: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit dd7a7d1ff2f1 ("drm/i915: use new reservation_object_fences helper")'
#8: 
dd7a7d1ff2f1 ("drm/i915: use new reservation_object_fences helper")

-:9: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 0e1d8083bddb ("dma-buf: further relax reservation_object_add_shared_fence")'
#9: 
0e1d8083bddb ("dma-buf: further relax reservation_object_add_shared_fence")

-:10: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 5d344f58da76 ("dma-buf: nuke reservation_object seq number")'
#10: 
5d344f58da76 ("dma-buf: nuke reservation_object seq number")

-:31: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#31: FILE: drivers/dma-buf/dma-buf.c:202:
+	unsigned shared_count, seq;

-:62: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#62: FILE: drivers/dma-buf/dma-buf.c:1160:
+	unsigned seq;

-:97: WARNING:STATIC_CONST_CHAR_ARRAY: const array should probably be static const
#97: FILE: drivers/dma-buf/dma-resv.c:55:
+const char reservation_seqcount_string[] = "reservation_seqcount";

-:154: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#154: FILE: drivers/dma-buf/dma-resv.c:315:
+	unsigned i;

-:165: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#165: FILE: drivers/dma-buf/dma-resv.c:324:
+		unsigned shared_count = src_list->shared_count;

-:230: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#230: FILE: drivers/dma-buf/dma-resv.c:414:
+		shared_count = i = 0;

-:269: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#269: FILE: drivers/dma-buf/dma-resv.c:504:
+	unsigned seq, shared_count;

-:315: WARNING:UNSPECIFIED_INT: Prefer 'unsigned int' to bare use of 'unsigned'
#315: FILE: drivers/dma-buf/dma-resv.c:603:
+	unsigned seq, shared_count;

total: 4 errors, 7 warnings, 1 checks, 513 lines checked

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

✓ Fi.CI.BAT: success for dma-buf: Restore seqlock around dma_resv updates

[Patchwork]

== Series Details ==

Series: dma-buf: Restore seqlock around dma_resv updates
URL   : https://patchwork.freedesktop.org/series/65196/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6709 -> Patchwork_14016
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_create@basic-files:
    - fi-bxt-dsi:         [PASS][1] -> [INCOMPLETE][2] ([fdo#103927])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/fi-bxt-dsi/igt@gem_ctx_create@basic-files.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/fi-bxt-dsi/igt@gem_ctx_create@basic-files.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [PASS][3] -> [FAIL][4] ([fdo#108511])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_execlists:
    - fi-skl-gvtdvm:      [PASS][5] -> [DMESG-FAIL][6] ([fdo#111108])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/fi-skl-gvtdvm/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_requests:
    - fi-byt-j1900:       [PASS][7] -> [INCOMPLETE][8] ([fdo#102657])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/fi-byt-j1900/igt@i915_selftest@live_requests.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/fi-byt-j1900/igt@i915_selftest@live_requests.html

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       [PASS][9] -> [SKIP][10] ([fdo#109271] / [fdo#109278]) +2 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html

  * igt@kms_chamelium@dp-edid-read:
    - fi-icl-u2:          [PASS][11] -> [FAIL][12] ([fdo#106766])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/fi-icl-u2/igt@kms_chamelium@dp-edid-read.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/fi-icl-u2/igt@kms_chamelium@dp-edid-read.html

  * igt@prime_vgem@basic-fence-wait-default:
    - fi-icl-u3:          [PASS][13] -> [DMESG-WARN][14] ([fdo#107724])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/fi-icl-u3/igt@prime_vgem@basic-fence-wait-default.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/fi-icl-u3/igt@prime_vgem@basic-fence-wait-default.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [INCOMPLETE][15] ([fdo#107718]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_selftest@live_sanitycheck:
    - fi-icl-u3:          [DMESG-WARN][17] ([fdo#107724]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/fi-icl-u3/igt@i915_selftest@live_sanitycheck.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/fi-icl-u3/igt@i915_selftest@live_sanitycheck.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7567u:       [FAIL][19] ([fdo#109485]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html

  
  [fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#106766]: https://bugs.freedesktop.org/show_bug.cgi?id=106766
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
  [fdo#111108]: https://bugs.freedesktop.org/show_bug.cgi?id=111108


Participating hosts (53 -> 43)
------------------------------

  Additional (1): fi-kbl-guc 
  Missing    (11): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-skl-6260u fi-cfl-8109u fi-icl-y fi-bdw-samus fi-byt-clapper fi-skl-6700k2 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6709 -> Patchwork_14016

  CI-20190529: 20190529
  CI_DRM_6709: 4c9976607118e10dfc9f9feb3b9be2b3579631c9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5134: 81df2f22385bc275975cf199d962eed9bc10f916 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14016: cffdefd474fc9ecf39ce403c3733e03d5326c0be @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

cffdefd474fc dma-buf: Restore seqlock around dma_resv updates

== Logs ==

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

✓ Fi.CI.IGT: success for dma-buf: Restore seqlock around dma_resv updates

[Patchwork]

== Series Details ==

Series: dma-buf: Restore seqlock around dma_resv updates
URL   : https://patchwork.freedesktop.org/series/65196/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6709_full -> Patchwork_14016_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@reset-stress:
    - shard-apl:          [PASS][1] -> [FAIL][2] ([fdo#109661])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-apl6/igt@gem_eio@reset-stress.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-apl1/igt@gem_eio@reset-stress.html

  * igt@gem_exec_schedule@preempt-contexts-bsd2:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#109276]) +19 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb2/igt@gem_exec_schedule@preempt-contexts-bsd2.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb5/igt@gem_exec_schedule@preempt-contexts-bsd2.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#111325]) +6 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb8/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb1/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +5 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-apl5/igt@gem_workarounds@suspend-resume-context.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-apl2/igt@gem_workarounds@suspend-resume-context.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([fdo#105363])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-glk6/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [PASS][11] -> [FAIL][12] ([fdo#105363])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-skl2/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-skl8/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [PASS][13] -> [FAIL][14] ([fdo#103167]) +6 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][15] -> [FAIL][16] ([fdo#108145])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-skl1/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][17] -> [FAIL][18] ([fdo#103166])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb7/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb7/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109441]) +4 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb5/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][21] -> [FAIL][22] ([fdo#99912])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-apl4/igt@kms_setmode@basic.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-apl6/igt@kms_setmode@basic.html

  * igt@perf@enable-disable:
    - shard-iclb:         [PASS][23] -> [INCOMPLETE][24] ([fdo#107713])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb4/igt@perf@enable-disable.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb7/igt@perf@enable-disable.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [DMESG-WARN][25] ([fdo#108566]) -> [PASS][26] +5 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-apl7/igt@gem_ctx_isolation@rcs0-s3.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-apl1/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [SKIP][27] ([fdo#111325]) -> [PASS][28] +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb2/igt@gem_exec_schedule@in-order-bsd.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb5/igt@gem_exec_schedule@in-order-bsd.html

  * igt@kms_cursor_legacy@cursor-vs-flip-varying-size:
    - shard-apl:          [INCOMPLETE][29] ([fdo#103927]) -> [PASS][30] +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-apl4/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-apl7/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-pwrite:
    - shard-iclb:         [FAIL][31] ([fdo#103167]) -> [PASS][32] +3 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-pwrite.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-pwrite.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][33] ([fdo#103166]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb7/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb7/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_cursor_mmap_gtt:
    - shard-iclb:         [SKIP][35] ([fdo#109441]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb8/igt@kms_psr@psr2_cursor_mmap_gtt.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_gtt.html

  * igt@prime_busy@hang-bsd2:
    - shard-iclb:         [SKIP][37] ([fdo#109276]) -> [PASS][38] +17 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb7/igt@prime_busy@hang-bsd2.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb1/igt@prime_busy@hang-bsd2.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [SKIP][39] ([fdo#109276]) -> [FAIL][40] ([fdo#111330]) +1 similar issue
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb3/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb2/igt@gem_mocs_settings@mocs-reset-bsd2.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [DMESG-WARN][41] ([fdo#107724]) -> [SKIP][42] ([fdo#109349])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-iclb4/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-fullscreen:
    - shard-apl:          [INCOMPLETE][43] ([fdo#103927]) -> [SKIP][44] ([fdo#109271])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6709/shard-apl2/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-fullscreen.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14016/shard-apl5/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-fullscreen.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [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#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6709 -> Patchwork_14016

  CI-20190529: 20190529
  CI_DRM_6709: 4c9976607118e10dfc9f9feb3b9be2b3579631c9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5134: 81df2f22385bc275975cf199d962eed9bc10f916 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14016: cffdefd474fc9ecf39ce403c3733e03d5326c0be @ 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_14016/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx