svn commit: r327710 - in head/sys/cam: . ata scsi

[Scott Long]

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);
 }