[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