This makes the ada(4) driver use UMA for its CCBs.
Details
Details
- Reviewers
imp mav - Group Reviewers
cam - Commits
- rG6f147a0734e0: cam: enable kern.cam.ada.enable_uma_ccbs by default
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 39584 Build 36473: arc lint + arc unit
Event Timeline
Comment Actions
We've had good luck with this one. I'd feel better if we could have a quick coccinelle script to find this... Ignore the "this is bogus" because they might not be...
// Script to find all the uses of ccb's allocated on the stack and change // them to use the new setup function which bzeros before doing a normal // setup of the CCB. @@ union ccb *CCB1; union ccb *CCB2; expression E; @@ - memcpy(CCB1,CCB2,E) + THIS_IS_BOGUS(CCB1,CCB2,E) @@ union ccb *CCB1; union ccb *CCB2; expression E; @@ - bcopy(CCB1,CCB2,E) + THIS_IS_BOGUS(CCB1,CCB2,E) @@ union ccb *CCB1; union ccb *CCB2; @@ - *CCB1=*CCB2 + THIS_IS_BOGUS(CCB1,CCB2)
Had the following hits:
diff -u -p a/cam_xpt.c b/cam_xpt.c --- a/cam_xpt.c +++ b/cam_xpt.c @@ -487,7 +487,7 @@ xptdoioctl(struct cdev *dev, u_long cmd, caddr_t addr, xpt_path_lock(ccb->ccb_h.path); cam_periph_runccb(ccb, NULL, 0, 0, NULL); xpt_path_unlock(ccb->ccb_h.path); - bcopy(ccb, inccb, sizeof(union ccb)); + THIS_IS_BOGUS(ccb, inccb, sizeof(union ccb)); xpt_free_path(ccb->ccb_h.path); xpt_free_ccb(ccb); break; @@ -518,7 +518,7 @@ xptdoioctl(struct cdev *dev, u_long cmd, caddr_t addr, inccb->ccb_h.pinfo.priority); xpt_merge_ccb(&ccb, inccb); xpt_action(&ccb); - bcopy(&ccb, inccb, sizeof(union ccb)); + THIS_IS_BOGUS(&ccb, inccb, sizeof(union ccb)); xpt_free_path(ccb.ccb_h.path); break; } diff -u -p a/scsi/scsi_pass.c b/scsi/scsi_pass.c --- a/scsi/scsi_pass.c +++ b/scsi/scsi_pass.c @@ -2232,7 +2232,7 @@ passsendccb(struct cam_periph *periph, union ccb *ccb, ccb->ccb_h.cbfcnp = NULL; ccb->ccb_h.periph_priv = inccb->ccb_h.periph_priv; - bcopy(ccb, inccb, sizeof(union ccb)); + THIS_IS_BOGUS(ccb, inccb, sizeof(union ccb)); return(0); } diff -u -p a/cam_periph.c b/cam_periph.c --- a/cam_periph.c +++ b/cam_periph.c @@ -1423,7 +1423,7 @@ camperiphdone(struct cam_periph *periph, union ccb *do * and the result will be the final one returned to the CCB owher. */ saved_ccb = (union ccb *)done_ccb->ccb_h.saved_ccb_ptr; - bcopy(saved_ccb, done_ccb, sizeof(*done_ccb)); + THIS_IS_BOGUS(saved_ccb, done_ccb, sizeof(*done_ccb)); xpt_free_ccb(saved_ccb); if (done_ccb->ccb_h.cbfcnp != camperiphdone) periph->flags &= ~CAM_PERIPH_RECOVERY_INPROG; @@ -1713,7 +1713,7 @@ camperiphscsisenseerror(union ccb *ccb, union ccb **or * this freeze will be dropped as part of ERESTART. */ ccb->ccb_h.status &= ~CAM_DEV_QFRZN; - bcopy(ccb, orig_ccb, sizeof(*orig_ccb)); + THIS_IS_BOGUS(ccb, orig_ccb, sizeof(*orig_ccb)); } switch (err_action & SS_MASK) {
There's a review for the cam_periph one. The two ioctls in cam_xpt.c I think are safe because we're copying like to like in terms of the CCB allocation. Ditto with the scsi_pass one.
Based on this, I think we're good, but wanted to "show my work" in case there's some flaw in it.
Comment Actions
Now that I've had time to look at the code in full context, the ioctl functions are definitely a copyout, so they are fine for sure. And the path that did trigger is only for da, not ada. So I'm quite convinced now.