Przeglądaj źródła

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 <[email protected]>
Urvesh Rathod 2 lat temu
rodzic
commit
6b566f4639
5 zmienionych plików z 87 dodań i 16 usunięć
  1. 7 11
      msm/synx/synx.c
  2. 25 0
      msm/synx/synx_global.c
  3. 10 0
      msm/synx/synx_global.h
  4. 44 5
      msm/synx/synx_util.c
  5. 1 0
      msm/synx/synx_util.h

+ 7 - 11
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 =

+ 25 - 0
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];
 

+ 10 - 0
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__ */

+ 44 - 5
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

+ 1 - 0
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;