From 7fd846f325d248d5095688d91ccdf57f5a12178a Mon Sep 17 00:00:00 2001 From: "Isaac J. Manjarres" Date: Mon, 17 Aug 2020 16:39:11 -0700 Subject: [PATCH 1/2] soc: qcom: mem-buf: Use an unbound high priority workqueue for mem-buf Use an unbound high priority workqueue to ensure that mem-buf transactions can be handled on any CPU, as well as with high priority. Also, increase the timeout for allocation responses from 2 seconds to 3.5 seconds. Change-Id: I2ec4d85bf7307d64d2f8a7dae6dfb2b29cfc45bb Signed-off-by: Isaac J. Manjarres --- drivers/soc/qcom/mem-buf.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/soc/qcom/mem-buf.c b/drivers/soc/qcom/mem-buf.c index 9ffe23127f25..3c8c0f59a05e 100644 --- a/drivers/soc/qcom/mem-buf.c +++ b/drivers/soc/qcom/mem-buf.c @@ -34,7 +34,7 @@ #define MEM_BUF_MAX_DEVS 1 #define MEM_BUF_MHP_ALIGNMENT (1UL << SUBSECTION_SHIFT) -#define MEM_BUF_TIMEOUT_MS 2000 +#define MEM_BUF_TIMEOUT_MS 3500 #define to_rmt_msg(_work) container_of(_work, struct mem_buf_rmt_msg, work) #define MEM_BUF_CAP_SUPPLIER BIT(0) @@ -59,6 +59,7 @@ static DEFINE_IDR(mem_buf_txn_idr); static struct task_struct *mem_buf_msgq_recv_thr; static void *mem_buf_hh_msgq_hdl; static unsigned char mem_buf_capability; +static struct workqueue_struct *mem_buf_wq; /** * struct mem_buf_txn: Represents a transaction (request/response pair) in the @@ -782,7 +783,7 @@ static void mem_buf_process_msg(void *buf, size_t size) work_fn = hdr->msg_type == MEM_BUF_ALLOC_REQ ? mem_buf_alloc_req_work : mem_buf_relinquish_work; INIT_WORK(&rmt_msg->work, work_fn); - schedule_work(&rmt_msg->work); + queue_work(mem_buf_wq, &rmt_msg->work); } else { pr_err("%s: received message of unknown type: %d\n", __func__, hdr->msg_type); @@ -2124,12 +2125,19 @@ static int mem_buf_probe(struct platform_device *pdev) return ret; } + mem_buf_wq = alloc_workqueue("mem_buf_wq", WQ_HIGHPRI | WQ_UNBOUND, 0); + if (!mem_buf_wq) { + dev_err(dev, "Unable to initialize workqueue\n"); + return -EINVAL; + } + mem_buf_msgq_recv_thr = kthread_create(mem_buf_msgq_recv_fn, NULL, "mem_buf_rcvr"); if (IS_ERR(mem_buf_msgq_recv_thr)) { dev_err(dev, "Failed to create msgq receiver thread rc: %d\n", PTR_ERR(mem_buf_msgq_recv_thr)); - return PTR_ERR(mem_buf_msgq_recv_thr); + ret = PTR_ERR(mem_buf_msgq_recv_thr); + goto err_kthread_create; } mem_buf_hh_msgq_hdl = hh_msgq_register(HH_MSGQ_LABEL_MEMBUF); @@ -2167,6 +2175,9 @@ err_cdev_add: err_msgq_register: kthread_stop(mem_buf_msgq_recv_thr); mem_buf_msgq_recv_thr = NULL; +err_kthread_create: + destroy_workqueue(mem_buf_wq); + mem_buf_wq = NULL; return ret; } @@ -2191,6 +2202,8 @@ static int mem_buf_remove(struct platform_device *pdev) mem_buf_hh_msgq_hdl = NULL; kthread_stop(mem_buf_msgq_recv_thr); mem_buf_msgq_recv_thr = NULL; + destroy_workqueue(mem_buf_wq); + mem_buf_wq = NULL; return 0; } From 5e3ebb30e21b0884de4137dc471a3bfda0cdf378 Mon Sep 17 00:00:00 2001 From: "Isaac J. Manjarres" Date: Tue, 18 Aug 2020 08:11:44 -0700 Subject: [PATCH 2/2] soc: qcom: mem-buf: Relinquish unclaimed donated memory A memory request may not be satisfied in time because of delays in allocation times. When the allocation response is given to the consumer, but the consumer's request has already timed out, the memory will be leaked, as it won't be claimed by the consumer, and it won't be relinquished to the supplier. Thus, to prevent the memory from being leaked, relinquish unclaimed memory back to the supplier. Change-Id: I91f52113c26f4536789fe6807175c845cc9c9e26 Signed-off-by: Isaac J. Manjarres --- drivers/soc/qcom/mem-buf.c | 56 ++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/drivers/soc/qcom/mem-buf.c b/drivers/soc/qcom/mem-buf.c index 3c8c0f59a05e..8bdc163bbcb9 100644 --- a/drivers/soc/qcom/mem-buf.c +++ b/drivers/soc/qcom/mem-buf.c @@ -719,11 +719,10 @@ static void mem_buf_relinquish_work(struct work_struct *work) kfree(rmt_msg); } -static int mem_buf_decode_alloc_resp(struct mem_buf_txn *txn, void *buf, - size_t size) +static int mem_buf_decode_alloc_resp(void *buf, size_t size, + hh_memparcel_handle_t *ret_hdl) { struct mem_buf_alloc_resp *alloc_resp = buf; - hh_memparcel_handle_t *hdl = txn->resp_buf; if (size != sizeof(*alloc_resp)) { pr_err("%s response received is not of correct size\n", @@ -736,15 +735,42 @@ static int mem_buf_decode_alloc_resp(struct mem_buf_txn *txn, void *buf, pr_err("%s remote allocation failed rc: %d\n", __func__, alloc_resp->ret); else - *hdl = alloc_resp->hdl; + *ret_hdl = alloc_resp->hdl; return alloc_resp->ret; } +static void mem_buf_relinquish_mem(u32 memparcel_hdl); + +static void mem_buf_process_alloc_resp(struct mem_buf_msg_hdr *hdr, void *buf, + size_t size) +{ + struct mem_buf_txn *txn; + hh_memparcel_handle_t hdl; + + mutex_lock(&mem_buf_idr_mutex); + txn = idr_find(&mem_buf_txn_idr, hdr->txn_id); + if (!txn) { + pr_err("%s no txn associated with id: %d\n", __func__, + hdr->txn_id); + /* + * If this was a legitimate allocation, we should let the + * allocator know that the memory is not in use, so that + * it can be reclaimed. + */ + if (!mem_buf_decode_alloc_resp(buf, size, &hdl)) + mem_buf_relinquish_mem(hdl); + } else { + txn->txn_ret = mem_buf_decode_alloc_resp(buf, size, + txn->resp_buf); + complete(&txn->txn_done); + } + mutex_unlock(&mem_buf_idr_mutex); +} + static void mem_buf_process_msg(void *buf, size_t size) { struct mem_buf_msg_hdr *hdr = buf; - struct mem_buf_txn *txn; struct mem_buf_rmt_msg *rmt_msg; work_func_t work_fn; @@ -758,17 +784,7 @@ static void mem_buf_process_msg(void *buf, size_t size) if ((hdr->msg_type == MEM_BUF_ALLOC_RESP) && (mem_buf_capability & MEM_BUF_CAP_CONSUMER)) { - mutex_lock(&mem_buf_idr_mutex); - txn = idr_find(&mem_buf_txn_idr, hdr->txn_id); - if (!txn) { - pr_err("%s no txn associated with id: %d\n", __func__, - hdr->txn_id); - } else { - txn->txn_ret = mem_buf_decode_alloc_resp(txn, buf, - size); - complete(&txn->txn_done); - } - mutex_unlock(&mem_buf_idr_mutex); + mem_buf_process_alloc_resp(hdr, buf, size); kfree(buf); } else if ((hdr->msg_type == MEM_BUF_ALLOC_REQ || hdr->msg_type == MEM_BUF_ALLOC_RELINQUISH) && @@ -916,7 +932,7 @@ out: return ret; } -static void mem_buf_relinquish_mem(struct mem_buf_desc *membuf) +static void mem_buf_relinquish_mem(u32 memparcel_hdl) { struct mem_buf_alloc_relinquish *msg; int ret; @@ -926,7 +942,7 @@ static void mem_buf_relinquish_mem(struct mem_buf_desc *membuf) return; msg->hdr.msg_type = MEM_BUF_ALLOC_RELINQUISH; - msg->hdl = membuf->memparcel_hdl; + msg->hdl = memparcel_hdl; trace_send_relinquish_msg(msg); ret = hh_msgq_send(mem_buf_hh_msgq_hdl, msg, sizeof(*msg), 0); @@ -1339,7 +1355,7 @@ static int mem_buf_buffer_release(struct inode *inode, struct file *filp) if (ret < 0) goto out_free_mem; - mem_buf_relinquish_mem(membuf); + mem_buf_relinquish_mem(membuf->memparcel_hdl); out_free_mem: mem_buf_free_mem_type_data(membuf->dst_mem_type, membuf->dst_data); @@ -1451,7 +1467,7 @@ err_map_mem_s1: if (mem_buf_unmap_mem_s2(membuf->memparcel_hdl) < 0) goto err_mem_req; err_map_mem_s2: - mem_buf_relinquish_mem(membuf); + mem_buf_relinquish_mem(membuf->memparcel_hdl); err_mem_req: mem_buf_free_mem_type_data(membuf->dst_mem_type, membuf->dst_data); err_alloc_dst_data: