video: driver: optimize buffer pool design

Added change to avoid unnecessary list iteration during
msm_memory_free also added logic to catch double free
request.

Change-Id: Ie3fb8019fd039e95ea75ba8f3fbd266af515e5cd
Signed-off-by: Govindaraj Rajagopal <grajagop@codeaurora.org>

driver/vidc/inc/msm_vidc_memory.h

@@ -28,11 +28,12 @@ enum msm_memory_pool_type {
 
 struct msm_memory_alloc_header {
 	struct list_head       list;
+	u32                    type;
+	bool                   busy;
 	void                  *buf;
 };
 
 struct msm_memory_pool {
-	u32                    type;
 	u32                    size;
 	char                  *name;
 	struct list_head       free_pool; /* list of struct msm_memory_alloc_header */
@@ -57,7 +58,6 @@ int msm_memory_pools_init(struct msm_vidc_inst *inst);
 void msm_memory_pools_deinit(struct msm_vidc_inst *inst);
 void *msm_memory_alloc(struct msm_vidc_inst *inst,
 	enum msm_memory_pool_type type);
-void msm_memory_free(struct msm_vidc_inst *inst,
-	enum msm_memory_pool_type type, void *vidc_buf);
+void msm_memory_free(struct msm_vidc_inst *inst, void *vidc_buf);
 
 #endif // _MSM_VIDC_MEMORY_H_

driver/vidc/src/msm_vdec.c

@@ -1848,7 +1848,7 @@ int msm_vdec_handle_release_buffer(struct msm_vidc_inst *inst,
 	print_vidc_buffer(VIDC_LOW, "low ", "release done", inst, buf);
 	/* delete the buffer from release list */
 	list_del(&buf->list);
-	msm_memory_free(inst, MSM_MEM_POOL_BUFFER, buf);
+	msm_memory_free(inst, buf);
 
 	return rc;
 }

driver/vidc/src/msm_vidc_driver.c

@@ -1983,7 +1983,7 @@ int msm_vidc_process_readonly_buffers(struct msm_vidc_inst *inst,
 			buf->attr |= MSM_VIDC_ATTR_READ_ONLY;
 			print_vidc_buffer(VIDC_LOW, "low ", "ro buf removed", inst, ro_buf);
 			list_del(&ro_buf->list);
-			msm_memory_free(inst, MSM_MEM_POOL_BUFFER, ro_buf);
+			msm_memory_free(inst, ro_buf);
 			break;
 		}
 	}
@@ -2010,7 +2010,7 @@ int msm_vidc_memory_unmap_completely(struct msm_vidc_inst *inst,
 		if (!map->refcount) {
 			msm_vidc_memory_put_dmabuf(inst, map->dmabuf);
 			list_del(&map->list);
-			msm_memory_free(inst, MSM_MEM_POOL_MAP, map);
+			msm_memory_free(inst, map);
 			break;
 		}
 	}
@@ -2175,7 +2175,7 @@ int msm_vidc_flush_ts(struct msm_vidc_inst *inst)
 		i_vpr_l(inst, "%s: flushing ts: val %lld, rank %%lld\n",
 			__func__, ts->sort.val, ts->rank);
 		list_del(&ts->sort.list);
-		msm_memory_free(inst, MSM_MEM_POOL_TIMESTAMP, ts);
+		msm_memory_free(inst, ts);
 	}
 	inst->timestamps.count = 0;
 	inst->timestamps.rank = 0;
@@ -2222,7 +2222,7 @@ int msm_vidc_update_timestamp(struct msm_vidc_inst *inst, u64 timestamp)
 		}
 		inst->timestamps.count--;
 		list_del(&ts->sort.list);
-		msm_memory_free(inst, MSM_MEM_POOL_TIMESTAMP, ts);
+		msm_memory_free(inst, ts);
 	}
 
 	return 0;
