From 6b566f46391921ad42db200102d66e94c3f99c1e Mon Sep 17 00:00:00 2001 From: Urvesh Rathod Date: Mon, 24 Apr 2023 14:20:26 +0530 Subject: [PATCH] msm: synx: Releases global handle index if handle is not signaled This change provides fix for below issues : 1. If local handle is imported as global, synx takes extra reference on global handles which were not released because on signal callback data had local handle instead of global causing handle leak. 2. If all the child handles of merge fence are local and merged fence is global, upon merge its signaled incorrectly as num_child == 0 even if no one signaled merged fence. 3. During merge synx takes one reference each child dma fences. When merged fence is released, dma fence reference of child handles were not released causing handle/dma fence leak. This change signals underlying child fences if the merged handle is ACTIVE during release and release reference on dma fence. 4. If local handle is imported as global, map_count was not getting incremented because of which object was destroyed more than once. This change increases the map_count variable when local handle is imported as global or vice-versa. 5. In synx_signal API, synx_signal_offload_job followed by signaling dma fence. synx_signal_offload_job internally calls synx_signal_handler which signals dma fence and because of which sometimes synx_signal was returning failure. This fix ensures that synx_signal_handler does not overtake synx signal API. Change-Id: Ia8d2eb969514347cac30f8ae33ce2028119dfd47 Signed-off-by: Urvesh Rathod --- msm/synx/synx.c | 18 ++++++---------- msm/synx/synx_global.c | 25 +++++++++++++++++++++ msm/synx/synx_global.h | 10 +++++++++ msm/synx/synx_util.c | 49 +++++++++++++++++++++++++++++++++++++----- msm/synx/synx_util.h | 1 + 5 files changed, 87 insertions(+), 16 deletions(-) diff --git a/msm/synx/synx.c b/msm/synx/synx.c index c2ed11762a..48ae8f5777 100644 --- a/msm/synx/synx.c +++ b/msm/synx/synx.c @@ -589,16 +589,8 @@ void synx_signal_handler(struct work_struct *cb_dispatch) mutex_lock(&synx_obj->obj_lock); if (signal_cb->flag & SYNX_SIGNAL_FROM_IPC) { - if (synx_util_is_merged_object(synx_obj)) { + if (synx_util_is_merged_object(synx_obj)) rc = synx_native_signal_merged_fence(synx_obj, status); - if (rc != SYNX_SUCCESS) { - mutex_unlock(&synx_obj->obj_lock); - dprintk(SYNX_ERR, - "failed to signal merged fence for %u failed=%d\n", - h_synx, rc); - goto fail; - } - } else rc = synx_native_signal_fence(synx_obj, status); } @@ -723,12 +715,15 @@ int synx_signal(struct synx_session *session, u32 h_synx, u32 status) goto fail; } + mutex_lock(&synx_obj->obj_lock); + if (synx_util_is_global_handle(h_synx) || synx_util_is_global_object(synx_obj)) rc = synx_global_update_status( synx_obj->global_idx, status); if (rc != SYNX_SUCCESS) { + mutex_unlock(&synx_obj->obj_lock); dprintk(SYNX_ERR, "[sess :%llu] status update %d failed=%d\n", client->id, h_synx, rc); @@ -744,7 +739,6 @@ int synx_signal(struct synx_session *session, u32 h_synx, u32 status) rc = synx_signal_offload_job(client, synx_obj, h_synx, status); - mutex_lock(&synx_obj->obj_lock); rc = synx_native_signal_fence(synx_obj, status); if (rc != SYNX_SUCCESS) dprintk(SYNX_ERR, @@ -1204,7 +1198,7 @@ int synx_wait(struct synx_session *session, else synx_native_signal_fence(synx_obj, rc); mutex_unlock(&synx_obj->obj_lock); - goto fail; + goto status; } } @@ -1218,6 +1212,7 @@ int synx_wait(struct synx_session *session, goto fail; } +status: mutex_lock(&synx_obj->obj_lock); rc = synx_util_get_object_status(synx_obj); mutex_unlock(&synx_obj->obj_lock); @@ -1437,6 +1432,7 @@ static struct synx_map_entry *synx_handle_conversion( } } } else { + synx_obj->map_count++; rc = synx_alloc_global_handle(h_synx); if (rc == SYNX_SUCCESS) { synx_obj->global_idx = diff --git a/msm/synx/synx_global.c b/msm/synx/synx_global.c index 9c474d535f..edfbf2faec 100644 --- a/msm/synx/synx_global.c +++ b/msm/synx/synx_global.c @@ -328,6 +328,28 @@ int synx_global_get_subscribed_cores(u32 idx, bool *cores) return SYNX_SUCCESS; } +int synx_global_fetch_handle_details(u32 idx, u32 *h_synx) +{ + int rc; + unsigned long flags; + struct synx_global_coredata *synx_g_obj; + + if (!synx_gmem.table) + return -SYNX_NOMEM; + + if (IS_ERR_OR_NULL(h_synx) || !synx_is_valid_idx(idx)) + return -SYNX_INVALID; + + rc = synx_gmem_lock(idx, &flags); + if (rc) + return rc; + synx_g_obj = &synx_gmem.table[idx]; + *h_synx = synx_g_obj->handle; + synx_gmem_unlock(idx, &flags); + + return SYNX_SUCCESS; +} + int synx_global_set_subscribed_core(u32 idx, enum synx_core_id id) { int rc; @@ -710,6 +732,9 @@ int synx_global_merge(u32 *idx_list, u32 num_list, u32 p_idx) if (!synx_is_valid_idx(p_idx)) return -SYNX_INVALID; + if (num_list == 0) + return SYNX_SUCCESS; + while (j < num_list) { idx = idx_list[j]; diff --git a/msm/synx/synx_global.h b/msm/synx/synx_global.h index 99f246490f..733be049e9 100644 --- a/msm/synx/synx_global.h +++ b/msm/synx/synx_global.h @@ -293,4 +293,14 @@ int synx_global_clean_cdsp_mem(void); int synx_global_dump_shared_memory(void); +/** + * synx_global_fetch_handle_details - Fetches the synx handle from + * global shared memory. + * + * @param idx : Global entry index whose handle is requested. + * + * @return SYNX_SUCCESS on success. Negative error on failure. + */ +int synx_global_fetch_handle_details(u32 idx, u32 *h_synx); + #endif /* __SYNX_SHARED_MEM_H__ */ diff --git a/msm/synx/synx_util.c b/msm/synx/synx_util.c index 0b0575bb70..89a1a7925f 100644 --- a/msm/synx/synx_util.c +++ b/msm/synx/synx_util.c @@ -12,6 +12,7 @@ #include "synx_util.h" extern void synx_external_callback(s32 sync_obj, int status, void *data); +static u32 __fence_state(struct dma_fence *fence, bool locked); int synx_util_init_coredata(struct synx_coredata *synx_obj, struct synx_create_params *params, @@ -247,6 +248,38 @@ void synx_util_put_object(struct synx_coredata *synx_obj) kref_put(&synx_obj->refcount, synx_util_destroy_coredata); } +int synx_util_cleanup_merged_fence(struct synx_coredata *synx_obj, int status) +{ + struct dma_fence_array *array = NULL; + u32 i; + int rc = 0; + + if (IS_ERR_OR_NULL(synx_obj) || IS_ERR_OR_NULL(synx_obj->fence)) + return -SYNX_INVALID; + + if (dma_fence_is_array(synx_obj->fence)) { + array = to_dma_fence_array(synx_obj->fence); + if (IS_ERR_OR_NULL(array)) + return -SYNX_INVALID; + + for (i = 0; i < array->num_fences; i++) { + if (kref_read(&array->fences[i]->refcount) == 1 && + __fence_state(array->fences[i], false) == SYNX_STATE_ACTIVE) { + dma_fence_set_error(array->fences[i], + -SYNX_STATE_SIGNALED_CANCEL); + + rc = dma_fence_signal(array->fences[i]); + if (rc) + dprintk(SYNX_ERR, + "signaling child fence %pK failed=%d\n", + array->fences[i], rc); + } + dma_fence_put(array->fences[i]); + } + } + return rc; +} + void synx_util_object_destroy(struct synx_coredata *synx_obj) { int rc; @@ -311,7 +344,10 @@ void synx_util_object_destroy(struct synx_coredata *synx_obj) */ if (!IS_ERR_OR_NULL(synx_obj->fence)) { spin_lock_irqsave(synx_obj->fence->lock, flags); - if (kref_read(&synx_obj->fence->refcount) == 1 && + if (synx_util_is_merged_object(synx_obj) && + synx_util_get_object_status_locked(synx_obj) == SYNX_STATE_ACTIVE) + rc = synx_util_cleanup_merged_fence(synx_obj, -SYNX_STATE_SIGNALED_CANCEL); + else if (kref_read(&synx_obj->fence->refcount) == 1 && (synx_util_get_object_status_locked(synx_obj) == SYNX_STATE_ACTIVE)) { // set fence error to cancel @@ -319,12 +355,12 @@ void synx_util_object_destroy(struct synx_coredata *synx_obj) -SYNX_STATE_SIGNALED_CANCEL); rc = dma_fence_signal_locked(synx_obj->fence); - if (rc) - dprintk(SYNX_ERR, - "signaling fence %pK failed=%d\n", - synx_obj->fence, rc); } spin_unlock_irqrestore(synx_obj->fence->lock, flags); + if (rc) + dprintk(SYNX_ERR, + "signaling fence %pK failed=%d\n", + synx_obj->fence, rc); } dma_fence_put(synx_obj->fence); @@ -873,6 +909,7 @@ static void synx_util_cleanup_fence( unsigned long flags; u32 g_status; u32 f_status; + u32 h_synx = 0; mutex_lock(&synx_obj->obj_lock); synx_obj->map_count--; @@ -903,6 +940,8 @@ static void synx_util_cleanup_fence( if (synx_util_get_object_status_locked(synx_obj) == SYNX_STATE_ACTIVE) { signal_cb->synx_obj = NULL; + synx_global_fetch_handle_details(synx_obj->global_idx, &h_synx); + signal_cb->handle = h_synx; synx_obj->signal_cb = NULL; /* * release reference held by signal cb and diff --git a/msm/synx/synx_util.h b/msm/synx/synx_util.h index fc6c3508fb..95f54c4bb2 100644 --- a/msm/synx/synx_util.h +++ b/msm/synx/synx_util.h @@ -60,6 +60,7 @@ static inline bool synx_util_is_external_object( struct synx_coredata *synx_obj) { if (synx_obj && + !(synx_obj->type & SYNX_CREATE_MERGED_FENCE) && (synx_obj->type & SYNX_CREATE_DMA_FENCE)) return true;