From owner-freebsd-current@FreeBSD.ORG Wed Nov 5 13:07:54 2003 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 932B816A4CE; Wed, 5 Nov 2003 13:07:54 -0800 (PST) Received: from oslonett.no (imail.oslonett.no [194.234.215.50]) by mx1.FreeBSD.org (Postfix) with ESMTP id B124D43FF2; Wed, 5 Nov 2003 13:07:51 -0800 (PST) (envelope-from mail@morten-johansen.net) Received: from morten-johansen.net [213.234.113.122] by oslonett.no with ESMTP (SMTPD32-8.01) id A6A22FDD006A; Wed, 05 Nov 2003 22:07:46 +0100 Message-ID: <3FA966B2.9040704@morten-johansen.net> Date: Wed, 05 Nov 2003 22:08:02 +0100 From: Morten Johansen User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.4) Gecko/20030817 X-Accept-Language: no, nb, en-us, en MIME-Version: 1.0 To: Robert Watson Content-Type: multipart/mixed; boundary="------------080303040204060804090000" X-Declude-Sender: mail@morten-johansen.net [213.234.113.122] X-Declude-Spoolname: D66a22fdd006aace5.SMD X-Note: This E-mail was scanned by Declude JunkMail (www.declude.com) for spam. cc: freebsd-current@freebsd.org Subject: the PS/2 mouse problem X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Nov 2003 21:07:54 -0000 This is a multi-part message in MIME format. --------------080303040204060804090000 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit 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@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 --------------080303040204060804090000 Content-Type: text/plain; name="patch-psm" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="patch-psm" --- 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); } --------------080303040204060804090000--