Add locking to scd(4) and mark MPSAFE.

- Actually use existing per-softc mutex.
- Use mutex in cdev routines and remove D_NEEDGIANT.
- Use callout(9) instead of timeout(9).
- Don't check for impossible conditions (e.g. SCDINIT being clear).
- Use bus_*() instead of bus_space_*().

Tested by:	no one
This commit is contained in:
John Baldwin 2014-11-18 22:02:37 +00:00
parent 369934d602
commit 42e8c47b78
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=274679
3 changed files with 86 additions and 74 deletions

View File

@ -120,7 +120,7 @@ static int get_result(struct scd_softc *, int result_len, u_char *result);
static void print_error(struct scd_softc *, int errcode);
static void scd_start(struct scd_softc *);
static timeout_t scd_timeout;
static void scd_timeout(void *);
static void scd_doread(struct scd_softc *, int state, struct scd_mbx *mbxin);
static int scd_eject(struct scd_softc *);
@ -153,7 +153,7 @@ static struct cdevsw scd_cdevsw = {
.d_ioctl = scdioctl,
.d_strategy = scdstrategy,
.d_name = "scd",
.d_flags = D_DISK | D_NEEDGIANT,
.d_flags = D_DISK,
};
int
@ -163,11 +163,13 @@ scd_attach(struct scd_softc *sc)
unit = device_get_unit(sc->dev);
SCD_LOCK(sc);
init_drive(sc);
sc->data.flags = SCDINIT;
sc->data.audio_status = CD_AS_AUDIO_INVALID;
bioq_init(&sc->data.head);
SCD_UNLOCK(sc);
sc->scd_dev_t = make_dev(&scd_cdevsw, 8 * unit,
UID_ROOT, GID_OPERATOR, 0640, "scd%d", unit);
@ -184,18 +186,18 @@ scdopen(struct cdev *dev, int flags, int fmt, struct thread *td)
sc = (struct scd_softc *)dev->si_drv1;
/* not initialized*/
if (!(sc->data.flags & SCDINIT))
return (ENXIO);
/* invalidated in the meantime? mark all open part's invalid */
if (sc->data.openflag)
/* mark all open part's invalid */
SCD_LOCK(sc);
if (sc->data.openflag) {
SCD_UNLOCK(sc);
return (ENXIO);
}
XDEBUG(sc, 1, "DEBUG: status = 0x%x\n", SCD_READ(sc, IREG_STATUS));
if ((rc = spin_up(sc)) != 0) {
print_error(sc, rc);
SCD_UNLOCK(sc);
return (EIO);
}
if (!(sc->data.flags & SCDTOC)) {
@ -205,18 +207,21 @@ scdopen(struct cdev *dev, int flags, int fmt, struct thread *td)
if (rc == ERR_NOT_SPINNING) {
rc = spin_up(sc);
if (rc) {
print_error(sc, rc);\
print_error(sc, rc);
SCD_UNLOCK(sc);
return (EIO);
}
continue;
}
device_printf(sc->dev, "TOC read error 0x%x\n", rc);
SCD_UNLOCK(sc);
return (EIO);
}
}
sc->data.openflag = 1;
sc->data.flags |= SCDVALID;
SCD_UNLOCK(sc);
return (0);
}
@ -228,17 +233,17 @@ scdclose(struct cdev *dev, int flags, int fmt, struct thread *td)
sc = (struct scd_softc *)dev->si_drv1;
if (!(sc->data.flags & SCDINIT) || !sc->data.openflag)
return (ENXIO);
SCD_LOCK(sc);
KASSERT(sc->data.openflag, ("device not open"));
if (sc->data.audio_status != CD_AS_PLAY_IN_PROGRESS) {
(void)send_cmd(sc, CMD_SPIN_DOWN, 0);
sc->data.flags &= ~SCDSPINNING;
}
/* close channel */
sc->data.openflag = 0;
SCD_UNLOCK(sc);
return (0);
}
@ -246,12 +251,12 @@ scdclose(struct cdev *dev, int flags, int fmt, struct thread *td)
static void
scdstrategy(struct bio *bp)
{
int s;
struct scd_softc *sc;
sc = (struct scd_softc *)bp->bio_dev->si_drv1;
/* if device invalidated (e.g. media change, door open), error */
SCD_LOCK(sc);
if (!(sc->data.flags & SCDVALID)) {
device_printf(sc->dev, "media changed\n");
bp->bio_error = EIO;
@ -276,17 +281,17 @@ scdstrategy(struct bio *bp)
bp->bio_resid = 0;
/* queue it */
s = splbio();
bioq_disksort(&sc->data.head, bp);
splx(s);
/* now check whether we can perform processing */
scd_start(sc);
SCD_UNLOCK(sc);
return;
bad:
bp->bio_flags |= BIO_ERROR;
done:
SCD_UNLOCK(sc);
bp->bio_resid = bp->bio_bcount;
biodone(bp);
return;
@ -296,27 +301,22 @@ static void
scd_start(struct scd_softc *sc)
{
struct bio *bp;
int s = splbio();
if (sc->data.flags & SCDMBXBSY) {
splx(s);
SCD_ASSERT_LOCKED(sc);
if (sc->data.flags & SCDMBXBSY)
return;
}
bp = bioq_takefirst(&sc->data.head);
if (bp != 0) {
/* block found to process, dequeue */
sc->data.flags |= SCDMBXBSY;
splx(s);
} else {
/* nothing to do */
splx(s);
return;
}
sc->data.mbx.retry = 3;
sc->data.mbx.bp = bp;
splx(s);
scd_doread(sc, SCD_S_BEGIN, &(sc->data.mbx));
return;
@ -326,39 +326,47 @@ static int
scdioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td)
{
struct scd_softc *sc;
int error;
sc = (struct scd_softc *)dev->si_drv1;
XDEBUG(sc, 1, "ioctl: cmd=0x%lx\n", cmd);
if (!(sc->data.flags & SCDVALID))
SCD_LOCK(sc);
if (!(sc->data.flags & SCDVALID)) {
SCD_UNLOCK(sc);
return (EIO);
}
error = 0;
switch (cmd) {
case DIOCGMEDIASIZE:
*(off_t *)addr = (off_t)sc->data.disksize * sc->data.blksize;
return (0);
break;
case DIOCGSECTORSIZE:
*(u_int *)addr = sc->data.blksize;
return (0);
break;
case CDIOCPLAYTRACKS:
return scd_playtracks(sc, (struct ioc_play_track *) addr);
error = scd_playtracks(sc, (struct ioc_play_track *) addr);
break;
case CDIOCPLAYBLOCKS:
return (EINVAL);
error = EINVAL;
break;
case CDIOCPLAYMSF:
return scd_playmsf(sc, (struct ioc_play_msf *) addr);
error = scd_playmsf(sc, (struct ioc_play_msf *) addr);
break;
case CDIOCREADSUBCHANNEL_SYSSPACE:
return scd_subchan(sc, (struct ioc_read_subchannel *) addr, 1);
case CDIOCREADSUBCHANNEL:
return scd_subchan(sc, (struct ioc_read_subchannel *) addr, 0);
case CDIOREADTOCHEADER:
return scd_toc_header (sc, (struct ioc_toc_header *) addr);
error = scd_toc_header (sc, (struct ioc_toc_header *) addr);
break;
case CDIOREADTOCENTRYS:
return scd_toc_entrys (sc, (struct ioc_read_toc_entry*) addr);
case CDIOREADTOCENTRY:
return scd_toc_entry (sc, (struct ioc_read_toc_single_entry*) addr);
error = scd_toc_entry (sc, (struct ioc_read_toc_single_entry*) addr);
break;
case CDIOCSETPATCH:
case CDIOCGETVOL:
case CDIOCSETVOL:
@ -367,34 +375,43 @@ scdioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *t
case CDIOCSETMUTE:
case CDIOCSETLEFT:
case CDIOCSETRIGHT:
return (EINVAL);
error = EINVAL;
break;
case CDIOCRESUME:
return scd_resume(sc);
error = scd_resume(sc);
break;
case CDIOCPAUSE:
return scd_pause(sc);
error = scd_pause(sc);
break;
case CDIOCSTART:
return (EINVAL);
error = EINVAL;
break;
case CDIOCSTOP:
return scd_stop(sc);
error = scd_stop(sc);
break;
case CDIOCEJECT:
return scd_eject(sc);
error = scd_eject(sc);
break;
case CDIOCALLOW:
return (0);
break;
case CDIOCSETDEBUG:
#ifdef SCD_DEBUG
scd_debuglevel++;
#endif
return (0);
break;
case CDIOCCLRDEBUG:
#ifdef SCD_DEBUG
scd_debuglevel = 0;
#endif
return (0);
break;
default:
device_printf(sc->dev, "unsupported ioctl (cmd=0x%lx)\n", cmd);
return (ENOTTY);
error = ENOTTY;
break;
}
SCD_UNLOCK(sc);
return (error);
}
/***************************************************************
@ -573,6 +590,7 @@ scd_subchan(struct scd_softc *sc, struct ioc_read_subchannel *sch, int nocopyout
data.what.position.absaddr.msf.minute = bcd2bin(q.abs_msf[0]);
data.what.position.absaddr.msf.second = bcd2bin(q.abs_msf[1]);
data.what.position.absaddr.msf.frame = bcd2bin(q.abs_msf[2]);
SCD_UNLOCK(sc);
if (nocopyout == 0) {
if (copyout(&data, sch->data, min(sizeof(struct cd_sub_channel_info), sch->data_len))!=0)
@ -680,6 +698,7 @@ scd_timeout(void *arg)
struct scd_softc *sc;
sc = (struct scd_softc *)arg;
SCD_ASSERT_LOCKED(sc);
scd_doread(sc, sc->ch_state, sc->ch_mbxsave);
}
@ -693,6 +712,7 @@ scd_doread(struct scd_softc *sc, int state, struct scd_mbx *mbxin)
caddr_t addr;
static char sdata[3]; /* Must be preserved between calls to this function */
SCD_ASSERT_LOCKED(sc);
loop:
switch (state) {
case SCD_S_BEGIN:
@ -707,7 +727,7 @@ scd_doread(struct scd_softc *sc, int state, struct scd_mbx *mbxin)
case SCD_S_WAITSTAT:
sc->ch_state = SCD_S_WAITSTAT;
untimeout(scd_timeout, (caddr_t)sc, sc->ch);
callout_stop(&sc->timer);
if (mbx->count-- <= 0) {
device_printf(sc->dev, "timeout. drive busy.\n");
goto harderr;
@ -716,7 +736,7 @@ scd_doread(struct scd_softc *sc, int state, struct scd_mbx *mbxin)
trystat:
if (IS_BUSY(sc)) {
sc->ch_state = SCD_S_WAITSTAT;
sc->ch = timeout(scd_timeout, (caddr_t)sc, hz/100); /* XXX */
callout_reset(&sc->timer, hz / 100, scd_timeout, sc); /* XXX */
return;
}
@ -754,19 +774,19 @@ scd_doread(struct scd_softc *sc, int state, struct scd_mbx *mbxin)
mbx->count = 100;
sc->ch_state = SCD_S_WAITFIFO;
sc->ch = timeout(scd_timeout, (caddr_t)sc, hz/100); /* XXX */
callout_reset(&sc->timer, hz / 100, scd_timeout, sc); /* XXX */
return;
case SCD_S_WAITSPIN:
sc->ch_state = SCD_S_WAITSPIN;
untimeout(scd_timeout,(caddr_t)sc, sc->ch);
callout_stop(&sc->timer);
if (mbx->count-- <= 0) {
device_printf(sc->dev, "timeout waiting for drive to spin up.\n");
goto harderr;
}
if (!STATUS_BIT(sc, SBIT_RESULT_READY)) {
sc->ch_state = SCD_S_WAITSPIN;
sc->ch = timeout(scd_timeout, (caddr_t)sc, hz/100); /* XXX */
callout_reset(&sc->timer, hz / 100, scd_timeout, sc); /* XXX */
return;
}
SCD_WRITE(sc, OREG_CONTROL, CBIT_RESULT_READY_CLEAR);
@ -787,14 +807,14 @@ scd_doread(struct scd_softc *sc, int state, struct scd_mbx *mbxin)
case SCD_S_WAITFIFO:
sc->ch_state = SCD_S_WAITFIFO;
untimeout(scd_timeout,(caddr_t)sc, sc->ch);
callout_stop(&sc->timer);
if (mbx->count-- <= 0) {
device_printf(sc->dev, "timeout. write param not ready.\n");
goto harderr;
}
if (!FSTATUS_BIT(sc, FBIT_WPARAM_READY)) {
sc->ch_state = SCD_S_WAITFIFO;
sc->ch = timeout(scd_timeout, (caddr_t)sc,hz/100); /* XXX */
callout_reset(&sc->timer, hz / 100, scd_timeout, sc); /* XXX */
return;
}
XDEBUG(sc, 1, "mbx->count (writeparamwait) = %d(%d)\n", mbx->count, 100);
@ -807,12 +827,11 @@ scd_doread(struct scd_softc *sc, int state, struct scd_mbx *mbxin)
SCD_WRITE(sc, OREG_COMMAND, CMD_SPIN_UP);
mbx->count = 300;
sc->ch_state = SCD_S_WAITSPIN;
sc->ch = timeout(scd_timeout, (caddr_t)sc, hz/100); /* XXX */
callout_reset(&sc->timer, hz / 100, scd_timeout, sc); /* XXX */
return;
}
/* send the read command */
critical_enter();
SCD_WRITE(sc, OREG_WPARAMS, sdata[0]);
SCD_WRITE(sc, OREG_WPARAMS, sdata[1]);
SCD_WRITE(sc, OREG_WPARAMS, sdata[2]);
@ -820,7 +839,6 @@ scd_doread(struct scd_softc *sc, int state, struct scd_mbx *mbxin)
SCD_WRITE(sc, OREG_WPARAMS, 0);
SCD_WRITE(sc, OREG_WPARAMS, 1);
SCD_WRITE(sc, OREG_COMMAND, CMD_READ);
critical_exit();
mbx->count = RDELAY_WAITREAD;
for (i = 0; i < 50; i++) {
@ -830,12 +848,12 @@ scd_doread(struct scd_softc *sc, int state, struct scd_mbx *mbxin)
}
sc->ch_state = SCD_S_WAITREAD;
sc->ch = timeout(scd_timeout, (caddr_t)sc, hz/100); /* XXX */
callout_reset(&sc->timer, hz / 100, scd_timeout, sc); /* XXX */
return;
case SCD_S_WAITREAD:
sc->ch_state = SCD_S_WAITREAD;
untimeout(scd_timeout,(caddr_t)sc, sc->ch);
callout_stop(&sc->timer);
if (mbx->count-- <= 0) {
if (STATUS_BIT(sc, SBIT_RESULT_READY))
goto got_param;
@ -847,7 +865,7 @@ scd_doread(struct scd_softc *sc, int state, struct scd_mbx *mbxin)
if (!(sc->data.flags & SCDVALID))
goto changed;
sc->ch_state = SCD_S_WAITREAD;
sc->ch = timeout(scd_timeout, (caddr_t)sc, hz/100); /* XXX */
callout_reset(&sc->timer, hz / 100, scd_timeout, sc); /* XXX */
return;
}
XDEBUG(sc, 2, "mbx->count (after RDY_BIT) = %d(%d)\n", mbx->count, RDELAY_WAITREAD);
@ -868,7 +886,7 @@ scd_doread(struct scd_softc *sc, int state, struct scd_mbx *mbxin)
case SCD_S_WAITPARAM:
sc->ch_state = SCD_S_WAITPARAM;
untimeout(scd_timeout,(caddr_t)sc, sc->ch);
callout_stop(&sc->timer);
if (mbx->count-- <= 0) {
device_printf(sc->dev, "timeout waiting for params\n");
goto readerr;
@ -877,7 +895,7 @@ scd_doread(struct scd_softc *sc, int state, struct scd_mbx *mbxin)
waitfor_param:
if (!STATUS_BIT(sc, SBIT_RESULT_READY)) {
sc->ch_state = SCD_S_WAITPARAM;
sc->ch = timeout(scd_timeout, (caddr_t)sc, hz/100); /* XXX */
callout_reset(&sc->timer, hz / 100, scd_timeout, sc); /* XXX */
return;
}
#ifdef SCD_DEBUG
@ -1293,7 +1311,9 @@ waitfor_status_bits(struct scd_softc *sc, int bits_set, int bits_clear)
{
break;
}
SCD_UNLOCK(sc);
pause("waitfor", hz/10);
SCD_LOCK(sc);
}
}
if ((c & bits_set) == bits_set &&
@ -1363,6 +1383,7 @@ scd_toc_entrys (struct scd_softc *sc, struct ioc_read_toc_entry *te)
toc_entry.addr.msf.second = bcd2bin(sc->data.toc[i].start_msf[1]);
toc_entry.addr.msf.frame = bcd2bin(sc->data.toc[i].start_msf[2]);
}
SCD_UNLOCK(sc);
/* copy the data back */
if (copyout(&toc_entry, te->data, sizeof(struct cd_toc_entry)) != 0)

View File

@ -125,6 +125,7 @@ scd_alloc_resources (device_t dev)
sc = device_get_softc(dev);
error = 0;
mtx_init(&sc->mtx, "scd", NULL, MTX_DEF);
if (sc->port_type) {
sc->port = bus_alloc_resource_any(dev, sc->port_type,
@ -134,13 +135,8 @@ scd_alloc_resources (device_t dev)
error = ENOMEM;
goto bad;
}
sc->port_bst = rman_get_bustag(sc->port);
sc->port_bsh = rman_get_bushandle(sc->port);
}
mtx_init(&sc->mtx, device_get_nameunit(dev),
"Interrupt lock", MTX_DEF | MTX_RECURSE);
bad:
return (error);
}
@ -152,14 +148,10 @@ scd_release_resources (device_t dev)
sc = device_get_softc(dev);
if (sc->port) {
if (sc->port)
bus_release_resource(dev, sc->port_type, sc->port_rid, sc->port);
sc->port_bst = 0;
sc->port_bsh = 0;
}
if (mtx_initialized(&sc->mtx) != 0)
mtx_destroy(&sc->mtx);
mtx_destroy(&sc->mtx);
return;
}

