svn commit: r251479 - in head/sys/cam: . scsi
Scott Long
scottl at FreeBSD.org
Fri Jun 7 00:22:40 UTC 2013
Author: scottl
Date: Fri Jun 7 00:22:38 2013
New Revision: 251479
URL: http://svnweb.freebsd.org/changeset/base/251479
Log:
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
Modified:
head/sys/cam/cam_periph.c
head/sys/cam/scsi/scsi_pass.c
head/sys/cam/scsi/scsi_sg.c
head/sys/cam/scsi/scsi_target.c
Modified: head/sys/cam/cam_periph.c
==============================================================================
--- head/sys/cam/cam_periph.c Thu Jun 6 23:21:41 2013 (r251478)
+++ head/sys/cam/cam_periph.c Fri Jun 7 00:22:38 2013 (r251479)
@@ -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
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
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
}
- /* 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, stru
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;
}
Modified: head/sys/cam/scsi/scsi_pass.c
==============================================================================
--- head/sys/cam/scsi/scsi_pass.c Thu Jun 6 23:21:41 2013 (r251478)
+++ head/sys/cam/scsi/scsi_pass.c Fri Jun 7 00:22:38 2013 (r251479)
@@ -668,12 +668,11 @@ passsendccb(struct cam_periph *periph, u
{
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, u
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, u
*/
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, u
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;
Modified: head/sys/cam/scsi/scsi_sg.c
==============================================================================
--- head/sys/cam/scsi/scsi_sg.c Thu Jun 6 23:21:41 2013 (r251478)
+++ head/sys/cam/scsi/scsi_sg.c Fri Jun 7 00:22:38 2013 (r251479)
@@ -946,25 +946,23 @@ sgsendccb(struct cam_periph *periph, uni
{
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));
-
- /*
- * 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;
- }
+ 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.
+ * 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, uni
SF_RETRY_UA,
softc->device_stats);
- if (need_unmap)
- cam_periph_unmapmem(ccb, &mapinfo);
+ cam_periph_unmapmem(ccb, &mapinfo);
return (error);
}
Modified: head/sys/cam/scsi/scsi_target.c
==============================================================================
--- head/sys/cam/scsi/scsi_target.c Thu Jun 6 23:21:41 2013 (r251478)
+++ head/sys/cam/scsi/scsi_target.c Fri Jun 7 00:22:38 2013 (r251479)
@@ -726,21 +726,8 @@ targsendccb(struct targ_softc *softc, un
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);
More information about the svn-src-all
mailing list