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
This commit is contained in:
Scott Long 2013-06-07 00:22:38 +00:00
parent ca8a27d4b1
commit 95fbded695
4 changed files with 42 additions and 78 deletions

View File

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

View File

@ -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;

View File

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

View File

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