Date: Tue, 9 Jan 2018 00:10:59 +0000 (UTC) From: Scott Long <scottl@FreeBSD.org> 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 Message-ID: <201801090010.w090AxKX071496@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201801090010.w090AxKX071496>