@@ -2268,7 +2268,7 @@ int msm_vidc_put_delayed_unmap(struct msm_vidc_inst *inst, struct msm_vidc_map *
 	if (!map->refcount) {
 		msm_vidc_memory_put_dmabuf(inst, map->dmabuf);
 		list_del(&map->list);
-		msm_memory_free(inst, MSM_MEM_POOL_MAP, map);
+		msm_memory_free(inst, map);
 	}
 
 	return rc;
@@ -2336,7 +2336,7 @@ int msm_vidc_unmap_driver_buf(struct msm_vidc_inst *inst,
 	if (!map->refcount) {
 		msm_vidc_memory_put_dmabuf(inst, map->dmabuf);
 		list_del(&map->list);
-		msm_memory_free(inst, MSM_MEM_POOL_MAP, map);
+		msm_memory_free(inst, map);
 	}
 
 	return rc;
@@ -2387,7 +2387,7 @@ int msm_vidc_map_driver_buf(struct msm_vidc_inst *inst,
 			rc = msm_vidc_get_delayed_unmap(inst, map);
 			if (rc) {
 				msm_vidc_memory_put_dmabuf(inst, map->dmabuf);
-				msm_memory_free(inst, MSM_MEM_POOL_MAP, map);
+				msm_memory_free(inst, map);
 				return rc;
 			}
 		}
@@ -2418,7 +2418,7 @@ int msm_vidc_put_driver_buf(struct msm_vidc_inst *inst,
 
 	/* delete the buffer from buffers->list */
 	list_del(&buf->list);
-	msm_memory_free(inst, MSM_MEM_POOL_BUFFER, buf);
+	msm_memory_free(inst, buf);
 
 	return rc;
 }
@@ -2472,7 +2472,7 @@ struct msm_vidc_buffer *msm_vidc_get_driver_buf(struct msm_vidc_inst *inst,
 error:
 	msm_vidc_memory_put_dmabuf(inst, buf->dmabuf);
 	list_del(&buf->list);
-	msm_memory_free(inst, MSM_MEM_POOL_BUFFER, buf);
+	msm_memory_free(inst, buf);
 	return NULL;
 }
 
@@ -2915,7 +2915,7 @@ int msm_vidc_destroy_internal_buffer(struct msm_vidc_inst *inst,
 		if (map->dmabuf == buffer->dmabuf) {
 			msm_vidc_memory_unmap(inst->core, map);
 			list_del(&map->list);
-			msm_memory_free(inst, MSM_MEM_POOL_MAP, map);
+			msm_memory_free(inst, map);
 			break;
 		}
 	}
@@ -2924,7 +2924,7 @@ int msm_vidc_destroy_internal_buffer(struct msm_vidc_inst *inst,
 		if (alloc->dmabuf == buffer->dmabuf) {
 			msm_vidc_memory_free(inst->core, alloc);
 			list_del(&alloc->list);
-			msm_memory_free(inst, MSM_MEM_POOL_ALLOC, alloc);
+			msm_memory_free(inst, alloc);
 			break;
 		}
 	}
@@ -2932,7 +2932,7 @@ int msm_vidc_destroy_internal_buffer(struct msm_vidc_inst *inst,
 	list_for_each_entry_safe(buf, dummy, &buffers->list, list) {
 		if (buf->dmabuf == buffer->dmabuf) {
 			list_del(&buf->list);
-			msm_memory_free(inst, MSM_MEM_POOL_BUFFER, buf);
+			msm_memory_free(inst, buf);
 			break;
 		}
 	}
@@ -4599,20 +4599,20 @@ void msm_vidc_destroy_buffers(struct msm_vidc_inst *inst)
 	list_for_each_entry_safe(buf, dummy, &inst->buffers.read_only.list, list) {
 		print_vidc_buffer(VIDC_ERR, "err ", "destroying ro buffer", inst, buf);
 		list_del(&buf->list);
-		msm_memory_free(inst, MSM_MEM_POOL_BUFFER, buf);
+		msm_memory_free(inst, buf);
 	}
 
 	list_for_each_entry_safe(buf, dummy, &inst->buffers.release.list, list) {
 		print_vidc_buffer(VIDC_ERR, "err ", "destroying release buffer", inst, buf);
 		list_del(&buf->list);
-		msm_memory_free(inst, MSM_MEM_POOL_BUFFER, buf);
+		msm_memory_free(inst, buf);
 	}
 
 	list_for_each_entry_safe(ts, dummy_ts, &inst->timestamps.list, sort.list) {
 		i_vpr_e(inst, "%s: removing ts: val %lld, rank %lld\n",
 			__func__, ts->sort.val, ts->rank);
 		list_del(&ts->sort.list);
-		msm_memory_free(inst, MSM_MEM_POOL_TIMESTAMP, ts);
+		msm_memory_free(inst, ts);
 	}
 
 	list_for_each_entry_safe(dbuf, dummy_dbuf, &inst->dmabuf_tracker, list) {

driver/vidc/src/msm_vidc_memory.c

@@ -145,7 +145,7 @@ void msm_vidc_memory_put_dmabuf(struct msm_vidc_inst *inst, struct dma_buf *dmab
 	dma_buf_put(buf->dmabuf);
 
 	/* put tracker instance back to pool */
-	msm_memory_free(inst, MSM_MEM_POOL_DMABUF, buf);
+	msm_memory_free(inst, buf);
 }
 
 void msm_vidc_memory_put_dmabuf_completely(struct msm_vidc_inst *inst,
@@ -166,7 +166,7 @@ void msm_vidc_memory_put_dmabuf_completely(struct msm_vidc_inst *inst,
 			dma_buf_put(buf->dmabuf);
 
 			/* put tracker instance back to pool */
-			msm_memory_free(inst, MSM_MEM_POOL_DMABUF, buf);
+			msm_memory_free(inst, buf);
 			break;
 		}
 	}
@@ -426,6 +426,9 @@ void *msm_memory_alloc(struct msm_vidc_inst *inst, enum msm_memory_pool_type typ
 		/* add to busy pool */
 		list_add_tail(&hdr->list, &pool->busy_pool);
 
+		/* set busy flag to true. This is to catch double free request */
+		hdr->busy = true;
+
 		return hdr->buf;
 	}
 
@@ -435,37 +438,46 @@ void *msm_memory_alloc(struct msm_vidc_inst *inst, enum msm_memory_pool_type typ
 		return NULL;
 	}
 	INIT_LIST_HEAD(&hdr->list);
+	hdr->type = type;
+	hdr->busy = true;
 	hdr->buf = (void *)(hdr + 1);
 	list_add_tail(&hdr->list, &pool->busy_pool);
 
 	return hdr->buf;
 }
 
-void msm_memory_free(struct msm_vidc_inst *inst, enum msm_memory_pool_type type,
-	void *vidc_buf)
+void msm_memory_free(struct msm_vidc_inst *inst, void *vidc_buf)
 {
 	struct msm_memory_alloc_header *hdr;
 	struct msm_memory_pool *pool;
-	bool found = false;
 
-	if (!inst || !vidc_buf || type < 0 || type >= MSM_MEM_POOL_MAX) {
+	if (!inst || !vidc_buf) {
 		d_vpr_e("%s: Invalid params\n", __func__);
 		return;
 	}
-	pool = &inst->pool[type];
+	hdr = (struct msm_memory_alloc_header *)vidc_buf - 1;
 
 	/* sanitize buffer addr */
-	list_for_each_entry(hdr, &pool->busy_pool, list) {
-		if (hdr->buf == vidc_buf) {
-			found = true;
-			break;
-		}
-	}
-	if (!found) {
+	if (hdr->buf != vidc_buf) {
 		i_vpr_e(inst, "%s: invalid buf addr %#x\n", __func__, vidc_buf);
 		return;
 	}
 
+	/* sanitize pool type */
+	if (hdr->type < 0 || hdr->type >= MSM_MEM_POOL_MAX) {
+		i_vpr_e(inst, "%s: invalid pool type %#x\n", __func__, hdr->type);
+		return;
+	}
+	pool = &inst->pool[hdr->type];
+
+	/* catch double-free request */
+	if (!hdr->busy) {
+		i_vpr_e(inst, "%s: double free request. type %s, addr %#x\n", __func__,
+			pool->name, vidc_buf);
+		return;
+	}
+	hdr->busy = false;
+
 	/* remove from busy pool */
 	list_del_init(&hdr->list);
 
@@ -558,8 +570,6 @@ int msm_memory_pools_init(struct msm_vidc_inst *inst)
 				i, buftype_size_name_arr[i].type);
 			return -EINVAL;
 		}
-
-		inst->pool[i].type = buftype_size_name_arr[i].type;
 		inst->pool[i].size = buftype_size_name_arr[i].size;
 		inst->pool[i].name = buftype_size_name_arr[i].name;
 		INIT_LIST_HEAD(&inst->pool[i].free_pool);

driver/vidc/src/venus_hfi_response.c

@@ -657,7 +657,7 @@ static int handle_non_read_only_buffer(struct msm_vidc_inst *inst,
 	if (found) {
 		print_vidc_buffer(VIDC_LOW, "low ", "ro buf deleted", inst, ro_buf);
 		list_del(&ro_buf->list);
-		msm_memory_free(inst, MSM_MEM_POOL_BUFFER, ro_buf);
+		msm_memory_free(inst, ro_buf);
 	}
 
 	return 0;