From c2bd48d7b84786d84a58263ca604dd2ac2264892 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Fri, 19 Jun 2015 17:59:06 -0400 Subject: [PATCH] mdss: rotator: fix memory scribble issue Prior to this patch it was possible for rotator wq to write to memory that had been freed in close session. Close session now offloads the close work to wq if it still has not completed the work. Change-Id: I4b48071f930b912a098094d7de2b2081dac4c2e5 Signed-off-by: Terence Hampson --- drivers/video/fbdev/msm/mdss_rotator.c | 109 +++++++++++------- .../video/fbdev/msm/mdss_rotator_internal.h | 6 +- 2 files changed, 68 insertions(+), 47 deletions(-) diff --git a/drivers/video/fbdev/msm/mdss_rotator.c b/drivers/video/fbdev/msm/mdss_rotator.c index ea7c42505b91..7298e59b8830 100644 --- a/drivers/video/fbdev/msm/mdss_rotator.c +++ b/drivers/video/fbdev/msm/mdss_rotator.c @@ -160,12 +160,13 @@ static unsigned long mdss_rotator_clk_rate_calc( mutex_lock(&private->perf_lock); list_for_each_entry(perf, &private->perf_list, list) { bool rate_accounted_for = false; + mutex_lock(&perf->work_dis_lock); /* * If there is one session that has two work items across * different hw blocks rate is accounted for in both blocks. */ for (i = 0; i < mgr->queue_count; i++) { - if (atomic_read(&perf->work_distribution[i])) { + if (perf->work_distribution[i]) { clk_rate[i] += perf->clk_rate; rate_accounted_for = true; } @@ -180,6 +181,7 @@ static unsigned long mdss_rotator_clk_rate_calc( if ((!rate_accounted_for) && (wb_idx >= 0) && (wb_idx < mgr->queue_count)) clk_rate[wb_idx] += perf->clk_rate; + mutex_unlock(&perf->work_dis_lock); } mutex_unlock(&private->perf_lock); @@ -335,14 +337,14 @@ int mdss_rotator_resource_ctrl(struct mdss_rot_mgr *mgr, int enable) return ret; } +/* caller is expected to hold perf->work_dis_lock lock */ static bool mdss_rotator_is_work_pending(struct mdss_rot_mgr *mgr, struct mdss_rot_perf *perf) { int i; for (i = 0; i < mgr->queue_count; i++) { - atomic_t *wrk_dis = &perf->work_distribution[i]; - if (atomic_read(wrk_dis)) { + if (perf->work_distribution[i]) { pr_debug("Work is still scheduled to complete\n"); return true; } @@ -565,28 +567,35 @@ end: return ret; } -static struct mdss_rot_perf *mdss_rotator_find_session( +static struct mdss_rot_perf *__mdss_rotator_find_session( struct mdss_rot_file_private *private, u32 session_id) { struct mdss_rot_perf *perf, *perf_next; bool found = false; - - mutex_lock(&private->perf_lock); - list_for_each_entry_safe(perf, perf_next, &private->perf_list, list) { if (perf->config.session_id == session_id) { found = true; break; } } - - mutex_unlock(&private->perf_lock); if (!found) perf = NULL; return perf; } +static struct mdss_rot_perf *mdss_rotator_find_session( + struct mdss_rot_file_private *private, + u32 session_id) +{ + struct mdss_rot_perf *perf; + + mutex_lock(&private->perf_lock); + perf = __mdss_rotator_find_session(private, session_id); + mutex_unlock(&private->perf_lock); + return perf; +} + static void mdss_rotator_release_data(struct mdss_rot_entry *entry) { mdss_mdp_data_free(&entry->src_buf, true, DMA_TO_DEVICE); @@ -946,7 +955,6 @@ static void mdss_rotator_queue_request(struct mdss_rot_mgr *mgr, struct mdss_rot_entry *entry; struct mdss_rot_queue *queue; unsigned long clk_rate; - atomic_t *wrk_dis; u32 wb_idx; int i; @@ -954,8 +962,9 @@ static void mdss_rotator_queue_request(struct mdss_rot_mgr *mgr, entry = req->entries + i; queue = entry->queue; wb_idx = queue->hw->wb_id; - wrk_dis = &entry->perf->work_distribution[wb_idx]; - atomic_inc(wrk_dis); + mutex_lock(&entry->perf->work_dis_lock); + entry->perf->work_distribution[wb_idx]++; + mutex_unlock(&entry->perf->work_dis_lock); entry->work_assigned = true; } @@ -1037,6 +1046,7 @@ static int mdss_rotator_update_perf(struct mdss_rot_mgr *mgr) } mutex_lock(&mgr->bus_lock); + total_bw += mgr->pending_close_bw_vote; mdss_rotator_enable_reg_bus(mgr, total_bw); mdss_rotator_bus_scale_set_quota(&mgr->data_bus, total_bw); mutex_unlock(&mgr->bus_lock); @@ -1048,17 +1058,30 @@ static void mdss_rotator_release_from_work_distribution( struct mdss_rot_entry *entry) { if (entry->work_assigned) { - atomic_t *wrk_dis; + bool free_perf = false; u32 wb_idx = entry->queue->hw->wb_id; - wrk_dis = &entry->perf->work_distribution[wb_idx]; - atomic_add_unless(wrk_dis, -1, 0); + mutex_lock(&entry->perf->work_dis_lock); + if (entry->perf->work_distribution[wb_idx]) + entry->perf->work_distribution[wb_idx]--; + + if (!entry->perf->work_distribution[wb_idx] + && list_empty(&entry->perf->list)) { + /* close session has offloaded perf free to us */ + free_perf = true; + } + mutex_unlock(&entry->perf->work_dis_lock); entry->work_assigned = false; - if (entry->perf->waiting_for_completion && - !mdss_rotator_is_work_pending(mgr, - entry->perf)) { - complete(&entry->perf->stop_comp); - entry->perf->waiting_for_completion = false; + if (free_perf) { + mutex_lock(&mgr->bus_lock); + mgr->pending_close_bw_vote -= entry->perf->bw; + mutex_unlock(&mgr->bus_lock); + mdss_rotator_resource_ctrl(mgr, false); + devm_kfree(&mgr->pdev->dev, + entry->perf->work_distribution); + devm_kfree(&mgr->pdev->dev, entry->perf); + mdss_rotator_update_perf(mgr); + entry->perf = NULL; } } } @@ -1742,7 +1765,7 @@ static int mdss_rotator_open_session(struct mdss_rot_mgr *mgr, { struct mdp_rotation_config config; struct mdss_rot_perf *perf; - int ret, i; + int ret; ret = copy_from_user(&config, (void __user *)arg, sizeof(config)); if (ret) { @@ -1763,7 +1786,7 @@ static int mdss_rotator_open_session(struct mdss_rot_mgr *mgr, } perf->work_distribution = devm_kzalloc(&mgr->pdev->dev, - sizeof(atomic_t) * mgr->queue_count, GFP_KERNEL); + sizeof(u32) * mgr->queue_count, GFP_KERNEL); if (!perf->work_distribution) { pr_err("fail to allocate work_distribution\n"); ret = -ENOMEM; @@ -1773,9 +1796,7 @@ static int mdss_rotator_open_session(struct mdss_rot_mgr *mgr, config.session_id = mdss_rotator_generator_session_id(mgr); perf->config = config; perf->last_wb_idx = -1; - init_completion(&perf->stop_comp); - for (i = 0; i < mgr->queue_count; i++) - atomic_set(&perf->work_distribution[i], 0); + mutex_init(&perf->work_dis_lock); INIT_LIST_HEAD(&perf->list); @@ -1825,37 +1846,37 @@ static int mdss_rotator_close_session(struct mdss_rot_mgr *mgr, struct mdss_rot_file_private *private, unsigned long arg) { struct mdss_rot_perf *perf; + bool offload_release_work = false; u32 id; id = (u32)arg; - perf = mdss_rotator_find_session(private, id); + mutex_lock(&mgr->lock); + mutex_lock(&private->perf_lock); + perf = __mdss_rotator_find_session(private, id); if (!perf) { + mutex_unlock(&private->perf_lock); + mutex_unlock(&mgr->lock); pr_err("Trying to close session that does not exist\n"); return -EINVAL; } - /* - * Client should not be calling close session when there - * are pending rotation work. Driver will attempt to - * wait for request to complete. - */ - + mutex_lock(&perf->work_dis_lock); if (mdss_rotator_is_work_pending(mgr, perf)) { - pr_debug("Work is still pending, attempting to wait\n"); - reinit_completion(&perf->stop_comp); - perf->waiting_for_completion = true; - wait_for_completion_timeout(&perf->stop_comp, 3 * KOFF_TIMEOUT); - - if (mdss_rotator_is_work_pending(mgr, perf)) { - pr_err("Calling session close, rot work is pending\n"); - return -EBUSY; - } + pr_debug("Work is still pending, offload free to wq\n"); + mutex_lock(&mgr->bus_lock); + mgr->pending_close_bw_vote += perf->bw; + mutex_unlock(&mgr->bus_lock); + offload_release_work = true; } + list_del_init(&perf->list); + mutex_unlock(&perf->work_dis_lock); + mutex_unlock(&private->perf_lock); + mutex_unlock(&mgr->lock); + + if (offload_release_work) + return 0; mdss_rotator_resource_ctrl(mgr, false); - mutex_lock(&private->perf_lock); - list_del_init(&perf->list); - mutex_unlock(&private->perf_lock); devm_kfree(&mgr->pdev->dev, perf->work_distribution); devm_kfree(&mgr->pdev->dev, perf); mdss_rotator_update_perf(mgr); diff --git a/drivers/video/fbdev/msm/mdss_rotator_internal.h b/drivers/video/fbdev/msm/mdss_rotator_internal.h index f85fba869276..30ea08f4b0dd 100644 --- a/drivers/video/fbdev/msm/mdss_rotator_internal.h +++ b/drivers/video/fbdev/msm/mdss_rotator_internal.h @@ -110,9 +110,8 @@ struct mdss_rot_perf { struct mdp_rotation_config config; unsigned long clk_rate; u64 bw; - bool waiting_for_completion; - struct completion stop_comp; - atomic_t *work_distribution; + struct mutex work_dis_lock; + u32 *work_distribution; int last_wb_idx; /* last known wb index, used when above count is 0 */ }; @@ -164,6 +163,7 @@ struct mdss_rot_mgr { struct list_head file_list; struct mutex bus_lock; + u64 pending_close_bw_vote; struct mdss_rot_bus_data_type data_bus; struct mdss_rot_bus_data_type reg_bus;