[PATCH 2/2] dma-buf: allow nested dma_resv_reserve_fences

Yunxiang Li Yunxiang.Li at amd.com
Wed Jun 21 16:23:44 UTC 2023


Calling dma_resv_reserve_fences a second time would not reserve memory
correctly, this is quite unintuitive and the current debug build option
cannot catch this error reliably because of the slack in max_fences.

Rework the function to allow multiple invocations, the side-effect being
reserve check are now stricter and need to be enabled all the time.

This fixes issue where ttm_bo_mem_space's reserve is ignored in various
amdgpu ioctl paths, and was causing fence lost leading to soft-lockup
when VRAM is exhausted.

Signed-off-by: Yunxiang Li <Yunxiang.Li at amd.com>
---
 drivers/dma-buf/dma-resv.c | 56 ++++++++++++++++++++++----------------
 include/linux/dma-resv.h   |  8 ++----
 2 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index b6f71eb00866..5ea97ee88cc0 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -62,7 +62,7 @@ EXPORT_SYMBOL(reservation_ww_class);
 
 struct dma_resv_list {
 	struct rcu_head rcu;
-	u32 num_fences, max_fences;
+	u32 num_fences, reserved_fences, max_fences;
 	struct dma_fence __rcu *table[];
 };
 
@@ -107,6 +107,8 @@ static struct dma_resv_list *dma_resv_list_alloc(unsigned int max_fences)
 	if (!list)
 		return NULL;
 
+	list->num_fences = 0;
+	list->reserved_fences = 0;
 	/* Given the resulting bucket size, recalculated max_fences. */
 	list->max_fences = (size - offsetof(typeof(*list), table)) /
 		sizeof(*list->table);
@@ -173,7 +175,7 @@ static inline struct dma_resv_list *dma_resv_fences_list(struct dma_resv *obj)
  * locked through dma_resv_lock().
  *
  * Note that the preallocated slots need to be re-reserved if @obj is unlocked
- * at any time before calling dma_resv_add_fence(). This is validated when
+ * at any time before calling dma_resv_add_fence(). This produces a warning when
  * CONFIG_DEBUG_MUTEXES is enabled.
  *
  * RETURNS
