Author: markj Date: Fri Sep 30 21:00:09 2016 New Revision: 306529 URL: https://svnweb.freebsd.org/changeset/base/306529
Log: cam_periph_ccbwait could return while ccb in progress In cam_periph_runccb, cam_periph_ccbwait was using the value of the ccb pinfo.index and status fields to determine whether the ccb was done, but these fields are updated without a contending lock and could glitch into states that would be erroneously interpreted as done. Instead, have cam_periph_ccbwait look for the explicit result of the function cam_periph_done. Submitted by: Ryan Libby <rli...@gmail.com> Reviewed by: mav MFC after: 3 weeks Sponsored by: Dell EMC Isilon Differential Revision: https://reviews.freebsd.org/D8020 Modified: head/sys/cam/cam_periph.c head/sys/cam/cam_periph.h Modified: head/sys/cam/cam_periph.c ============================================================================== --- head/sys/cam/cam_periph.c Fri Sep 30 20:35:12 2016 (r306528) +++ head/sys/cam/cam_periph.c Fri Sep 30 21:00:09 2016 (r306529) @@ -975,16 +975,6 @@ cam_periph_unmapmem(union ccb *ccb, stru PRELE(curproc); } -void -cam_periph_ccbwait(union ccb *ccb) -{ - - if ((ccb->ccb_h.pinfo.index != CAM_UNQUEUED_INDEX) - || ((ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_INPROG)) - xpt_path_sleep(ccb->ccb_h.path, &ccb->ccb_h.cbfcnp, PRIBIO, - "cbwait", 0); -} - int cam_periph_ioctl(struct cam_periph *periph, u_long cmd, caddr_t addr, int (*error_routine)(union ccb *ccb, @@ -1048,13 +1038,38 @@ cam_periph_ioctl(struct cam_periph *peri } static void +cam_periph_done_panic(struct cam_periph *periph, union ccb *done_ccb) +{ + + panic("%s: already done with ccb %p", __func__, done_ccb); +} + +static void cam_periph_done(struct cam_periph *periph, union ccb *done_ccb) { /* Caller will release the CCB */ + xpt_path_assert(done_ccb->ccb_h.path, MA_OWNED); + done_ccb->ccb_h.cbfcnp = cam_periph_done_panic; wakeup(&done_ccb->ccb_h.cbfcnp); } +static void +cam_periph_ccbwait(union ccb *ccb) +{ + + if ((ccb->ccb_h.func_code & XPT_FC_QUEUED) != 0) { + while (ccb->ccb_h.cbfcnp != cam_periph_done_panic) + xpt_path_sleep(ccb->ccb_h.path, &ccb->ccb_h.cbfcnp, + PRIBIO, "cbwait", 0); + } + KASSERT(ccb->ccb_h.pinfo.index == CAM_UNQUEUED_INDEX && + (ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_INPROG, + ("%s: proceeding with incomplete ccb: ccb=%p, func_code=%#x, " + "status=%#x, index=%d", __func__, ccb, ccb->ccb_h.func_code, + ccb->ccb_h.status, ccb->ccb_h.pinfo.index)); +} + int cam_periph_runccb(union ccb *ccb, int (*error_routine)(union ccb *ccb, @@ -1069,6 +1084,9 @@ cam_periph_runccb(union ccb *ccb, starttime = NULL; xpt_path_assert(ccb->ccb_h.path, MA_OWNED); + KASSERT((ccb->ccb_h.flags & CAM_UNLOCKED) == 0, + ("%s: ccb=%p, func_code=%#x, flags=%#x", __func__, ccb, + ccb->ccb_h.func_code, ccb->ccb_h.flags)); /* * If the user has supplied a stats structure, and if we understand @@ -1088,9 +1106,10 @@ cam_periph_runccb(union ccb *ccb, cam_periph_ccbwait(ccb); if ((ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) error = 0; - else if (error_routine != NULL) + else if (error_routine != NULL) { + ccb->ccb_h.cbfcnp = cam_periph_done; error = (*error_routine)(ccb, camflags, sense_flags); - else + } else error = 0; } while (error == ERESTART); Modified: head/sys/cam/cam_periph.h ============================================================================== --- head/sys/cam/cam_periph.h Fri Sep 30 20:35:12 2016 (r306528) +++ head/sys/cam/cam_periph.h Fri Sep 30 21:00:09 2016 (r306529) @@ -166,7 +166,6 @@ void cam_periph_unmapmem(union ccb *ccb struct cam_periph_map_info *mapinfo); union ccb *cam_periph_getccb(struct cam_periph *periph, u_int32_t priority); -void cam_periph_ccbwait(union ccb *ccb); int cam_periph_runccb(union ccb *ccb, int (*error_routine)(union ccb *ccb, cam_flags camflags, _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
