the PS/2 mouse problem
Morten Johansen
mail at morten-johansen.net
Wed Nov 5 13:07:54 PST 2003
Robert Watson wrote:
> There's been some speculation that the PS/2 mouse problem could be due to
> high interrupt latency for non-fast interrupt handlers (especially ones
> not MPSAFE) in 5.x. I think it would make a lot of sense for us to push
> Giant off both the PS/2 mouse and syscons interrupt handlers in the near
> future. For syscons, this would also improve the reliability of dropping
> into the debugger from a non-serial console.
>
> Robert N M Watson FreeBSD Core Team, TrustedBSD Projects
> robert at fledge.watson.org Network Associates Laboratories
Hi,
I tried pushing Giant out of psm a while ago, a patch is attached.
It did not help, but probably eases contention on Giant a bit.
psm gets a lot of interrupts.
I am still seeing occasional weirdness from the mouse.
It freezes then goes berserk (moving and triggering events) for a few
seconds.
The kernel says:
psmintr: delay too long; resetting byte count
(I was wrong about the message in a previous mail)
This actually happens more often in -stable than in -current.
moused or not does not make a difference.
The latest SCHED_ULE and interrupt changes, have not fixed this problem.
Otherwise, FreeBSD 5-current is the best OS I have ever run.
Morten Johansen
-------------- next part --------------
--- psm-0.c Wed Nov 5 19:30:09 2003
+++ psm.c Wed Nov 5 20:23:41 2003
@@ -129,6 +129,11 @@ __FBSDID("$FreeBSD: src/sys/isa/psm.c,v
#define PSM_NBLOCKIO(dev) (minor(dev) & 1)
#define PSM_MKMINOR(unit,block) (((unit) << 1) | ((block) ? 0:1))
+#define PSM_MTX_INIT(_sc, name) mtx_init(&_sc->psm_mtx, name, NULL, MTX_DEF)
+#define PSM_MTX_DESTROY(_sc) mtx_destroy(&_sc->psm_mtx)
+#define PSM_LOCK(_sc) mtx_lock(&(_sc)->psm_mtx)
+#define PSM_UNLOCK(_sc) mtx_unlock(&(_sc)->psm_mtx)
+
/* ring buffer */
typedef struct ringbuf {
int count; /* # of valid elements in the buffer */
@@ -160,9 +165,10 @@ struct psm_softc { /* Driver status inf
int syncerrors;
struct timeval inputtimeout;
int watchdog; /* watchdog timer flag */
- struct callout_handle callout; /* watchdog timer call out */
+ struct callout callout; /* watchdog timer call out */
dev_t dev;
dev_t bdev;
+ struct mtx psm_mtx;
};
static devclass_t psm_devclass;
#define PSM_SOFTC(unit) ((struct psm_softc*)devclass_get_softc(psm_devclass, unit))
@@ -250,6 +256,7 @@ static int doinitialize(struct psm_softc
static int doopen(struct psm_softc *, int);
static int reinitialize(struct psm_softc *, int);
static char *model_name(int);
+static void _psmintr(void *);
static void psmintr(void *);
static void psmtimeout(void *);
@@ -769,7 +776,7 @@ doopen(struct psm_softc *sc, int command
/* start the watchdog timer */
sc->watchdog = FALSE;
- sc->callout = timeout(psmtimeout, (void *)(uintptr_t)sc, hz*2);
+ callout_reset(&sc->callout, hz*2, psmtimeout, (void *)(uintptr_t)sc);
return (0);
}
@@ -779,17 +786,16 @@ reinitialize(struct psm_softc *sc, int d
{
int err;
int c;
- int s;
/* don't let anybody mess with the aux device */
if (!kbdc_lock(sc->kbdc, TRUE))
return (EIO);
- s = spltty();
+ PSM_LOCK(sc);
/* block our watchdog timer */
sc->watchdog = FALSE;
- untimeout(psmtimeout, (void *)(uintptr_t)sc, sc->callout);
- callout_handle_init(&sc->callout);
+ callout_stop(&sc->callout);
+ callout_init(&sc->callout, CALLOUT_MPSAFE);
/* save the current controller command byte */
empty_both_buffers(sc->kbdc, 10);
@@ -804,7 +810,7 @@ reinitialize(struct psm_softc *sc, int d
KBD_DISABLE_KBD_PORT | KBD_DISABLE_KBD_INT
| KBD_ENABLE_AUX_PORT | KBD_DISABLE_AUX_INT)) {
/* CONTROLLER ERROR */
- splx(s);
+ PSM_UNLOCK(sc);
kbdc_lock(sc->kbdc, FALSE);
log(LOG_ERR, "psm%d: unable to set the command byte (reinitialize).\n",
sc->unit);
@@ -834,7 +840,7 @@ reinitialize(struct psm_softc *sc, int d
err = ENXIO;
}
}
- splx(s);
+ PSM_UNLOCK(sc);
/* restore the driver state */
if ((sc->state & PSM_OPEN) && (err == 0)) {
@@ -1225,9 +1231,10 @@ psmattach(device_t dev)
if (sc == NULL) /* shouldn't happen */
return (ENXIO);
+ PSM_MTX_INIT(sc, device_get_nameunit(dev));
/* Setup initial state */
sc->state = PSM_VALID;
- callout_handle_init(&sc->callout);
+ callout_init(&sc->callout, CALLOUT_MPSAFE);
/* Setup our interrupt handler */
rid = KBDC_RID_AUX;
@@ -1235,7 +1242,7 @@ psmattach(device_t dev)
RF_SHAREABLE | RF_ACTIVE);
if (sc->intr == NULL)
return (ENXIO);
- error = bus_setup_intr(dev, sc->intr, INTR_TYPE_TTY, psmintr, sc, &sc->ih);
+ error = bus_setup_intr(dev, sc->intr, INTR_TYPE_TTY | INTR_MPSAFE, psmintr, sc, &sc->ih);
if (error) {
bus_release_resource(dev, SYS_RES_IRQ, rid, sc->intr);
return (error);
@@ -1282,6 +1289,8 @@ psmdetach(device_t dev)
destroy_dev(sc->dev);
destroy_dev(sc->bdev);
+
+ PSM_MTX_DESTROY(sc);
return 0;
}
@@ -1293,7 +1302,6 @@ psmopen(dev_t dev, int flag, int fmt, st
struct psm_softc *sc;
int command_byte;
int err;
- int s;
/* Get device data */
sc = PSM_SOFTC(unit);
@@ -1334,7 +1342,7 @@ psmopen(dev_t dev, int flag, int fmt, st
return (EIO);
/* save the current controller command byte */
- s = spltty();
+ PSM_LOCK(sc);
command_byte = get_controller_command_byte(sc->kbdc);
/* enable the aux port and temporalily disable the keyboard */
@@ -1344,8 +1352,8 @@ psmopen(dev_t dev, int flag, int fmt, st
KBD_DISABLE_KBD_PORT | KBD_DISABLE_KBD_INT
| KBD_ENABLE_AUX_PORT | KBD_DISABLE_AUX_INT)) {
/* CONTROLLER ERROR; do you know how to get out of this? */
+ PSM_UNLOCK(sc);
kbdc_lock(sc->kbdc, FALSE);
- splx(s);
log(LOG_ERR, "psm%d: unable to set the command byte (psmopen).\n",
unit);
return (EIO);
@@ -1357,7 +1365,7 @@ psmopen(dev_t dev, int flag, int fmt, st
* but timeout routines will be blocked by the poll flag set
* via `kbdc_lock()'
*/
- splx(s);
+ PSM_UNLOCK(sc);
/* enable the mouse device */
err = doopen(sc, command_byte);
@@ -1376,18 +1384,17 @@ psmclose(dev_t dev, int flag, int fmt, s
struct psm_softc *sc = PSM_SOFTC(unit);
int stat[3];
int command_byte;
- int s;
/* don't let timeout routines in the keyboard driver to poll the kbdc */
if (!kbdc_lock(sc->kbdc, TRUE))
return (EIO);
/* save the current controller command byte */
- s = spltty();
+ PSM_LOCK(sc);
command_byte = get_controller_command_byte(sc->kbdc);
if (command_byte == -1) {
+ PSM_UNLOCK(sc);
kbdc_lock(sc->kbdc, FALSE);
- splx(s);
return (EIO);
}
@@ -1405,11 +1412,11 @@ psmclose(dev_t dev, int flag, int fmt, s
* so long as the mouse will accept the DISABLE command.
*/
}
- splx(s);
+ PSM_UNLOCK(sc);
/* stop the watchdog timer */
- untimeout(psmtimeout, (void *)(uintptr_t)sc, sc->callout);
- callout_handle_init(&sc->callout);
+ callout_stop(&sc->callout);
+ callout_init(&sc->callout, CALLOUT_MPSAFE);
/* remove anything left in the output buffer */
empty_aux_buffer(sc->kbdc, 10);
@@ -1517,36 +1524,35 @@ psmread(dev_t dev, struct uio *uio, int
register struct psm_softc *sc = PSM_SOFTC(PSM_UNIT(dev));
unsigned char buf[PSM_SMALLBUFSIZE];
int error = 0;
- int s;
int l;
if ((sc->state & PSM_VALID) == 0)
return EIO;
/* block until mouse activity occured */
- s = spltty();
+ PSM_LOCK(sc);
while (sc->queue.count <= 0) {
if (PSM_NBLOCKIO(dev)) {
- splx(s);
+ PSM_UNLOCK(sc);
return EWOULDBLOCK;
}
sc->state |= PSM_ASLP;
error = tsleep( sc, PZERO | PCATCH, "psmrea", 0);
sc->state &= ~PSM_ASLP;
if (error) {
- splx(s);
+ PSM_UNLOCK(sc);
return error;
} else if ((sc->state & PSM_VALID) == 0) {
/* the device disappeared! */
- splx(s);
+ PSM_UNLOCK(sc);
return EIO;
}
}
- splx(s);
+ PSM_UNLOCK(sc);
/* copy data to the user land */
while ((sc->queue.count > 0) && (uio->uio_resid > 0)) {
- s = spltty();
+ PSM_LOCK(sc);
l = imin(sc->queue.count, uio->uio_resid);
if (l > sizeof(buf))
l = sizeof(buf);
@@ -1561,7 +1567,7 @@ psmread(dev_t dev, struct uio *uio, int
}
sc->queue.count -= l;
sc->queue.head = (sc->queue.head + l) % sizeof(sc->queue.buf);
- splx(s);
+ PSM_UNLOCK(sc);
error = uiomove(buf, l, uio);
if (error)
break;
@@ -1573,12 +1579,10 @@ psmread(dev_t dev, struct uio *uio, int
static int
block_mouse_data(struct psm_softc *sc, int *c)
{
- int s;
-
if (!kbdc_lock(sc->kbdc, TRUE))
return EIO;
- s = spltty();
+ PSM_LOCK(sc);
*c = get_controller_command_byte(sc->kbdc);
if ((*c == -1)
|| !set_controller_command_byte(sc->kbdc,
@@ -1586,7 +1590,7 @@ block_mouse_data(struct psm_softc *sc, i
KBD_DISABLE_KBD_PORT | KBD_DISABLE_KBD_INT
| KBD_ENABLE_AUX_PORT | KBD_DISABLE_AUX_INT)) {
/* this is CONTROLLER ERROR */
- splx(s);
+ PSM_UNLOCK(sc);
kbdc_lock(sc->kbdc, FALSE);
return EIO;
}
@@ -1606,7 +1610,7 @@ block_mouse_data(struct psm_softc *sc, i
empty_aux_buffer(sc->kbdc, 0); /* flush the queue */
read_aux_data_no_wait(sc->kbdc); /* throw away data if any */
sc->inputbytes = 0;
- splx(s);
+ PSM_UNLOCK(sc);
return 0;
}
@@ -1650,30 +1654,31 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
int stat[3];
int command_byte;
int error = 0;
- int s;
-
+
+ DROP_GIANT();
+
/* Perform IOCTL command */
switch (cmd) {
case OLD_MOUSE_GETHWINFO:
- s = spltty();
+ PSM_LOCK(sc);
((old_mousehw_t *)addr)->buttons = sc->hw.buttons;
((old_mousehw_t *)addr)->iftype = sc->hw.iftype;
((old_mousehw_t *)addr)->type = sc->hw.type;
((old_mousehw_t *)addr)->hwid = sc->hw.hwid & 0x00ff;
- splx(s);
+ PSM_UNLOCK(sc);
break;
case MOUSE_GETHWINFO:
- s = spltty();
+ PSM_LOCK(sc);
*(mousehw_t *)addr = sc->hw;
if (sc->mode.level == PSM_LEVEL_BASE)
((mousehw_t *)addr)->model = MOUSE_MODEL_GENERIC;
- splx(s);
+ PSM_UNLOCK(sc);
break;
case OLD_MOUSE_GETMODE:
- s = spltty();
+ PSM_LOCK(sc);
switch (sc->mode.level) {
case PSM_LEVEL_BASE:
((old_mousemode_t *)addr)->protocol = MOUSE_PROTO_PS2;
@@ -1688,11 +1693,11 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
((old_mousemode_t *)addr)->rate = sc->mode.rate;
((old_mousemode_t *)addr)->resolution = sc->mode.resolution;
((old_mousemode_t *)addr)->accelfactor = sc->mode.accelfactor;
- splx(s);
+ PSM_UNLOCK(sc);
break;
case MOUSE_GETMODE:
- s = spltty();
+ PSM_LOCK(sc);
*(mousemode_t *)addr = sc->mode;
((mousemode_t *)addr)->resolution =
MOUSE_RES_LOW - sc->mode.resolution;
@@ -1712,7 +1717,7 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
((mousemode_t *)addr)->protocol = MOUSE_PROTO_PS2;
break;
}
- splx(s);
+ PSM_UNLOCK(sc);
break;
case OLD_MOUSE_SETMODE:
@@ -1736,17 +1741,23 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
}
/* adjust and validate parameters. */
- if (mode.rate > UCHAR_MAX)
- return EINVAL;
+ if (mode.rate > UCHAR_MAX) {
+ error = EINVAL;
+ break;
+ }
if (mode.rate == 0)
mode.rate = sc->dflt_mode.rate;
else if (mode.rate == -1)
/* don't change the current setting */
;
- else if (mode.rate < 0)
- return EINVAL;
- if (mode.resolution >= UCHAR_MAX)
- return EINVAL;
+ else if (mode.rate < 0) {
+ error = EINVAL;
+ break;
+ }
+ if (mode.resolution >= UCHAR_MAX) {
+ error = EINVAL;
+ break;
+ }
if (mode.resolution >= 200)
mode.resolution = MOUSE_RES_HIGH;
else if (mode.resolution >= 100)
@@ -1765,18 +1776,22 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
if (mode.level == -1)
/* don't change the current setting */
mode.level = sc->mode.level;
- else if ((mode.level < PSM_LEVEL_MIN) || (mode.level > PSM_LEVEL_MAX))
- return EINVAL;
+ else if ((mode.level < PSM_LEVEL_MIN) || (mode.level > PSM_LEVEL_MAX)) {
+ error = EINVAL;
+ break;
+ }
if (mode.accelfactor == -1)
/* don't change the current setting */
mode.accelfactor = sc->mode.accelfactor;
- else if (mode.accelfactor < 0)
- return EINVAL;
+ else if (mode.accelfactor < 0) {
+ error = EINVAL;
+ break;
+ }
/* don't allow anybody to poll the keyboard controller */
error = block_mouse_data(sc, &command_byte);
if (error)
- return error;
+ break;
/* set mouse parameters */
if (mode.rate > 0)
@@ -1786,12 +1801,12 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
set_mouse_scaling(sc->kbdc, 1);
get_mouse_status(sc->kbdc, stat, 0, 3);
- s = spltty();
+ PSM_LOCK(sc);
sc->mode.rate = mode.rate;
sc->mode.resolution = mode.resolution;
sc->mode.accelfactor = mode.accelfactor;
sc->mode.level = mode.level;
- splx(s);
+ PSM_UNLOCK(sc);
unblock_mouse_data(sc, command_byte);
break;
@@ -1801,13 +1816,15 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
break;
case MOUSE_SETLEVEL:
- if ((*(int *)addr < PSM_LEVEL_MIN) || (*(int *)addr > PSM_LEVEL_MAX))
- return EINVAL;
+ if ((*(int *)addr < PSM_LEVEL_MIN) || (*(int *)addr > PSM_LEVEL_MAX)) {
+ error = EINVAL;
+ break;
+ }
sc->mode.level = *(int *)addr;
break;
case MOUSE_GETSTATUS:
- s = spltty();
+ PSM_LOCK(sc);
status = sc->status;
sc->status.flags = 0;
sc->status.obutton = sc->status.button;
@@ -1815,7 +1832,7 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
sc->status.dx = 0;
sc->status.dy = 0;
sc->status.dz = 0;
- splx(s);
+ PSM_UNLOCK(sc);
*(mousestatus_t *)addr = status;
break;
@@ -1823,26 +1840,29 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
case MOUSE_GETVARS:
var = (mousevar_t *)addr;
bzero(var, sizeof(*var));
- s = spltty();
+ PSM_LOCK(sc);
var->var[0] = MOUSE_VARS_PS2_SIG;
var->var[1] = sc->config;
var->var[2] = sc->flags;
- splx(s);
+ PSM_UNLOCK(sc);
break;
case MOUSE_SETVARS:
- return ENODEV;
+ error = ENODEV;
+ break;
#endif /* MOUSE_GETVARS */
case MOUSE_READSTATE:
case MOUSE_READDATA:
data = (mousedata_t *)addr;
- if (data->len > sizeof(data->buf)/sizeof(data->buf[0]))
- return EINVAL;
+ if (data->len > sizeof(data->buf)/sizeof(data->buf[0])) {
+ error = EINVAL;
+ break;
+ }
error = block_mouse_data(sc, &command_byte);
if (error)
- return error;
+ break;
if ((data->len = get_mouse_status(sc->kbdc, data->buf,
(cmd == MOUSE_READDATA) ? 1 : 0, data->len)) <= 0)
error = EIO;
@@ -1852,8 +1872,10 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
#if (defined(MOUSE_SETRESOLUTION))
case MOUSE_SETRESOLUTION:
mode.resolution = *(int *)addr;
- if (mode.resolution >= UCHAR_MAX)
- return EINVAL;
+ if (mode.resolution >= UCHAR_MAX) {
+ error = EINVAL;
+ break;
+ }
else if (mode.resolution >= 200)
mode.resolution = MOUSE_RES_HIGH;
else if (mode.resolution >= 100)
@@ -1871,7 +1893,7 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
error = block_mouse_data(sc, &command_byte);
if (error)
- return error;
+ break;
sc->mode.resolution = set_mouse_resolution(sc->kbdc, mode.resolution);
if (sc->mode.resolution != mode.resolution)
error = EIO;
@@ -1882,8 +1904,10 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
#if (defined(MOUSE_SETRATE))
case MOUSE_SETRATE:
mode.rate = *(int *)addr;
- if (mode.rate > UCHAR_MAX)
- return EINVAL;
+ if (mode.rate > UCHAR_MAX) {
+ error = EINVAL;
+ break;
+ }
if (mode.rate == 0)
mode.rate = sc->dflt_mode.rate;
else if (mode.rate < 0)
@@ -1891,7 +1915,7 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
error = block_mouse_data(sc, &command_byte);
if (error)
- return error;
+ break;
sc->mode.rate = set_mouse_sampling_rate(sc->kbdc, mode.rate);
if (sc->mode.rate != mode.rate)
error = EIO;
@@ -1901,12 +1925,14 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
#if (defined(MOUSE_SETSCALING))
case MOUSE_SETSCALING:
- if ((*(int *)addr <= 0) || (*(int *)addr > 2))
- return EINVAL;
+ if ((*(int *)addr <= 0) || (*(int *)addr > 2)) {
+ error = EINVAL;
+ break;
+ }
error = block_mouse_data(sc, &command_byte);
if (error)
- return error;
+ break;
if (!set_mouse_scaling(sc->kbdc, *(int *)addr))
error = EIO;
unblock_mouse_data(sc, command_byte);
@@ -1917,7 +1943,7 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
case MOUSE_GETHWID:
error = block_mouse_data(sc, &command_byte);
if (error)
- return error;
+ break;
sc->hw.hwid &= ~0x00ff;
sc->hw.hwid |= get_aux_id(sc->kbdc);
*(int *)addr = sc->hw.hwid & 0x00ff;
@@ -1926,9 +1952,11 @@ psmioctl(dev_t dev, u_long cmd, caddr_t
#endif /* MOUSE_GETHWID */
default:
- return ENOTTY;
+ error = ENOTTY;
}
+ PICKUP_GIANT();
+
return error;
}
@@ -1936,24 +1964,31 @@ static void
psmtimeout(void *arg)
{
struct psm_softc *sc;
- int s;
sc = (struct psm_softc *)arg;
- s = spltty();
+ PSM_LOCK(sc);
if (sc->watchdog && kbdc_lock(sc->kbdc, TRUE)) {
if (verbose >= 4)
log(LOG_DEBUG, "psm%d: lost interrupt?\n", sc->unit);
- psmintr(sc);
+ _psmintr(sc);
kbdc_lock(sc->kbdc, FALSE);
}
sc->watchdog = TRUE;
- splx(s);
- sc->callout = timeout(psmtimeout, (void *)(uintptr_t)sc, hz);
+ PSM_UNLOCK(sc);
+ callout_reset(&sc->callout, hz, psmtimeout, (void *)(uintptr_t)sc);
}
static void
psmintr(void *arg)
{
+ PSM_LOCK((struct psm_softc *)arg);
+ _psmintr(arg);
+ PSM_UNLOCK((struct psm_softc *)arg);
+}
+
+static void
+_psmintr(void *arg)
+{
/*
* the table to turn PS/2 mouse button bits (MOUSE_PS2_BUTTON?DOWN)
* into `mousestatus' button bits (MOUSE_BUTTON?DOWN).
@@ -2379,18 +2414,17 @@ static int
psmpoll(dev_t dev, int events, struct thread *td)
{
struct psm_softc *sc = PSM_SOFTC(PSM_UNIT(dev));
- int s;
int revents = 0;
/* Return true if a mouse event available */
- s = spltty();
+ PSM_LOCK(sc);
if (events & (POLLIN | POLLRDNORM)) {
if (sc->queue.count > 0)
revents |= events & (POLLIN | POLLRDNORM);
else
selrecord(td, &sc->rsel);
}
- splx(s);
+ PSM_UNLOCK(sc);
return (revents);
}
More information about the freebsd-current
mailing list