@@ -182,22 +184,26 @@ static inline struct dma_resv_list *dma_resv_fences_list(struct dma_resv *obj)
 int dma_resv_reserve_fences(struct dma_resv *obj, unsigned int num_fences)
 {
 	struct dma_resv_list *old, *new;
-	unsigned int i, j, k, max;
+	unsigned int i, j, k, new_max;
 
 	dma_resv_assert_held(obj);
 
 	old = dma_resv_fences_list(obj);
 	if (old && old->max_fences) {
-		if ((old->num_fences + num_fences) <= old->max_fences)
+		new_max = old->num_fences + old->reserved_fences + num_fences;
+		if (new_max <= old->max_fences) {
+			old->reserved_fences += num_fences;
 			return 0;
-		max = max(old->num_fences + num_fences, old->max_fences * 2);
+		}
+		new_max = max(new_max, old->max_fences * 2);
 	} else {
-		max = max(4ul, roundup_pow_of_two(num_fences));
+		new_max = max(4ul, roundup_pow_of_two(num_fences));
 	}
 
-	new = dma_resv_list_alloc(max);
+	new = dma_resv_list_alloc(new_max);
 	if (!new)
 		return -ENOMEM;
+	new_max = new->max_fences;
 
 	/*
 	 * no need to bump fence refcounts, rcu_read access
@@ -205,7 +211,7 @@ int dma_resv_reserve_fences(struct dma_resv *obj, unsigned int num_fences)
 	 * references from the old struct are carried over to
 	 * the new.
 	 */
-	for (i = 0, j = 0, k = max; i < (old ? old->num_fences : 0); ++i) {
+	for (i = 0, j = 0, k = new_max; i < (old ? old->num_fences : 0); ++i) {
 		enum dma_resv_usage usage;
 		struct dma_fence *fence;
 
@@ -216,6 +222,7 @@ int dma_resv_reserve_fences(struct dma_resv *obj, unsigned int num_fences)
 			dma_resv_list_set(new, j++, fence, usage);
 	}
 	new->num_fences = j;
+	new->reserved_fences = num_fences + (old ? old->reserved_fences : 0);
 
 	/*
 	 * We are not changing the effective set of fences here so can
@@ -231,7 +238,7 @@ int dma_resv_reserve_fences(struct dma_resv *obj, unsigned int num_fences)
 		return 0;
 
 	/* Drop the references to the signaled fences */
-	for (i = k; i < max; ++i) {
+	for (i = k; i < new_max; ++i) {
 		struct dma_fence *fence;
 
 		fence = rcu_dereference_protected(new->table[i],
@@ -244,27 +251,29 @@ int dma_resv_reserve_fences(struct dma_resv *obj, unsigned int num_fences)
 }
 EXPORT_SYMBOL(dma_resv_reserve_fences);
 
-#ifdef CONFIG_DEBUG_MUTEXES
 /**
- * dma_resv_reset_max_fences - reset fences for debugging
+ * dma_resv_reset_reserved_fences - reset fence reservation
  * @obj: the dma_resv object to reset
  *
- * Reset the number of pre-reserved fence slots to test that drivers do
+ * Reset the number of pre-reserved fence slots to make sure that drivers do
  * correct slot allocation using dma_resv_reserve_fences(). See also
- * &dma_resv_list.max_fences.
+ * &dma_resv_list.reserved_fences.
  */
-void dma_resv_reset_max_fences(struct dma_resv *obj)
+void dma_resv_reset_reserved_fences(struct dma_resv *obj)
 {
 	struct dma_resv_list *fences = dma_resv_fences_list(obj);
 
 	dma_resv_assert_held(obj);
 
-	/* Test fence slot reservation */
-	if (fences)
-		fences->max_fences = fences->num_fences;
-}
-EXPORT_SYMBOL(dma_resv_reset_max_fences);
+	/* reset fence slot reservation */
+	if (fences) {
+#ifdef CONFIG_DEBUG_MUTEXES
+		WARN(fences->reserved_fences, "reserved too many fence slots");
 #endif
+		fences->reserved_fences = 0;
+	}
+}
+EXPORT_SYMBOL(dma_resv_reset_reserved_fences);
 
 /**
  * dma_resv_add_fence - Add a fence to the dma_resv obj
@@ -294,8 +303,12 @@ void dma_resv_add_fence(struct dma_resv *obj, struct dma_fence *fence,
 	WARN_ON(dma_fence_is_container(fence));
 
 	fobj = dma_resv_fences_list(obj);
-	count = fobj->num_fences;
 
+	/* dma_resv_reserve_fences() has not been called */
+	BUG_ON(!fobj->reserved_fences);
+	fobj->reserved_fences -= 1;
+
+	count = fobj->num_fences;
 	for (i = 0; i < count; ++i) {
 		enum dma_resv_usage old_usage;
 
@@ -308,8 +321,6 @@ void dma_resv_add_fence(struct dma_resv *obj, struct dma_fence *fence,
 			return;
 		}
 	}
-
-	BUG_ON(fobj->num_fences >= fobj->max_fences);
 	count++;
 
 	dma_resv_list_set(fobj, i, fence, usage);
@@ -531,7 +542,6 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src)
 				dma_resv_iter_end(&cursor);
 				return -ENOMEM;
 			}
-			list->num_fences = 0;
 		}
 
 		dma_fence_get(f);
diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
index 8d0e34dad446..9b2d76484ff4 100644
--- a/include/linux/dma-resv.h
+++ b/include/linux/dma-resv.h
@@ -311,11 +311,7 @@ static inline bool dma_resv_iter_is_restarted(struct dma_resv_iter *cursor)
 #define dma_resv_held(obj) lockdep_is_held(&(obj)->lock.base)
 #define dma_resv_assert_held(obj) lockdep_assert_held(&(obj)->lock.base)
 
-#ifdef CONFIG_DEBUG_MUTEXES
-void dma_resv_reset_max_fences(struct dma_resv *obj);
-#else
-static inline void dma_resv_reset_max_fences(struct dma_resv *obj) {}
-#endif
+void dma_resv_reset_reserved_fences(struct dma_resv *obj);
 
 /**
  * dma_resv_lock - lock the reservation object
@@ -460,7 +456,7 @@ static inline struct ww_acquire_ctx *dma_resv_locking_ctx(struct dma_resv *obj)
  */
 static inline void dma_resv_unlock(struct dma_resv *obj)
 {
-	dma_resv_reset_max_fences(obj);
+	dma_resv_reset_reserved_fences(obj);
 	ww_mutex_unlock(&obj->lock);
 }
 
-- 
2.41.0



More information about the dri-devel mailing list