From owner-svn-src-head@freebsd.org Tue Mar 10 23:59:59 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 6DABC273FF5; Tue, 10 Mar 2020 23:59:59 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 48cXC31gWYz41bm; Tue, 10 Mar 2020 23:59:59 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 212B83AA8; Tue, 10 Mar 2020 23:59:59 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 02ANxxUk057254; Tue, 10 Mar 2020 23:59:59 GMT (envelope-from imp@FreeBSD.org) Received: (from imp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 02ANxwLj057251; Tue, 10 Mar 2020 23:59:58 GMT (envelope-from imp@FreeBSD.org) Message-Id: <202003102359.02ANxwLj057251@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: imp set sender to imp@FreeBSD.org using -f From: Warner Losh Date: Tue, 10 Mar 2020 23:59:58 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r358864 - head/sys/cam X-SVN-Group: head X-SVN-Commit-Author: imp X-SVN-Commit-Paths: head/sys/cam X-SVN-Commit-Revision: 358864 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Mar 2020 23:59:59 -0000 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);