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
This commit is contained in:
Scott Long 2018-01-09 00:10:59 +00:00
parent 329e7a8b51
commit 04e814aecd
3 changed files with 6 additions and 21 deletions

View File

@ -1062,15 +1062,12 @@ adadump(void *arg, void *virtual, vm_offset_t physical, off_t offset, size_t len
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, off_t offset, size_t len
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, off_t offset, size_t len
if (error != 0)
xpt_print(periph->path, "Synchronize cache failed\n");
}
cam_periph_unlock(periph);
return (error);
}

View File

@ -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);
}
@ -1227,9 +1225,6 @@ cam_periph_runccb(union ccb *ccb,
} 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,
/* relsim_flags */0,

View File

@ -1647,13 +1647,10 @@ dadump(void *arg, void *virtual, vm_offset_t physical, off_t offset, size_t leng
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, off_t offset, size_t leng
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, off_t offset, size_t leng
if (error != 0)
xpt_print(periph->path, "Synchronize cache failed\n");
}
cam_periph_unlock(periph);
return (error);
}