From fd763c3274cd89ec15f61ddb306ed523a252a505 Mon Sep 17 00:00:00 2001 From: Herman Chen Date: Wed, 2 Jun 2021 11:53:23 +0800 Subject: [PATCH] video: rockchip: mpp: Fix mpp_free_task crash Due to hardware timeout the session may be released before the remaining task is alll finished. So mpp_free_task could run after mpp_dev_release. Then the session used in mpp_free_task could be invalid and crash the thread. So we attach session to the mpp_taskqueue and let the work thread to destroy and release session later. NOTE: the session is created in mpp_dev_open and attached to corresponding taskqueue on client init. So when we release the session in mpp_dev_release if the session is not attached to certain taskqueue just release it immediately. Signed-off-by: Herman Chen Change-Id: I9d4b2358154522e6bcde6e688592c0058781529a --- drivers/video/rockchip/mpp/mpp_common.c | 221 ++++++++++++++++------- drivers/video/rockchip/mpp/mpp_common.h | 13 ++ drivers/video/rockchip/mpp/mpp_debug.h | 17 ++ drivers/video/rockchip/mpp/mpp_service.c | 2 +- 4 files changed, 182 insertions(+), 71 deletions(-) diff --git a/drivers/video/rockchip/mpp/mpp_common.c b/drivers/video/rockchip/mpp/mpp_common.c index 3d3630c8116d..7c0c8d0ad244 100644 --- a/drivers/video/rockchip/mpp/mpp_common.c +++ b/drivers/video/rockchip/mpp/mpp_common.c @@ -220,6 +220,129 @@ int mpp_power_off(struct mpp_dev *mpp) return 0; } +static int mpp_session_clear(struct mpp_dev *mpp, + struct mpp_session *session) +{ + struct mpp_task *task = NULL, *n; + + /* clear session done list */ + mutex_lock(&session->done_lock); + list_for_each_entry_safe(task, n, + &session->done_list, + done_link) { + list_del_init(&task->done_link); + kref_put(&task->ref, mpp_free_task); + } + mutex_unlock(&session->done_lock); + + /* clear session pending list */ + mutex_lock(&session->pending_lock); + list_for_each_entry_safe(task, n, + &session->pending_list, + pending_link) { + /* abort task in taskqueue */ + atomic_inc(&task->abort_request); + list_del_init(&task->pending_link); + kref_put(&task->ref, mpp_free_task); + } + mutex_unlock(&session->pending_lock); + + return 0; +} + +static struct mpp_session *mpp_session_init(void) +{ + struct mpp_session *session = kzalloc(sizeof(*session), GFP_KERNEL); + + if (!session) + return NULL; + + session->pid = current->pid; + + mutex_init(&session->pending_lock); + mutex_init(&session->done_lock); + INIT_LIST_HEAD(&session->pending_list); + INIT_LIST_HEAD(&session->done_list); + INIT_LIST_HEAD(&session->service_link); + INIT_LIST_HEAD(&session->session_link); + + init_waitqueue_head(&session->wait); + atomic_set(&session->task_count, 0); + atomic_set(&session->release_request, 0); + + mpp_dbg_session("session %p init\n", session); + return session; +} + +int mpp_session_deinit(struct mpp_session *session) +{ + u32 task_count = atomic_read(&session->task_count); + + mpp_dbg_session("session %p:%d task %d release\n", + session, session->index, task_count); + if (task_count) + return -1; + + if (session->mpp) { + struct mpp_dev *mpp = session->mpp; + + if (mpp->dev_ops->free_session) + mpp->dev_ops->free_session(session); + + mpp_session_clear(mpp, session); + + if (session->dma) { + mpp_iommu_down_read(mpp->iommu_info); + mpp_dma_session_destroy(session->dma); + mpp_iommu_up_read(mpp->iommu_info); + session->dma = NULL; + } + } + + if (session->srv) { + struct mpp_service *srv = session->srv; + + mutex_lock(&srv->session_lock); + list_del_init(&session->service_link); + mutex_unlock(&srv->session_lock); + } + + list_del_init(&session->session_link); + + mpp_dbg_session("session %p:%d deinit\n", session, session->index); + + kfree(session); + return 0; +} + +static void mpp_session_attach_workqueue(struct mpp_session *session, + struct mpp_taskqueue *queue) +{ + mpp_dbg_session("session %p:%d attach\n", session, session->index); + mutex_lock(&queue->session_lock); + list_add_tail(&session->session_link, &queue->session_attach); + mutex_unlock(&queue->session_lock); +} + +static void mpp_session_detach_workqueue(struct mpp_session *session) +{ + struct mpp_taskqueue *queue; + + if (!session->mpp || !session->mpp->queue) + return; + + mpp_dbg_session("session %p:%d detach\n", session, session->index); + queue = session->mpp->queue; + + mutex_lock(&queue->session_lock); + list_del_init(&session->session_link); + list_add_tail(&session->session_link, &queue->session_detach); + atomic_inc(&queue->detach_count); + mutex_unlock(&queue->session_lock); + + mpp_taskqueue_trigger_work(session->mpp); +} + static int mpp_session_push_pending(struct mpp_session *session, struct mpp_task *task) @@ -588,37 +711,21 @@ static void mpp_task_try_run(struct kthread_work *work_s) } done: - mpp_debug_leave(); -} + if (atomic_read(&queue->detach_count)) { + struct mpp_session *session = NULL, *n; -static int mpp_session_clear(struct mpp_dev *mpp, - struct mpp_session *session) -{ - struct mpp_task *task = NULL, *n; + mpp_dbg_session("%s detach count %d\n", dev_name(mpp->dev), + atomic_read(&queue->detach_count)); - /* clear session done list */ - mutex_lock(&session->done_lock); - list_for_each_entry_safe(task, n, - &session->done_list, - done_link) { - list_del_init(&task->done_link); - kref_put(&task->ref, mpp_free_task); + mutex_lock(&queue->session_lock); + list_for_each_entry_safe(session, n, &queue->session_detach, + session_link) { + if (!mpp_session_deinit(session)) + atomic_dec(&queue->detach_count); + } + + mutex_unlock(&queue->session_lock); } - mutex_unlock(&session->done_lock); - - /* clear session pending list */ - mutex_lock(&session->pending_lock); - list_for_each_entry_safe(task, n, - &session->pending_list, - pending_link) { - /* abort task in taskqueue */ - atomic_inc(&task->abort_request); - list_del_init(&task->pending_link); - kref_put(&task->ref, mpp_free_task); - } - mutex_unlock(&session->pending_lock); - - return 0; } static int mpp_wait_result(struct mpp_session *session, @@ -785,14 +892,19 @@ struct mpp_taskqueue *mpp_taskqueue_init(struct device *dev) if (!queue) return NULL; + mutex_init(&queue->session_lock); mutex_init(&queue->pending_lock); mutex_init(&queue->running_lock); mutex_init(&queue->mmu_lock); mutex_init(&queue->dev_lock); + INIT_LIST_HEAD(&queue->session_attach); + INIT_LIST_HEAD(&queue->session_detach); INIT_LIST_HEAD(&queue->pending_list); INIT_LIST_HEAD(&queue->running_list); INIT_LIST_HEAD(&queue->mmu_list); INIT_LIST_HEAD(&queue->dev_list); + + atomic_set(&queue->detach_count, 0); /* default taskqueue has max 16 task capacity */ queue->task_capacity = MPP_MAX_TASK_CAPACITY; @@ -972,6 +1084,7 @@ static int mpp_process_request(struct mpp_session *session, if (ret) return ret; } + mpp_session_attach_workqueue(session, mpp->queue); } break; case MPP_CMD_INIT_DRIVER_DATA: { u32 val; @@ -1206,26 +1319,17 @@ static int mpp_dev_open(struct inode *inode, struct file *filp) mpp_cdev); mpp_debug_enter(); - session = kzalloc(sizeof(*session), GFP_KERNEL); + session = mpp_session_init(); if (!session) return -ENOMEM; session->srv = srv; - session->pid = current->pid; - mutex_init(&session->pending_lock); - mutex_init(&session->done_lock); - INIT_LIST_HEAD(&session->pending_list); - INIT_LIST_HEAD(&session->done_list); - INIT_LIST_HEAD(&session->session_link); - - init_waitqueue_head(&session->wait); - atomic_set(&session->task_count, 0); - atomic_set(&session->release_request, 0); - - mutex_lock(&srv->session_lock); - list_add_tail(&session->session_link, &srv->session_list); - mutex_unlock(&srv->session_lock); + if (session->srv) { + mutex_lock(&srv->session_lock); + list_add_tail(&session->service_link, &srv->session_list); + mutex_unlock(&srv->session_lock); + } filp->private_data = (void *)session; mpp_debug_leave(); @@ -1235,9 +1339,6 @@ static int mpp_dev_open(struct inode *inode, struct file *filp) static int mpp_dev_release(struct inode *inode, struct file *filp) { - int ret; - int val; - struct mpp_dev *mpp; struct mpp_session *session = filp->private_data; mpp_debug_enter(); @@ -1249,32 +1350,12 @@ static int mpp_dev_release(struct inode *inode, struct file *filp) /* wait for task all done */ atomic_inc(&session->release_request); - ret = readx_poll_timeout(atomic_read, - &session->task_count, - val, val == 0, 1000, 500000); - if (ret == -ETIMEDOUT) - mpp_err("session %p, wait task count %d time out\n", - session, atomic_read(&session->task_count)); - wake_up(&session->wait); - /* release device resource */ - mpp = session->mpp; - if (mpp) { - if (mpp->dev_ops->free_session) - mpp->dev_ops->free_session(session); + if (session->mpp) + mpp_session_detach_workqueue(session); + else + mpp_session_deinit(session); - /* remove this filp from the asynchronusly notified filp's */ - mpp_session_clear(mpp, session); - - mpp_iommu_down_read(mpp->iommu_info); - mpp_dma_session_destroy(session->dma); - mpp_iommu_up_read(mpp->iommu_info); - } - mutex_lock(&session->srv->session_lock); - list_del_init(&session->session_link); - mutex_unlock(&session->srv->session_lock); - - kfree(session); filp->private_data = NULL; mpp_debug_leave(); diff --git a/drivers/video/rockchip/mpp/mpp_common.h b/drivers/video/rockchip/mpp/mpp_common.h index 1cf0f19d82f6..a581b27b8d2c 100644 --- a/drivers/video/rockchip/mpp/mpp_common.h +++ b/drivers/video/rockchip/mpp/mpp_common.h @@ -350,6 +350,8 @@ struct mpp_session { u16 trans_table[MPP_MAX_REG_TRANS_NUM]; u32 msg_flags; /* link to mpp_service session_list */ + struct list_head service_link; + /* link to mpp_workqueue session_attach / session_detach */ struct list_head session_link; /* private data */ void *priv; @@ -399,6 +401,14 @@ struct mpp_task { }; struct mpp_taskqueue { + /* lock for session attach and session_detach */ + struct mutex session_lock; + /* link to session session_link for attached sessions */ + struct list_head session_attach; + /* link to session session_link for detached sessions */ + struct list_head session_detach; + atomic_t detach_count; + /* lock for pending list */ struct mutex pending_lock; struct list_head pending_list; @@ -450,6 +460,7 @@ struct mpp_service { struct mpp_taskqueue *task_queues[MPP_DEVICE_BUTT]; u32 reset_group_cnt; struct mpp_reset_group *reset_groups[MPP_DEVICE_BUTT]; + /* lock for session list */ struct mutex session_lock; struct list_head session_list; @@ -543,6 +554,8 @@ int mpp_task_dump_reg(struct mpp_dev *mpp, int mpp_task_dump_hw_reg(struct mpp_dev *mpp, struct mpp_task *task); +int mpp_session_deinit(struct mpp_session *session); + int mpp_dev_probe(struct mpp_dev *mpp, struct platform_device *pdev); int mpp_dev_remove(struct mpp_dev *mpp); diff --git a/drivers/video/rockchip/mpp/mpp_debug.h b/drivers/video/rockchip/mpp/mpp_debug.h index 0c1132f2e947..c279d94a03c8 100644 --- a/drivers/video/rockchip/mpp/mpp_debug.h +++ b/drivers/video/rockchip/mpp/mpp_debug.h @@ -47,6 +47,8 @@ #define DEBUG_GET_PERF_VAL 0x00100000 #define DEBUG_SRAM_INFO 0x00200000 +#define DEBUG_SESSION 0x00400000 + #define PRINT_FUNCTION 0x80000000 #define PRINT_LINE 0x40000000 @@ -88,4 +90,19 @@ extern unsigned int mpp_dev_debug; #define mpp_err(fmt, args...) \ pr_err("%s:%d: " fmt, __func__, __LINE__, ##args) +#define mpp_dbg_link_flow(fmt, args...) \ + do { \ + if (unlikely(mpp_dev_debug & DEBUG_LINK_TABLE)) { \ + pr_info("%s:%d: " fmt, \ + __func__, __LINE__, ##args); \ + } \ + } while (0) + +#define mpp_dbg_session(fmt, args...) \ + do { \ + if (unlikely(mpp_dev_debug & DEBUG_SESSION)) { \ + pr_info(fmt, ##args); \ + } \ + } while (0) + #endif diff --git a/drivers/video/rockchip/mpp/mpp_service.c b/drivers/video/rockchip/mpp/mpp_service.c index a25232a65ff2..b404c726d624 100644 --- a/drivers/video/rockchip/mpp/mpp_service.c +++ b/drivers/video/rockchip/mpp/mpp_service.c @@ -170,7 +170,7 @@ static int mpp_show_session_summary(struct seq_file *seq, void *offset) mutex_lock(&srv->session_lock); list_for_each_entry_safe(session, n, &srv->session_list, - session_link) { + service_link) { struct mpp_dev *mpp; if (!session->priv)