Date: Sun, 15 Apr 2007 03:06:23 GMT From: Scott Long <scottl@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 118135 for review Message-ID: <200704150306.l3F36Nwt059646@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=118135 Change 118135 by scottl@scottl-x64 on 2007/04/15 03:06:13 Another try at the locking protocol. Rename cam_periph_block to cam_periph_hold. Give it the same arguments and semantics as the original cam_periph_lock(), but require that the periph lock already be held. Locking and holding can't be combined because several periph_register functions are called with the lock held already. Add cam_periph_hold/unhold to the periph drivers that need it, basically replacing the old instances of cam_periph_lock(). Some drivers where only using cam_periph_lock to protect flag changing, so there is not need to add a hold as well. Note that the scsi_sa and scsi_target drivers aren't being touched yet. Affected files ... .. //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#19 edit .. //depot/projects/scottl-camlock/src/sys/cam/cam_periph.h#12 edit .. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_cd.c#18 edit .. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_ch.c#12 edit .. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_da.c#30 edit Differences ... ==== //depot/projects/scottl-camlock/src/sys/cam/cam_periph.c#19 (text+ko) ==== @@ -309,27 +309,52 @@ } -void -cam_periph_block(struct cam_periph *periph) +int +cam_periph_hold(struct cam_periph *periph, int priority) { + struct mtx *mtx; + int error; mtx_assert(periph->sim->mtx, MA_OWNED); - cam_periph_acquire(periph); + /* + * Increment the reference count on the peripheral + * while we wait for our lock attempt to succeed + * to ensure the peripheral doesn't disappear out + * from user us while we sleep. + */ + + if (cam_periph_acquire(periph) != CAM_REQ_CMP) + return (ENXIO); + + mtx = periph->sim->mtx; + if (mtx == &Giant) + mtx = NULL; + + while ((periph->flags & CAM_PERIPH_LOCKED) != 0) { + periph->flags |= CAM_PERIPH_LOCK_WANTED; + if ((error = msleep(periph, mtx, priority, "caplck", 0)) != 0) { + cam_periph_release(periph); + return (error); + } + } + periph->flags |= CAM_PERIPH_LOCKED; + return (0); } void -cam_periph_unblock(struct cam_periph *periph) +cam_periph_unhold(struct cam_periph *periph) { mtx_assert(periph->sim->mtx, MA_OWNED); periph->flags &= ~CAM_PERIPH_LOCKED; - if (periph->flags &CAM_PERIPH_LOCK_WANTED) { + if ((periph->flags & CAM_PERIPH_LOCK_WANTED) != 0) { periph->flags &= ~CAM_PERIPH_LOCK_WANTED; wakeup(periph); } + cam_periph_release(periph); } @@ -532,16 +557,8 @@ void cam_periph_lock(struct cam_periph *periph) { - struct mtx *mtx; - mtx = periph->sim->mtx; - mtx_lock(mtx); - if (periph->flags & CAM_PERIPH_LOCKED) { - periph->flags |= CAM_PERIPH_LOCK_WANTED; - if (mtx == &Giant) - mtx = NULL; - msleep(periph, mtx, PCATCH, "periph", 0); - }; + mtx_lock(periph->sim->mtx); } /* ==== //depot/projects/scottl-camlock/src/sys/cam/cam_periph.h#12 (text+ko) ==== @@ -142,8 +142,8 @@ void cam_periph_unlock(struct cam_periph *periph); cam_status cam_periph_acquire(struct cam_periph *periph); void cam_periph_release(struct cam_periph *periph); -void cam_periph_block(struct cam_periph *periph); -void cam_periph_unblock(struct cam_periph *periph); +int cam_periph_hold(struct cam_periph *periph, int priority); +void cam_periph_unhold(struct cam_periph *periph); void cam_periph_invalidate(struct cam_periph *periph); int cam_periph_mapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo); ==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_cd.c#18 (text+ko) ==== @@ -976,8 +976,11 @@ cdregisterexit: - /* Refcount this periph now that it's been created. */ - cam_periph_block(periph); + /* + * Refcount and block open attempts until we are setup + * Can't block + */ + (void)cam_periph_hold(periph, PRIBIO); if ((softc->flags & CD_FLAG_CHANGER) == 0) xpt_schedule(periph, /*priority*/5); @@ -992,6 +995,7 @@ { struct cam_periph *periph; struct cd_softc *softc; + int error; periph = (struct cam_periph *)dp->d_drv1; if (periph == NULL) @@ -1010,6 +1014,12 @@ return(ENXIO); } + if ((error = cam_periph_hold(periph, PRIBIO | PCATCH)) != 0) { + cam_periph_unlock(periph); + cam_periph_release(periph); + return (error); + } + /* Closes aren't symmetrical with opens, so fix up the refcounting. */ if (softc->flags & CD_FLAG_OPEN) cam_periph_release(periph); @@ -1024,6 +1034,7 @@ cdcheckmedia(periph); CAM_DEBUG(periph->path, CAM_DEBUG_TRACE, ("leaving cdopen\n")); + cam_periph_unhold(periph); cam_periph_unlock(periph); return (0); @@ -1042,6 +1053,7 @@ softc = (struct cd_softc *)periph->softc; cam_periph_lock(periph); + cam_periph_hold(periph, PRIBIO); if ((softc->flags & CD_FLAG_DISC_REMOVABLE) != 0) cdprevent(periph, PR_ALLOW); @@ -1057,6 +1069,7 @@ */ softc->flags &= ~(CD_FLAG_VALID_MEDIA|CD_FLAG_VALID_TOC|CD_FLAG_OPEN); + cam_periph_unhold(periph); cam_periph_unlock(periph); cam_periph_release(periph); @@ -1784,7 +1797,7 @@ * operation. */ xpt_release_ccb(done_ccb); - cam_periph_unblock(periph); + cam_periph_unhold(periph); return; } case CD_CCB_WAITING: @@ -1843,14 +1856,20 @@ if (periph == NULL) return(ENXIO); + cam_periph_lock(periph); CAM_DEBUG(periph->path, CAM_DEBUG_TRACE, ("entering cdioctl\n")); softc = (struct cd_softc *)periph->softc; - cam_periph_lock(periph); CAM_DEBUG(periph->path, CAM_DEBUG_TRACE, ("trying to do ioctl %#lx\n", cmd)); + if ((error = cam_periph_hold(periph, PRIBIO | PCATCH)) != 0) { + cam_periph_unlock(periph); + cam_periph_release(periph); + return (error); + } + /* * If we don't have media loaded, check for it. If still don't * have media loaded, we can only do a load or eject. @@ -1866,6 +1885,10 @@ && (IOCGROUP(cmd) == 'c')) { error = cdcheckmedia(periph); } + /* + * Drop the lock here so later mallocs can use WAITOK. The periph + * is essentially locked still with the cam_periph_hold call above. + */ cam_periph_unlock(periph); if (error != 0) return (error); @@ -2687,9 +2710,14 @@ break; } + cam_periph_lock(periph); + cam_periph_unhold(periph); + + CAM_DEBUG(periph->path, CAM_DEBUG_TRACE, ("leaving cdioctl\n")); if (error && bootverbose) { printf("scsi_cd.c::ioctl cmd=%08lx error=%d\n", cmd, error); } + cam_periph_unlock(periph); return (error); } ==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_ch.c#12 (text+ko) ==== @@ -392,8 +392,11 @@ csa.callback_arg = periph; xpt_action((union ccb *)&csa); - /* Refcount this periph now that it's been created. */ - cam_periph_block(periph); + /* + * Lock this periph until we are setup. + * This first call can't block + */ + (void)cam_periph_hold(periph, PRIBIO); xpt_schedule(periph, /*priority*/5); return(CAM_REQ_CMP); @@ -425,6 +428,12 @@ else cam_periph_release(periph); + if ((error = cam_periph_hold(periph, PRIBIO | PCATCH)) != 0) { + cam_periph_unlock(periph); + cam_periph_release(periph); + return (error); + } + /* * Load information about this changer device into the softc. */ @@ -435,6 +444,7 @@ return(error); } + cam_periph_unhold(periph); cam_periph_unlock(periph); return(error); @@ -655,7 +665,7 @@ * operation. */ xpt_release_ccb(done_ccb); - cam_periph_unblock(periph); + cam_periph_unhold(periph); return; } case CH_CCB_WAITING: ==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_da.c#30 (text+ko) ==== @@ -573,6 +573,11 @@ } cam_periph_lock(periph); + if ((error = cam_periph_hold(periph, PRIBIO|PCATCH)) != 0) { + cam_periph_unlock(periph); + cam_periph_release(periph); + return (error); + } unit = periph->unit_number; softc = (struct da_softc *)periph->softc; @@ -608,6 +613,7 @@ softc->flags &= ~DA_FLAG_OPEN; cam_periph_release(periph); } + cam_periph_unhold(periph); cam_periph_unlock(periph); return (error); } @@ -617,12 +623,18 @@ { struct cam_periph *periph; struct da_softc *softc; + int error; periph = (struct cam_periph *)dp->d_drv1; if (periph == NULL) return (ENXIO); cam_periph_lock(periph); + if ((error = cam_periph_hold(periph, PRIBIO)) != 0) { + cam_periph_unlock(periph); + cam_periph_release(periph); + return (error); + } softc = (struct da_softc *)periph->softc; @@ -687,6 +699,7 @@ } softc->flags &= ~DA_FLAG_OPEN; + cam_periph_unhold(periph); cam_periph_unlock(periph); cam_periph_release(periph); return (0); @@ -1205,7 +1218,7 @@ * to finish the probe. The reference will be dropped in dadone at * the end of probe. */ - cam_periph_block(periph); + (void)cam_periph_hold(periph, PRIBIO); xpt_schedule(periph, /*priority*/5); /* @@ -1687,7 +1700,7 @@ * operation. */ xpt_release_ccb(done_ccb); - cam_periph_unblock(periph); + cam_periph_unhold(periph); return; } case DA_CCB_WAITING:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200704150306.l3F36Nwt059646>