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, also make reserve
check stricter. This fixes issue where ttm_bo_mem_space's reserve is
ignored in various amdgpu ioctl paths, and was causing soft-lockup when
VRAM is exhausted.

v2: try to avoid memory corruption if possible.

Signed-off-by: Yunxiang Li <yunxiang...@amd.com>
---
v3: fix typo

 drivers/dma-buf/dma-resv.c | 56 ++++++++++++++++++++++++--------------
 include/linux/dma-resv.h   |  8 ++----
 2 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index b6f71eb00866..f8e2974771e5 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,27 @@ 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)
+               num_fences += old->reserved_fences;
+               if ((old->num_fences + num_fences) <= old->max_fences) {
+                       old->reserved_fences = num_fences;
                        return 0;
-               max = max(old->num_fences + num_fences, old->max_fences * 2);
+               }
+               new_max =
+                       max(old->num_fences + num_fences, old->max_fences * 2);
        } else {
-               max = max(4ul, roundup_pow_of_two(num_fences));
+               new_max = max(num_fences, 4u);
        }
 
-       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 +212,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 +223,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;
 
        /*
         * We are not changing the effective set of fences here so can
@@ -231,7 +239,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 +252,25 @@ 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 */
+       /* reset fence slot reservation */
        if (fences)
-               fences->max_fences = fences->num_fences;
+               fences->reserved_fences = 0;
 }
-EXPORT_SYMBOL(dma_resv_reset_max_fences);
-#endif
+EXPORT_SYMBOL(dma_resv_reset_reserved_fences);
 
 /**
  * dma_resv_add_fence - Add a fence to the dma_resv obj
@@ -293,6 +299,7 @@ void dma_resv_add_fence(struct dma_resv *obj, struct 
dma_fence *fence,
         */
        WARN_ON(dma_fence_is_container(fence));
 
+retry:
        fobj = dma_resv_fences_list(obj);
        count = fobj->num_fences;
 
@@ -309,7 +316,17 @@ void dma_resv_add_fence(struct dma_resv *obj, struct 
dma_fence *fence,
                }
        }
 
-       BUG_ON(fobj->num_fences >= fobj->max_fences);
+       if (WARN_ON(fobj->num_fences == fobj->max_fences)) {
+               // try our best to avoid memory corruption
+               dma_resv_reserve_fences(obj, 1);
+               goto retry;
+       }
+       if (fobj->reserved_fences)
+               fobj->reserved_fences -= 1;
+#ifdef CONFIG_DEBUG_MUTEXES
+       else
+               WARN_ON(1); // missing fence slot allocation
+#endif
        count++;
 
        dma_resv_list_set(fobj, i, fence, usage);
@@ -531,7 +548,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

Reply via email to