Date: Mon, 30 Oct 2006 04:56:51 GMT From: Scott Long <scottl@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 108710 for review Message-ID: <200610300456.k9U4upYp021069@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=108710 Change 108710 by scottl@scottl-x64 on 2006/10/30 04:56:49 Conver the MPT driver to use CAM locking. Some attempt was made to keep the driver portable, mainly with the differences in callout vs timeout, but other things were left until later. The locking structure of the driver was not significantly changed, other than assertions were changed on entry to mpt_action since the lock is already held there, and the mpt<->cam lock switch macros were nulled out since they aren't needed. Affected files ... .. //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt.c#12 edit .. //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt.h#13 edit .. //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_cam.c#13 edit .. //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_pci.c#15 edit .. //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_raid.c#8 edit Differences ... ==== //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt.c#12 (text+ko) ==== @@ -1167,7 +1167,7 @@ } KASSERT(req->state != REQ_STATE_FREE, ("freeing free request")); KASSERT(!(req->state & REQ_STATE_LOCKED), ("freeing locked request")); - KASSERT(MPT_OWNED(mpt), ("mpt_free_request: mpt not locked\n")); + mtx_assert(&mpt->mpt_lock, MA_OWNED); KASSERT(mpt_req_on_free_list(mpt, req) == 0, ("mpt_free_request: req %p:%u func %x already on freelist", req, req->serno, ((MSG_REQUEST_HEADER *)req->req_vbuf)->Function)); @@ -1216,7 +1216,7 @@ request_t *req; retry: - KASSERT(MPT_OWNED(mpt), ("mpt_get_request: mpt not locked\n")); + mtx_assert(&mpt->mpt_lock, MA_OWNED); req = TAILQ_FIRST(&mpt->request_free_list); if (req != NULL) { KASSERT(req == &mpt->request_pool[req->index], ==== //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt.h#13 (text+ko) ==== @@ -234,7 +234,7 @@ bus_dma_tag_create(parent_tag, alignment, boundary, \ lowaddr, highaddr, filter, filterarg, \ maxsize, nsegments, maxsegsz, flags, \ - busdma_lock_mutex, &Giant, \ + busdma_lock_mutex, &(mpt)->mpt_lock, \ dma_tagp) #else #define mpt_dma_tag_create(mpt, parent_tag, alignment, boundary, \ @@ -766,8 +766,7 @@ } #else -#ifdef LOCKING_WORKED_AS_IT_SHOULD -#error "Shouldn't Be Here!" +#if 1 #define MPT_IFLAGS INTR_TYPE_CAM | INTR_ENTROPY | INTR_MPSAFE #define MPT_LOCK_SETUP(mpt) \ mtx_init(&mpt->mpt_lock, "mpt", NULL, MTX_DEF); \ @@ -781,12 +780,16 @@ #define MPT_LOCK(mpt) mtx_lock(&(mpt)->mpt_lock) #define MPT_UNLOCK(mpt) mtx_unlock(&(mpt)->mpt_lock) #define MPT_OWNED(mpt) mtx_owned(&(mpt)->mpt_lock) -#define MPTLOCK_2_CAMLOCK(mpt) \ - mtx_unlock(&(mpt)->mpt_lock); mtx_lock(&Giant) -#define CAMLOCK_2_MPTLOCK(mpt) \ - mtx_unlock(&Giant); mtx_lock(&(mpt)->mpt_lock) +#define MPTLOCK_2_CAMLOCK(mpt) +#define CAMLOCK_2_MPTLOCK(mpt) #define mpt_sleep(mpt, ident, priority, wmesg, timo) \ msleep(ident, &(mpt)->mpt_lock, priority, wmesg, timo) +#define mpt_ccb_timeout(ccb, ticks, func, arg) \ + callout_reset(&(ccb)->ccb_h.callout, (ticks), (func), (arg)); +#define mpt_ccb_untimeout(ccb, func, arg) \ + callout_stop(&(ccb)->ccb_h.callout) +#define mpt_ccb_timeout_init(ccb) \ + callout_init(&(ccb)->ccb_h.callout, 1) #else @@ -821,6 +824,15 @@ static __inline int mpt_sleep(struct mpt_softc *, void *, int, const char *, int); +#define mpt_ccb_timeout(ccb, ticks, func, arg) \ + do { \ + (ccb)->ccb_h.timeout_ch = timeout((func), (arg), (ticks)); \ + } while (0) +#define mpt_ccb_untimeout(ccb, func, arg) \ + untimeout((func), (arg), (ccb)->ccb_h.timeout_ch) +#define mpt_ccb_timeout_init(ccb) \ + callout_handle_init(&(ccb)->ccb_h.timeout_ch) + static __inline int mpt_sleep(struct mpt_softc *mpt, void *i, int p, const char *w, int t) { ==== //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_cam.c#13 (text+ko) ==== @@ -304,7 +304,7 @@ * Construct our SIM entry. */ mpt->sim = cam_sim_alloc(mpt_action, mpt_poll, "mpt", mpt, - mpt->unit, &Giant, M_NOWAIT, 1, maxq, devq); + mpt->unit, &mpt->mpt_lock, M_NOWAIT, 1, maxq, devq); if (mpt->sim == NULL) { mpt_prt(mpt, "Unable to allocate CAM SIM!\n"); cam_simq_free(devq); @@ -341,7 +341,7 @@ * Create a "bus" to export all hidden disks to CAM. */ mpt->phydisk_sim = cam_sim_alloc(mpt_action, mpt_poll, "mpt", mpt, - mpt->unit, &Giant, M_NOWAIT, 1, maxq, devq); + mpt->unit, &mpt->mpt_lock, M_NOWAIT, 1, maxq, devq); if (mpt->phydisk_sim == NULL) { mpt_prt(mpt, "Unable to allocate Physical Disk CAM SIM!\n"); error = ENOMEM; @@ -1295,11 +1295,10 @@ ccb->ccb_h.status |= CAM_SIM_QUEUED; if (ccb->ccb_h.timeout != CAM_TIME_INFINITY) { - ccb->ccb_h.timeout_ch = - timeout(mpt_timeout, (caddr_t)ccb, - (ccb->ccb_h.timeout * hz) / 1000); + mpt_ccb_timeout(ccb, (ccb->ccb_h.timeout * hz) / 1000, + mpt_timeout, ccb); } else { - callout_handle_init(&ccb->ccb_h.timeout_ch); + mpt_ccb_timeout_init(ccb); } if (mpt->verbose > MPT_PRT_DEBUG) { int nc = 0; @@ -1694,11 +1693,10 @@ ccb->ccb_h.status |= CAM_SIM_QUEUED; if (ccb->ccb_h.timeout != CAM_TIME_INFINITY) { - ccb->ccb_h.timeout_ch = - timeout(mpt_timeout, (caddr_t)ccb, - (ccb->ccb_h.timeout * hz) / 1000); + mpt_ccb_timeout(ccb, (ccb->ccb_h.timeout * hz) / 1000, + mpt_timeout, ccb); } else { - callout_handle_init(&ccb->ccb_h.timeout_ch); + mpt_ccb_timeout_init(ccb); } if (mpt->verbose > MPT_PRT_DEBUG) { int nc = 0; @@ -2236,7 +2234,7 @@ } tgt = scsi_req->TargetID; - untimeout(mpt_timeout, ccb, ccb->ccb_h.timeout_ch); + mpt_ccb_untimeout(ccb, mpt_timeout, ccb); ccb->ccb_h.status &= ~CAM_SIM_QUEUED; if ((ccb->ccb_h.flags & CAM_DIR_MASK) != CAM_DIR_NONE) { @@ -2857,7 +2855,6 @@ CAM_DEBUG(ccb->ccb_h.path, CAM_DEBUG_TRACE, ("mpt_action\n")); mpt = (struct mpt_softc *)cam_sim_softc(sim); - KASSERT(MPT_OWNED(mpt) == 0, ("mpt owned on entrance to mpt_action")); raid_passthru = (sim == mpt->phydisk_sim); tgt = ccb->ccb_h.target_id; @@ -3534,9 +3531,6 @@ { struct mpt_softc *mpt; -#if __FreeBSD_version >= 500000 - mtx_lock(&Giant); -#endif mpt = (struct mpt_softc *)arg; MPT_LOCK(mpt); for (;;) { @@ -3553,9 +3547,6 @@ mpt->recovery_thread = NULL; wakeup(&mpt->recovery_thread); MPT_UNLOCK(mpt); -#if __FreeBSD_version >= 500000 - mtx_unlock(&Giant); -#endif kthread_exit(0); } @@ -4485,7 +4476,7 @@ req->serno, tgt->resid); if (ccb) { ccb->ccb_h.status = CAM_SIM_QUEUED | CAM_REQ_INPROG; - ccb->ccb_h.timeout_ch = timeout(mpt_timeout, ccb, 60 * hz); + mpt_ccb_timeout(ccb, 60 * hz, mpt_timeout, ccb); } mpt_send_cmd(mpt, req); } @@ -4884,7 +4875,7 @@ } tgt->ccb = NULL; tgt->nxfers++; - untimeout(mpt_timeout, ccb, ccb->ccb_h.timeout_ch); + mpt_ccb_untimeout(ccb, mpt_timeout, ccb); mpt_lprt(mpt, MPT_PRT_DEBUG, "TARGET_ASSIST %p (req %p:%u) done tag 0x%x\n", ccb, tgt->req, tgt->req->serno, ccb->csio.tag_id); @@ -4949,8 +4940,7 @@ TGT_STATE_MOVING_DATA_AND_STATUS) { tgt->nxfers++; } - untimeout(mpt_timeout, ccb, - ccb->ccb_h.timeout_ch); + mpt_ccb_untimeout(ccb, mpt_timeout, ccb); if (ccb->ccb_h.flags & CAM_SEND_SENSE) { ccb->ccb_h.status |= CAM_SENT_SENSE; } ==== //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_pci.c#15 (text+ko) ==== @@ -580,7 +580,6 @@ MPT_UNLOCK(mpt); goto bad; } - KASSERT(MPT_OWNED(mpt) == 0, ("leaving attach with device locked")); return (0); bad: ==== //depot/projects/scottl-camlock/src/sys/dev/mpt/mpt_raid.c#8 (text+ko) ==== @@ -658,9 +658,6 @@ struct mpt_softc *mpt; int firstrun; -#if __FreeBSD_version >= 500000 - mtx_lock(&Giant); -#endif mpt = (struct mpt_softc *)arg; firstrun = 1; MPT_LOCK(mpt); @@ -717,9 +714,6 @@ mpt->raid_thread = NULL; wakeup(&mpt->raid_thread); MPT_UNLOCK(mpt); -#if __FreeBSD_version >= 500000 - mtx_unlock(&Giant); -#endif kthread_exit(0); } @@ -756,8 +750,7 @@ if (rv != 0) return (CAM_REQ_CMP_ERR); - ccb->ccb_h.timeout_ch = - timeout(mpt_raid_quiesce_timeout, (caddr_t)ccb, 5 * hz); + mpt_ccb_timeout(ccb, mpt_raid_quiesce_timeout, ccb, 5 * hz); #if 0 if (rv == ETIMEDOUT) { mpt_disk_prt(mpt, mpt_disk, "mpt_raid_quiesce_disk: "
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200610300456.k9U4upYp021069>