svn commit: r358864 - head/sys/cam

[Warner Losh]

Author: imp
Date: Tue Mar 10 23:59:58 2020
New Revision: 358864
URL: https://svnweb.freebsd.org/changeset/base/358864

Log:
  Eliminate xpt_copy_path.
  
  It's used in exactly one place. In that place it's used so we can hold the lock
  on the device associated with the path (since we do a xpt_path_lock and unlock
  pair around the callback). Instead, inline taking and dropping the reference to
  the device so we can ensure we can unlock the mutex after the callback finishes
  if the path in the ccb that's queued to be processed by xpt_scanner_thread is
  destroyed while being processed. We don't actually need the path itself for
  anything other than dereferencing it to get the device to do the lock and
  unlock.
  
  This also makes the locking / use model for cam_path a little cleaner by
  eliminating a case where we needlessly copy the object.
  
  Reviewed by: chuck, chs, ken
  Differential Revision:	https://reviews.freebsd.org/D24008

Modified:
  head/sys/cam/cam_xpt.c
  head/sys/cam/cam_xpt.h

Modified: head/sys/cam/cam_xpt.c
==============================================================================
--- head/sys/cam/cam_xpt.c	Tue Mar 10 23:58:41 2020	(r358863)
+++ head/sys/cam/cam_xpt.c	Tue Mar 10 23:59:58 2020	(r358864)
@@ -803,7 +803,8 @@ static void
 xpt_scanner_thread(void *dummy)
 {
 	union ccb	*ccb;
-	struct cam_path	 path;
+	struct mtx	*mtx;
+	struct cam_ed	*device;
 
 	xpt_lock_buses();
 	for (;;) {
@@ -815,15 +816,22 @@ xpt_scanner_thread(void *dummy)
 			xpt_unlock_buses();
 
 			/*
-			 * Since lock can be dropped inside and path freed
-			 * by completion callback even before return here,
-			 * take our own path copy for reference.
+			 * We need to lock the device's mutex which we use as
+			 * the path mutex. We can't do it directly because the
+			 * cam_path in the ccb may wind up going away because
+			 * the path lock may be dropped and the path retired in
+			 * the completion callback. We do this directly to keep
+			 * the reference counts in cam_path sane. We also have
+			 * to copy the device pointer because ccb_h.path may
+			 * be freed in the callback.
 			 */
-			xpt_copy_path(&path, ccb->ccb_h.path);
-			xpt_path_lock(&path);
+			mtx = xpt_path_mtx(ccb->ccb_h.path);
+			device = ccb->ccb_h.path->device;
+			xpt_acquire_device(device);
+			mtx_lock(mtx);
 			xpt_action(ccb);
-			xpt_path_unlock(&path);
-			xpt_release_path(&path);
+			mtx_unlock(mtx);
+			xpt_release_device(device);
 
 			xpt_lock_buses();
 		}
@@ -3686,15 +3694,6 @@ xpt_clone_path(struct cam_path **new_path_ptr, struct 
 	new_path = (struct cam_path *)malloc(sizeof(*path), M_CAMPATH, M_NOWAIT);
 	if (new_path == NULL)
 		return(CAM_RESRC_UNAVAIL);
-	xpt_copy_path(new_path, path);
-	*new_path_ptr = new_path;
-	return (CAM_REQ_CMP);
-}
-
-void
-xpt_copy_path(struct cam_path *new_path, struct cam_path *path)
-{
-
 	*new_path = *path;
 	if (path->bus != NULL)
 		xpt_acquire_bus(path->bus);
@@ -3702,6 +3701,8 @@ xpt_copy_path(struct cam_path *new_path, struct cam_pa
 		xpt_acquire_target(path->target);
 	if (path->device != NULL)
 		xpt_acquire_device(path->device);
+	*new_path_ptr = new_path;
+	return (CAM_REQ_CMP);
 }
 
 void

Modified: head/sys/cam/cam_xpt.h
==============================================================================
--- head/sys/cam/cam_xpt.h	Tue Mar 10 23:58:41 2020	(r358863)
+++ head/sys/cam/cam_xpt.h	Tue Mar 10 23:59:58 2020	(r358864)
@@ -140,8 +140,6 @@ cam_status		xpt_compile_path(struct cam_path *new_path
 					 lun_id_t lun_id);
 cam_status		xpt_clone_path(struct cam_path **new_path,
 				      struct cam_path *path);
-void			xpt_copy_path(struct cam_path *new_path,
-				      struct cam_path *path);
 
 void			xpt_release_path(struct cam_path *path);