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