From owner-svn-src-head@freebsd.org Tue Jan 9 00:11:00 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 780C0E6DA76; Tue, 9 Jan 2018 00:11:00 +0000 (UTC) (envelope-from scottl@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 5326D7C82E; Tue, 9 Jan 2018 00:11:00 +0000 (UTC) (envelope-from scottl@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 A739271AF; Tue, 9 Jan 2018 00:10:59 +0000 (UTC) (envelope-from scottl@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w090Axab071499; Tue, 9 Jan 2018 00:10:59 GMT (envelope-from scottl@FreeBSD.org) Received: (from scottl@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w090AxKX071496; Tue, 9 Jan 2018 00:10:59 GMT (envelope-from scottl@FreeBSD.org) Message-Id: <201801090010.w090AxKX071496@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: scottl set sender to scottl@FreeBSD.org using -f From: Scott Long Date: Tue, 9 Jan 2018 00:10:59 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r327710 - in head/sys/cam: . ata scsi X-SVN-Group: head X-SVN-Commit-Author: scottl X-SVN-Commit-Paths: in head/sys/cam: . ata scsi X-SVN-Commit-Revision: 327710 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.25 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, 09 Jan 2018 00:11:00 -0000 Author: scottl Date: Tue Jan 9 00:10:59 2018 New Revision: 327710 URL: https://svnweb.freebsd.org/changeset/base/327710 Log: Don't hold the periph lock when calling into cam_periph_runccb() from the ada and da dump routines. This avoids difficult locking problems from needing to be handled. While it might seem like this would leave the periphs unprotected during dump, they were aleady at risk of unexpected removal due to the dump functions not keeping refcount state across the many calls that come in during a dump. This is an exercise for future work. Obtained from: Netflix Modified: head/sys/cam/ata/ata_da.c head/sys/cam/cam_periph.c head/sys/cam/scsi/scsi_da.c Modified: head/sys/cam/ata/ata_da.c ============================================================================== --- head/sys/cam/ata/ata_da.c Tue Jan 9 00:00:55 2018 (r327709) +++ head/sys/cam/ata/ata_da.c Tue Jan 9 00:10:59 2018 (r327710) @@ -1062,15 +1062,12 @@ adadump(void *arg, void *virtual, vm_offset_t physical dp = arg; periph = dp->d_drv1; softc = (struct ada_softc *)periph->softc; - cam_periph_lock(periph); secsize = softc->params.secsize; lba = offset / secsize; count = length / secsize; - if ((periph->flags & CAM_PERIPH_INVALID) != 0) { - cam_periph_unlock(periph); + if ((periph->flags & CAM_PERIPH_INVALID) != 0) return (ENXIO); - } memset(&ataio, 0, sizeof(ataio)); if (length > 0) { @@ -1098,7 +1095,6 @@ adadump(void *arg, void *virtual, vm_offset_t physical if (error != 0) printf("Aborting dump due to I/O error.\n"); - cam_periph_unlock(periph); return (error); } @@ -1129,7 +1125,6 @@ adadump(void *arg, void *virtual, vm_offset_t physical if (error != 0) xpt_print(periph->path, "Synchronize cache failed\n"); } - cam_periph_unlock(periph); return (error); } Modified: head/sys/cam/cam_periph.c ============================================================================== --- head/sys/cam/cam_periph.c Tue Jan 9 00:00:55 2018 (r327709) +++ head/sys/cam/cam_periph.c Tue Jan 9 00:10:59 2018 (r327710) @@ -1160,8 +1160,6 @@ cam_periph_runccb(union ccb *ccb, struct bintime ltime; int error; bool must_poll; - struct mtx *periph_mtx; - struct cam_periph *periph; uint32_t timeout = 1; starttime = NULL; @@ -1188,20 +1186,20 @@ cam_periph_runccb(union ccb *ccb, * stopped for dumping, except when we call doadump from ddb. While the * scheduler is running in this case, we still need to poll the I/O to * avoid sleeping waiting for the ccb to complete. + * + * XXX To avoid locking problems, dumping/polling callers must call + * without a periph lock held. */ must_poll = dumping; ccb->ccb_h.cbfcnp = cam_periph_done; - periph = xpt_path_periph(ccb->ccb_h.path); - periph_mtx = cam_periph_mtx(periph); /* * If we're polling, then we need to ensure that we have ample resources - * in the periph. We also need to drop the periph lock while we're polling. + * in the periph. * cam_periph_error can reschedule the ccb by calling xpt_action and returning * ERESTART, so we have to effect the polling in the do loop below. */ if (must_poll) { - mtx_unlock(periph_mtx); timeout = xpt_poll_setup(ccb); } @@ -1226,9 +1224,6 @@ cam_periph_runccb(union ccb *ccb, error = 0; } while (error == ERESTART); } - - if (must_poll) - mtx_lock(periph_mtx); if ((ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) { cam_release_devq(ccb->ccb_h.path, Modified: head/sys/cam/scsi/scsi_da.c ============================================================================== --- head/sys/cam/scsi/scsi_da.c Tue Jan 9 00:00:55 2018 (r327709) +++ head/sys/cam/scsi/scsi_da.c Tue Jan 9 00:10:59 2018 (r327710) @@ -1647,13 +1647,10 @@ dadump(void *arg, void *virtual, vm_offset_t physical, dp = arg; periph = dp->d_drv1; softc = (struct da_softc *)periph->softc; - cam_periph_lock(periph); secsize = softc->params.secsize; - if ((softc->flags & DA_FLAG_PACK_INVALID) != 0) { - cam_periph_unlock(periph); + if ((softc->flags & DA_FLAG_PACK_INVALID) != 0) return (ENXIO); - } memset(&csio, 0, sizeof(csio)); if (length > 0) { @@ -1676,7 +1673,6 @@ dadump(void *arg, void *virtual, vm_offset_t physical, 0, SF_NO_RECOVERY | SF_NO_RETRY, NULL); if (error != 0) printf("Aborting dump due to I/O error.\n"); - cam_periph_unlock(periph); return (error); } @@ -1700,7 +1696,6 @@ dadump(void *arg, void *virtual, vm_offset_t physical, if (error != 0) xpt_print(periph->path, "Synchronize cache failed\n"); } - cam_periph_unlock(periph); return (error); }