From 88aeb5cd042ec1b6b5670115251e545483a79419 Mon Sep 17 00:00:00 2001 From: Vikram Sharma Date: Tue, 14 Jan 2020 14:44:43 +0530 Subject: [PATCH] msm: camera: req_mgr: Update link activate/deactivate to avoid race When cam request manager gets a request to deactivate a link, we do pause for each of the device for that link. There is a race here in current scenario that workqueues can be scheduled even after the link has been deactivated. This can lead to unexpected behavior. This change has updated the activate and deactivate handling to take care of the race. CRs-Fixed: 2601863 Change-Id: I7ff03c74c240fc3250618db518d586531d87369f Signed-off-by: Vikram Sharma --- drivers/cam_req_mgr/cam_req_mgr_core.c | 96 +++++++++++++++----------- 1 file changed, 56 insertions(+), 40 deletions(-) diff --git a/drivers/cam_req_mgr/cam_req_mgr_core.c b/drivers/cam_req_mgr/cam_req_mgr_core.c index 2d5e12c09522..8071261e6bcb 100644 --- a/drivers/cam_req_mgr/cam_req_mgr_core.c +++ b/drivers/cam_req_mgr/cam_req_mgr_core.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2016-2020, The Linux Foundation. All rights reserved. */ #include @@ -533,32 +533,40 @@ static void __cam_req_mgr_validate_crm_wd_timer( CAM_DBG(CAM_CRM, "rd_idx: %d idx: %d current_frame_timeout: %d ms", in_q->rd_idx, idx, current_frame_timeout); - - if ((next_frame_timeout + CAM_REQ_MGR_WATCHDOG_TIMEOUT) > - link->watchdog->expires) { - CAM_DBG(CAM_CRM, - "Modifying wd timer expiry from %d ms to %d ms", - link->watchdog->expires, - (next_frame_timeout + CAM_REQ_MGR_WATCHDOG_TIMEOUT)); - crm_timer_modify(link->watchdog, - next_frame_timeout + - CAM_REQ_MGR_WATCHDOG_TIMEOUT); - } else if (current_frame_timeout) { - CAM_DBG(CAM_CRM, - "Reset wd timer to current frame from %d ms to %d ms", - link->watchdog->expires, - (current_frame_timeout + CAM_REQ_MGR_WATCHDOG_TIMEOUT)); - crm_timer_modify(link->watchdog, - current_frame_timeout + - CAM_REQ_MGR_WATCHDOG_TIMEOUT); - } else if (link->watchdog->expires > - CAM_REQ_MGR_WATCHDOG_TIMEOUT) { - CAM_DBG(CAM_CRM, - "Reset wd timer to default from %d ms to %d ms", - link->watchdog->expires, CAM_REQ_MGR_WATCHDOG_TIMEOUT); - crm_timer_modify(link->watchdog, - CAM_REQ_MGR_WATCHDOG_TIMEOUT); + spin_lock_bh(&link->link_state_spin_lock); + if (link->watchdog) { + if ((next_frame_timeout + CAM_REQ_MGR_WATCHDOG_TIMEOUT) > + link->watchdog->expires) { + CAM_DBG(CAM_CRM, + "Modifying wd timer expiry from %d ms to %d ms", + link->watchdog->expires, + (next_frame_timeout + + CAM_REQ_MGR_WATCHDOG_TIMEOUT)); + crm_timer_modify(link->watchdog, + next_frame_timeout + + CAM_REQ_MGR_WATCHDOG_TIMEOUT); + } else if (current_frame_timeout) { + CAM_DBG(CAM_CRM, + "Reset wd timer to frame from %d ms to %d ms", + link->watchdog->expires, + (current_frame_timeout + + CAM_REQ_MGR_WATCHDOG_TIMEOUT)); + crm_timer_modify(link->watchdog, + current_frame_timeout + + CAM_REQ_MGR_WATCHDOG_TIMEOUT); + } else if (link->watchdog->expires > + CAM_REQ_MGR_WATCHDOG_TIMEOUT) { + CAM_DBG(CAM_CRM, + "Reset wd timer to default from %d ms to %d ms", + link->watchdog->expires, + CAM_REQ_MGR_WATCHDOG_TIMEOUT); + crm_timer_modify(link->watchdog, + CAM_REQ_MGR_WATCHDOG_TIMEOUT); + } + } else { + CAM_WARN(CAM_CRM, "Watchdog timer exited already"); } + spin_unlock_bh(&link->link_state_spin_lock); } /** @@ -1712,6 +1720,14 @@ static int __cam_req_mgr_process_sof_freeze(void *priv, void *data) link = (struct cam_req_mgr_core_link *)priv; session = (struct cam_req_mgr_core_session *)link->parent; + spin_lock_bh(&link->link_state_spin_lock); + if ((link->watchdog) && (link->watchdog->pause_timer)) { + CAM_INFO(CAM_CRM, "Watchdog Paused"); + spin_unlock_bh(&link->link_state_spin_lock); + return rc; + } + spin_unlock_bh(&link->link_state_spin_lock); + CAM_ERR(CAM_CRM, "SOF freeze for session %d link 0x%x", session->session_hdl, link->link_hdl); @@ -1756,9 +1772,6 @@ static void __cam_req_mgr_sof_freeze(struct timer_list *timer_data) link = (struct cam_req_mgr_core_link *)timer->parent; - if (link->watchdog->pause_timer) - return; - task = cam_req_mgr_workq_get_task(link->workq); if (!task) { CAM_ERR(CAM_CRM, "No empty task"); @@ -2694,11 +2707,9 @@ static int cam_req_mgr_cb_notify_timer( rc = -EPERM; goto end; } - spin_unlock_bh(&link->link_state_spin_lock); - - - if (!timer_data->state) + if ((link->watchdog) && (!timer_data->state)) link->watchdog->pause_timer = true; + spin_unlock_bh(&link->link_state_spin_lock); end: return rc; @@ -2744,7 +2755,7 @@ static int cam_req_mgr_cb_notify_trigger( goto end; } - if (link->watchdog->pause_timer) + if ((link->watchdog) && (link->watchdog->pause_timer)) link->watchdog->pause_timer = false; crm_timer_reset(link->watchdog); @@ -3029,14 +3040,15 @@ static int __cam_req_mgr_unlink(struct cam_req_mgr_core_link *link) /* Destroy workq of link */ cam_req_mgr_workq_destroy(&link->workq); - + spin_lock_bh(&link->link_state_spin_lock); /* Destroy timer of link */ crm_timer_exit(&link->watchdog); + spin_unlock_bh(&link->link_state_spin_lock); /* Cleanup request tables and unlink devices */ __cam_req_mgr_destroy_link_info(link); - /* Free memory holding data of linked devs */ + __cam_req_mgr_destroy_subdev(link->l_dev); /* Destroy the link handle */ @@ -3664,6 +3676,9 @@ int cam_req_mgr_link_control(struct cam_req_mgr_link_control *control) mutex_lock(&link->lock); if (control->ops == CAM_REQ_MGR_LINK_ACTIVATE) { + spin_lock_bh(&link->link_state_spin_lock); + link->state = CAM_CRM_LINK_STATE_READY; + spin_unlock_bh(&link->link_state_spin_lock); /* Start SOF watchdog timer */ rc = crm_timer_init(&link->watchdog, CAM_REQ_MGR_WATCHDOG_TIMEOUT, link, @@ -3685,10 +3700,6 @@ int cam_req_mgr_link_control(struct cam_req_mgr_link_control *control) dev->ops->process_evt(&evt_data); } } else if (control->ops == CAM_REQ_MGR_LINK_DEACTIVATE) { - /* Destroy SOF watchdog timer */ - spin_lock_bh(&link->link_state_spin_lock); - crm_timer_exit(&link->watchdog); - spin_unlock_bh(&link->link_state_spin_lock); /* notify nodes */ for (j = 0; j < link->num_devs; j++) { dev = &link->l_dev[j]; @@ -3699,6 +3710,11 @@ int cam_req_mgr_link_control(struct cam_req_mgr_link_control *control) if (dev->ops && dev->ops->process_evt) dev->ops->process_evt(&evt_data); } + /* Destroy SOF watchdog timer */ + spin_lock_bh(&link->link_state_spin_lock); + link->state = CAM_CRM_LINK_STATE_IDLE; + crm_timer_exit(&link->watchdog); + spin_unlock_bh(&link->link_state_spin_lock); } else { CAM_ERR(CAM_CRM, "Invalid link control command"); rc = -EINVAL;