Browse Source

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 <[email protected]>
Vikram Sharma 5 years ago
parent
commit
88aeb5cd04
1 changed files with 56 additions and 40 deletions
  1. 56 40
      drivers/cam_req_mgr/cam_req_mgr_core.c

+ 56 - 40
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 <linux/module.h>
@@ -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;