diff options
author | scottl <scottl@FreeBSD.org> | 2013-06-07 00:22:38 +0000 |
---|---|---|
committer | scottl <scottl@FreeBSD.org> | 2013-06-07 00:22:38 +0000 |
commit | 3300ef5cf82d8e71039c6c0584f38403c5d3dd25 (patch) | |
tree | f508fae12ebb2df91603ae6933790bfa4ddfebec /sys/cam | |
parent | b20363fe596a7e3a55d5b62f4d3fdb482c65c47a (diff) | |
download | FreeBSD-src-3300ef5cf82d8e71039c6c0584f38403c5d3dd25.zip FreeBSD-src-3300ef5cf82d8e71039c6c0584f38403c5d3dd25.tar.gz |
Simplify the checking of flags for cam_periph_mapmem(). This gets rid of
a lot of code redundancy and grossness at very minor expense.
Reviewed by: smh
Obtained from: Netflix
MFC after: 3 days
Diffstat (limited to 'sys/cam')
-rw-r--r-- | sys/cam/cam_periph.c | 25 | ||||
-rw-r--r-- | sys/cam/scsi/scsi_pass.c | 45 | ||||
-rw-r--r-- | sys/cam/scsi/scsi_sg.c | 33 | ||||
-rw-r--r-- | sys/cam/scsi/scsi_target.c | 17 |
4 files changed, 42 insertions, 78 deletions
diff --git a/sys/cam/cam_periph.c b/sys/cam/cam_periph.c index c3e3bd4..f3300db 100644 --- a/sys/cam/cam_periph.c +++ b/sys/cam/cam_periph.c @@ -681,9 +681,9 @@ camperiphfree(struct cam_periph *periph) /* * Map user virtual pointers into kernel virtual address space, so we can - * access the memory. This won't work on physical pointers, for now it's - * up to the caller to check for that. (XXX KDM -- should we do that here - * instead?) This also only works for up to MAXPHYS memory. Since we use + * access the memory. This is now a generic function that centralizes most + * of the sanity checks on the data flags, if any. + * This also only works for up to MAXPHYS memory. Since we use * buffers to map stuff in and out, we're limited to the buffer size. */ int @@ -728,9 +728,8 @@ cam_periph_mapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo) case XPT_CONT_TARGET_IO: if ((ccb->ccb_h.flags & CAM_DIR_MASK) == CAM_DIR_NONE) return(0); - KASSERT((ccb->ccb_h.flags & CAM_DATA_MASK) == CAM_DATA_VADDR, - ("not VADDR for SCSI_IO %p %x\n", ccb, ccb->ccb_h.flags)); - + if ((ccb->ccb_h.flags & CAM_DATA_MASK) != CAM_DATA_VADDR) + return (EINVAL); data_ptrs[0] = &ccb->csio.data_ptr; lengths[0] = ccb->csio.dxfer_len; dirs[0] = ccb->ccb_h.flags & CAM_DIR_MASK; @@ -739,9 +738,8 @@ cam_periph_mapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo) case XPT_ATA_IO: if ((ccb->ccb_h.flags & CAM_DIR_MASK) == CAM_DIR_NONE) return(0); - KASSERT((ccb->ccb_h.flags & CAM_DATA_MASK) == CAM_DATA_VADDR, - ("not VADDR for ATA_IO %p %x\n", ccb, ccb->ccb_h.flags)); - + if ((ccb->ccb_h.flags & CAM_DATA_MASK) != CAM_DATA_VADDR) + return (EINVAL); data_ptrs[0] = &ccb->ataio.data_ptr; lengths[0] = ccb->ataio.dxfer_len; dirs[0] = ccb->ccb_h.flags & CAM_DIR_MASK; @@ -812,8 +810,12 @@ cam_periph_mapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo) } - /* this keeps the current process from getting swapped */ /* + * This keeps the the kernel stack of current thread from getting + * swapped. In low-memory situations where the kernel stack might + * otherwise get swapped out, this holds it and allows the thread + * to make progress and release the kernel mapped pages sooner. + * * XXX KDM should I use P_NOSWAP instead? */ PHOLD(curproc); @@ -885,8 +887,7 @@ cam_periph_unmapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo) u_int8_t **data_ptrs[CAM_PERIPH_MAXMAPS]; if (mapinfo->num_bufs_used <= 0) { - /* allow ourselves to be swapped once again */ - PRELE(curproc); + /* nothing to free and the process wasn't held. */ return; } diff --git a/sys/cam/scsi/scsi_pass.c b/sys/cam/scsi/scsi_pass.c index eb5a57d..2b8b176 100644 --- a/sys/cam/scsi/scsi_pass.c +++ b/sys/cam/scsi/scsi_pass.c @@ -668,12 +668,11 @@ passsendccb(struct cam_periph *periph, union ccb *ccb, union ccb *inccb) { struct pass_softc *softc; struct cam_periph_map_info mapinfo; - int error, need_unmap; + xpt_opcode fc; + int error; softc = (struct pass_softc *)periph->softc; - need_unmap = 0; - /* * There are some fields in the CCB header that need to be * preserved, the rest we get from the user. @@ -687,28 +686,13 @@ passsendccb(struct cam_periph *periph, union ccb *ccb, union ccb *inccb) ccb->ccb_h.cbfcnp = passdone; /* - * We only attempt to map the user memory into kernel space - * if they haven't passed in a physical memory pointer, - * and if there is actually an I/O operation to perform. - * cam_periph_mapmem() supports SCSI, ATA, SMP, ADVINFO and device - * match CCBs. For the SCSI, ATA and ADVINFO CCBs, we only pass the - * CCB in if there's actually data to map. cam_periph_mapmem() will - * do the right thing, even if there isn't data to map, but since CCBs - * without data are a reasonably common occurrence (e.g. test unit - * ready), it will save a few cycles if we check for it here. - * - * XXX What happens if a sg list is supplied? We don't filter that - * out. + * Let cam_periph_mapmem do a sanity check on the data pointer format. + * Even if no data transfer is needed, it's a cheap check and it + * simplifies the code. */ - if (((ccb->ccb_h.flags & CAM_DATA_MASK) == CAM_DATA_VADDR) - && (((ccb->ccb_h.func_code == XPT_SCSI_IO || - ccb->ccb_h.func_code == XPT_ATA_IO) - && ((ccb->ccb_h.flags & CAM_DIR_MASK) != CAM_DIR_NONE)) - || (ccb->ccb_h.func_code == XPT_DEV_MATCH) - || (ccb->ccb_h.func_code == XPT_SMP_IO) - || ((ccb->ccb_h.func_code == XPT_DEV_ADVINFO) - && (ccb->cdai.bufsiz > 0)))) { - + fc = ccb->ccb_h.func_code; + if ((fc == XPT_SCSI_IO) || (fc == XPT_ATA_IO) || (fc == XPT_SMP_IO) + || (fc == XPT_DEV_MATCH) || (fc == XPT_DEV_ADVINFO)) { bzero(&mapinfo, sizeof(mapinfo)); /* @@ -726,13 +710,9 @@ passsendccb(struct cam_periph *periph, union ccb *ccb, union ccb *inccb) */ if (error) return(error); - - /* - * We successfully mapped the memory in, so we need to - * unmap it when the transaction is done. - */ - need_unmap = 1; - } + } else + /* Ensure that the unmap call later on is a no-op. */ + mapinfo.num_bufs_used = 0; /* * If the user wants us to perform any error recovery, then honor @@ -744,8 +724,7 @@ passsendccb(struct cam_periph *periph, union ccb *ccb, union ccb *inccb) SF_RETRY_UA : SF_NO_RECOVERY) | SF_NO_PRINT, softc->device_stats); - if (need_unmap != 0) - cam_periph_unmapmem(ccb, &mapinfo); + cam_periph_unmapmem(ccb, &mapinfo); ccb->ccb_h.cbfcnp = NULL; ccb->ccb_h.periph_priv = inccb->ccb_h.periph_priv; diff --git a/sys/cam/scsi/scsi_sg.c b/sys/cam/scsi/scsi_sg.c index 9836e29..1ad78ad 100644 --- a/sys/cam/scsi/scsi_sg.c +++ b/sys/cam/scsi/scsi_sg.c @@ -946,25 +946,23 @@ sgsendccb(struct cam_periph *periph, union ccb *ccb) { struct sg_softc *softc; struct cam_periph_map_info mapinfo; - int error, need_unmap = 0; + int error; softc = periph->softc; - if (((ccb->ccb_h.flags & CAM_DIR_MASK) != CAM_DIR_NONE) - && (ccb->csio.data_ptr != NULL)) { - bzero(&mapinfo, sizeof(mapinfo)); + bzero(&mapinfo, sizeof(mapinfo)); - /* - * cam_periph_mapmem calls into proc and vm functions that can - * sleep as well as trigger I/O, so we can't hold the lock. - * Dropping it here is reasonably safe. - */ - cam_periph_unlock(periph); - error = cam_periph_mapmem(ccb, &mapinfo); - cam_periph_lock(periph); - if (error) - return (error); - need_unmap = 1; - } + /* + * cam_periph_mapmem calls into proc and vm functions that can + * sleep as well as trigger I/O, so we can't hold the lock. + * Dropping it here is reasonably safe. + * The only CCB opcode that is possible here is XPT_SCSI_IO, no + * need for additional checks. + */ + cam_periph_unlock(periph); + error = cam_periph_mapmem(ccb, &mapinfo); + cam_periph_lock(periph); + if (error) + return (error); error = cam_periph_runccb(ccb, sgerror, @@ -972,8 +970,7 @@ sgsendccb(struct cam_periph *periph, union ccb *ccb) SF_RETRY_UA, softc->device_stats); - if (need_unmap) - cam_periph_unmapmem(ccb, &mapinfo); + cam_periph_unmapmem(ccb, &mapinfo); return (error); } diff --git a/sys/cam/scsi/scsi_target.c b/sys/cam/scsi/scsi_target.c index 56ff8a9..78e96fb 100644 --- a/sys/cam/scsi/scsi_target.c +++ b/sys/cam/scsi/scsi_target.c @@ -726,21 +726,8 @@ targsendccb(struct targ_softc *softc, union ccb *ccb, ccb_h->cbfcnp = targdone; ccb_h->targ_descr = descr; - /* - * We only attempt to map the user memory into kernel space - * if they haven't passed in a physical memory pointer, - * and if there is actually an I/O operation to perform. - * Right now cam_periph_mapmem() only supports SCSI and device - * match CCBs. For the SCSI CCBs, we only pass the CCB in if - * there's actually data to map. cam_periph_mapmem() will do the - * right thing, even if there isn't data to map, but since CCBs - * without data are a reasonably common occurrence (e.g. test unit - * ready), it will save a few cycles if we check for it here. - */ - if (((ccb_h->flags & CAM_DATA_MASK) == CAM_DATA_VADDR) - && (((ccb_h->func_code == XPT_CONT_TARGET_IO) - && ((ccb_h->flags & CAM_DIR_MASK) != CAM_DIR_NONE)) - || (ccb_h->func_code == XPT_DEV_MATCH))) { + if ((ccb_h->func_code == XPT_CONT_TARGET_IO) || + (ccb_h->func_code == XPT_DEV_MATCH)) { error = cam_periph_mapmem(ccb, mapinfo); |