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); _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