View File

@ -40,27 +40,26 @@ struct scd_softc {
struct resource * port;
int port_rid;
int port_type;
bus_space_tag_t port_bst;
bus_space_handle_t port_bsh;
struct mtx mtx;
struct callout_handle ch;
struct callout timer;
int ch_state;
struct scd_mbx * ch_mbxsave;
struct scd_data data;
};
#define SCD_LOCK(_sc) splx(&(_sc)->mtx
#define SCD_UNLOCK(_sc) splx(&(_sc)->mtx
#define SCD_LOCK(_sc) mtx_lock(&_sc->mtx)
#define SCD_UNLOCK(_sc) mtx_unlock(&_sc->mtx)
#define SCD_ASSERT_LOCKED(_sc) mtx_assert(&_sc->mtx, MA_OWNED)
#define SCD_READ(_sc, _reg) \
bus_space_read_1(_sc->port_bst, _sc->port_bsh, _reg)
bus_read_1(_sc->port, _reg)
#define SCD_READ_MULTI(_sc, _reg, _addr, _count) \
bus_space_read_multi_1(_sc->port_bst, _sc->port_bsh, _reg, _addr, _count)
bus_read_multi_1(_sc->port, _reg, _addr, _count)
#define SCD_WRITE(_sc, _reg, _val) \
bus_space_write_1(_sc->port_bst, _sc->port_bsh, _reg, _val)
bus_write_1(_sc->port, _reg, _val)
int scd_probe (struct scd_softc *);
int scd_attach (struct scd_softc *);