USB keyboard lockups & patch to (probably) fix.
Twice in the last month my workstation has locked up. It was very odd.
Everything would be going just fine, a ton of open windows, browser, etc,
but in both cases I simply hit a key on the keyboard, and poof, the box
decided to take a permanent siesta.
I was able to get a dump the second time. It turns out that the CLIST
module's freelist (kern/tty_subr.c primarily) was getting corrupted.
And then it hit me... tty's aren't the only things that use the clist
module. The keyboard code does too, and I had recently switched to
a USB keyboard. It turns out that the clist code was only protecting
itself against tty interrupts (spltty()), so every time you hit a key
on a USB keyboard while other clist-using entities (like anything that
uses a tty or pty) are active, there is a small but not-zero chance that
you will bork the machine.
So, here's a tentitive patch to fix the problem. Basically it protects
clist operations with a critical section instead of with spltty(). I
also revamped the clist code a bit and added a large number of KKASSERTs
to make it more robust.
My intention is to commit this on Friday and to also slip the
DragonFly_Stable tag for the commit. Testing would be appreciated!
-Matt
Matthew Dillon
<dillon@backplane.com>
Index: dev/misc/kbd/kbd.c
===================================================================
RCS file: /cvs/src/sys/dev/misc/kbd/kbd.c,v
retrieving revision 1.12
diff -u -r1.12 kbd.c
--- dev/misc/kbd/kbd.c 19 Sep 2004 02:15:44 -0000 1.12
+++ dev/misc/kbd/kbd.c 5 Oct 2004 20:54:26 -0000
@@ -26,6 +26,13 @@
* $FreeBSD: src/sys/dev/kbd/kbd.c,v 1.17.2.2 2001/07/30 16:46:43 yokota Exp $
* $DragonFly: src/sys/dev/misc/kbd/kbd.c,v 1.12 2004/09/19 02:15:44 dillon Exp $
*/
+/*
+ * Generic keyboard driver.
+ *
+ * Interrupt note: keyboards use clist functions and since usb keyboard
+ * interrupts are not protected by spltty(), we must use a critical section
+ * to protect against corruption.
+ */
#include "opt_kbd.h"
@@ -39,6 +46,8 @@
#include <sys/poll.h>
#include <sys/vnode.h>
#include <sys/uio.h>
+#include <sys/thread.h>
+#include <sys/thread2.h>
#include <machine/console.h>
@@ -80,9 +89,7 @@
keyboard_t **new_kbd;
keyboard_switch_t **new_kbdsw;
int newsize;
- int s;
- s = spltty();
newsize = ((keyboards + ARRAY_DELTA)/ARRAY_DELTA)*ARRAY_DELTA;
new_kbd = malloc(sizeof(*new_kbd) * newsize, M_DEVBUF,
M_WAITOK | M_ZERO);
@@ -90,6 +97,7 @@
M_WAITOK | M_ZERO);
bcopy(keyboard, new_kbd, sizeof(*keyboard)*keyboards);
bcopy(kbdsw, new_kbdsw, sizeof(*kbdsw)*keyboards);
+ crit_enter();
if (keyboards > 1) {
free(keyboard, M_DEVBUF);
free(kbdsw, M_DEVBUF);
@@ -97,7 +105,7 @@
keyboard = new_kbd;
kbdsw = new_kbdsw;
keyboards = newsize;
- splx(s);
+ crit_exit();
if (bootverbose)
printf("kbd: new array size %d\n", keyboards);
@@ -226,23 +234,22 @@
kbd_unregister(keyboard_t *kbd)
{
int error;
- int s;
if ((kbd->kb_index < 0) || (kbd->kb_index >= keyboards))
return ENOENT;
if (keyboard[kbd->kb_index] != kbd)
return ENOENT;
- s = spltty();
+ crit_enter();
if (KBD_IS_BUSY(kbd)) {
error = (*kbd->kb_callback.kc_func)(kbd, KBDIO_UNLOADING,
kbd->kb_callback.kc_arg);
if (error) {
- splx(s);
+ crit_exit();
return error;
}
if (KBD_IS_BUSY(kbd)) {
- splx(s);
+ crit_exit();
return EBUSY;
}
}
@@ -250,7 +257,7 @@
keyboard[kbd->kb_index] = NULL;
kbdsw[kbd->kb_index] = NULL;
- splx(s);
+ crit_exit();
return 0;
}
@@ -315,16 +322,15 @@
void *arg)
{
int index;
- int s;
if (func == NULL)
return -1;
- s = spltty();
+ crit_enter();
index = kbd_find_keyboard(driver, unit);
if (index >= 0) {
if (KBD_IS_BUSY(keyboard[index])) {
- splx(s);
+ crit_exit();
return -1;
}
callout_init(&keyboard[index]->kb_atkbd_timeout_ch);
@@ -334,7 +340,7 @@
keyboard[index]->kb_callback.kc_arg = arg;
(*kbdsw[index]->clear_state)(keyboard[index]);
}
- splx(s);
+ crit_exit();
return index;
}
@@ -342,9 +348,8 @@
kbd_release(keyboard_t *kbd, void *id)
{
int error;
- int s;
- s = spltty();
+ crit_enter();
if (!KBD_IS_VALID(kbd) || !KBD_IS_BUSY(kbd)) {
error = EINVAL;
} else if (kbd->kb_token != id) {
@@ -358,7 +363,7 @@
(*kbdsw[kbd->kb_index]->clear_state)(kbd);
error = 0;
}
- splx(s);
+ crit_exit();
return error;
}
@@ -367,9 +372,8 @@
void *arg)
{
int error;
- int s;
- s = spltty();
+ crit_enter();
if (!KBD_IS_VALID(kbd) || !KBD_IS_BUSY(kbd)) {
error = EINVAL;
} else if (kbd->kb_token != id) {
@@ -381,7 +385,7 @@
kbd->kb_callback.kc_arg = arg;
error = 0;
}
- splx(s);
+ crit_exit();
return error;
}
@@ -523,20 +527,19 @@
{
keyboard_t *kbd;
genkbd_softc_t *sc;
- int s;
int i;
- s = spltty();
+ crit_enter();
sc = dev->si_drv1;
kbd = kbd_get_keyboard(KBD_INDEX(dev));
if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) {
- splx(s);
+ crit_exit();
return ENXIO;
}
i = kbd_allocate(kbd->kb_name, kbd->kb_unit, sc,
genkbd_event, (void *)sc);
if (i < 0) {
- splx(s);
+ crit_exit();
return EBUSY;
}
/* assert(i == kbd->kb_index) */
@@ -553,7 +556,7 @@
clist_alloc_cblocks(&sc->gkb_q, KB_QSIZE, KB_QSIZE/2); /* XXX */
sc->gkb_rsel.si_flags = 0;
sc->gkb_rsel.si_pid = 0;
- splx(s);
+ crit_exit();
return 0;
}
@@ -563,13 +566,12 @@
{
keyboard_t *kbd;
genkbd_softc_t *sc;
- int s;
/*
* NOTE: the device may have already become invalid.
* kbd == NULL || !KBD_IS_VALID(kbd)
*/
- s = spltty();
+ crit_enter();
sc = dev->si_drv1;
kbd = kbd_get_keyboard(KBD_INDEX(dev));
if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) {
@@ -580,7 +582,7 @@
clist_free_cblocks(&sc->gkb_q);
#endif
}
- splx(s);
+ crit_exit();
return 0;
}
@@ -592,35 +594,34 @@
u_char buffer[KB_BUFSIZE];
int len;
int error;
- int s;
/* wait for input */
- s = spltty();
+ crit_enter();
sc = dev->si_drv1;
kbd = kbd_get_keyboard(KBD_INDEX(dev));
if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) {
- splx(s);
+ crit_exit();
return ENXIO;
}
while (sc->gkb_q.c_cc == 0) {
if (flag & IO_NDELAY) {
- splx(s);
+ crit_exit();
return EWOULDBLOCK;
}
sc->gkb_flags |= KB_ASLEEP;
error = tsleep((caddr_t)sc, PCATCH, "kbdrea", 0);
kbd = kbd_get_keyboard(KBD_INDEX(dev));
if ((kbd == NULL) || !KBD_IS_VALID(kbd)) {
- splx(s);
+ crit_exit();
return ENXIO; /* our keyboard has gone... */
}
if (error) {
sc->gkb_flags &= ~KB_ASLEEP;
- splx(s);
+ crit_exit();
return error;
}
}
- splx(s);
+ crit_exit();
/* copy as much input as possible */
error = 0;
@@ -669,10 +670,9 @@
keyboard_t *kbd;
genkbd_softc_t *sc;
int revents;
- int s;
revents = 0;
- s = spltty();
+ crit_enter();
sc = dev->si_drv1;
kbd = kbd_get_keyboard(KBD_INDEX(dev));
if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) {
@@ -683,7 +683,7 @@
else
selrecord(td, &sc->gkb_rsel);
}
- splx(s);
+ crit_exit();
return revents;
}
@@ -799,10 +799,9 @@
{
keyarg_t *keyp;
fkeyarg_t *fkeyp;
- int s;
int i;
- s = spltty();
+ crit_enter();
switch (cmd) {
case KDGKBINFO: /* get keyboard information */
@@ -834,7 +833,7 @@
bcopy(arg, kbd->kb_keymap, sizeof(*kbd->kb_keymap));
break;
#else
- splx(s);
+ crit_exit();
return ENODEV;
#endif
@@ -842,7 +841,7 @@
keyp = (keyarg_t *)arg;
if (keyp->keynum >= sizeof(kbd->kb_keymap->key)
/sizeof(kbd->kb_keymap->key[0])) {
- splx(s);
+ crit_exit();
return EINVAL;
}
bcopy(&kbd->kb_keymap->key[keyp->keynum], &keyp->key,
@@ -853,14 +852,14 @@
keyp = (keyarg_t *)arg;
if (keyp->keynum >= sizeof(kbd->kb_keymap->key)
/sizeof(kbd->kb_keymap->key[0])) {
- splx(s);
+ crit_exit();
return EINVAL;
}
bcopy(&keyp->key, &kbd->kb_keymap->key[keyp->keynum],
sizeof(keyp->key));
break;
#else
- splx(s);
+ crit_exit();
return ENODEV;
#endif
@@ -872,14 +871,14 @@
bcopy(arg, kbd->kb_accentmap, sizeof(*kbd->kb_accentmap));
break;
#else
- splx(s);
+ crit_exit();
return ENODEV;
#endif
case GETFKEY: /* get functionkey string */
fkeyp = (fkeyarg_t *)arg;
if (fkeyp->keynum >= kbd->kb_fkeytab_size) {
- splx(s);
+ crit_exit();
return EINVAL;
}
bcopy(kbd->kb_fkeytab[fkeyp->keynum].str, fkeyp->keydef,
@@ -890,7 +889,7 @@
#ifndef KBD_DISABLE_KEYMAP_LOAD
fkeyp = (fkeyarg_t *)arg;
if (fkeyp->keynum >= kbd->kb_fkeytab_size) {
- splx(s);
+ crit_exit();
return EINVAL;
}
kbd->kb_fkeytab[fkeyp->keynum].len = imin(fkeyp->flen, MAXFK);
@@ -898,16 +897,16 @@
kbd->kb_fkeytab[fkeyp->keynum].len);
break;
#else
- splx(s);
+ crit_exit();
return ENODEV;
#endif
default:
- splx(s);
+ crit_exit();
return ENOIOCTL;
}
- splx(s);
+ crit_exit();
return 0;
}
Index: kern/tty.c
===================================================================
RCS file: /cvs/src/sys/kern/tty.c,v
retrieving revision 1.12
diff -u -r1.12 tty.c
--- kern/tty.c 13 Sep 2004 16:22:36 -0000 1.12
+++ kern/tty.c 5 Oct 2004 20:49:31 -0000
@@ -80,6 +80,7 @@
#include <sys/proc.h>
#define TTYDEFCHARS
#include <sys/tty.h>
+#include <sys/clist.h>
#undef TTYDEFCHARS
#include <sys/fcntl.h>
#include <sys/conf.h>
@@ -2554,8 +2555,7 @@
if (tp)
return(tp);
- tp = malloc(sizeof *tp, M_TTYS, M_WAITOK);
- bzero(tp, sizeof *tp);
+ tp = malloc(sizeof *tp, M_TTYS, M_WAITOK|M_ZERO);
ttyregister(tp);
return (tp);
}
Index: kern/tty_subr.c
===================================================================
RCS file: /cvs/src/sys/kern/tty_subr.c,v
retrieving revision 1.4
diff -u -r1.4 tty_subr.c
--- kern/tty_subr.c 8 Jan 2004 18:39:18 -0000 1.4
+++ kern/tty_subr.c 5 Oct 2004 20:55:22 -0000
@@ -30,6 +30,16 @@
/*
* clist support routines
+ *
+ * NOTE on cblock->c_cf: This pointer may point at the base of a cblock,
+ * which is &cblock->c_info[0], but will never
+ * point at the end of a cblock (char *)(cblk + 1)
+ *
+ * NOTE on cblock->c_cl: This pointer will never point at the base of
+ * a block but may point at the end of one.
+ *
+ * These routines may be used by more then just ttys, so a critical section
+ * must be used to access the free list, and for general safety.
*/
#include <sys/param.h>
@@ -38,6 +48,7 @@
#include <sys/malloc.h>
#include <sys/tty.h>
#include <sys/clist.h>
+#include <sys/thread2.h>
static void clist_init (void *);
SYSINIT(clist, SI_SUB_CLIST, SI_ORDER_FIRST, clist_init, NULL)
@@ -76,8 +87,7 @@
*/
/* ARGSUSED*/
static void
-clist_init(dummy)
- void *dummy;
+clist_init(void *dummy)
{
/*
* Allocate an initial base set of cblocks as a 'slush'.
@@ -88,46 +98,55 @@
* interrupt handlers when it may be unsafe to call malloc().
*/
cblock_alloc_cblocks(cslushcount = INITIAL_CBLOCKS);
+ KKASSERT(sizeof(struct cblock) == CBLOCK);
}
/*
* Remove a cblock from the cfreelist queue and return a pointer
* to it.
+ *
+ * May not block.
*/
-static __inline struct cblock *
-cblock_alloc()
+static struct cblock *
+cblock_alloc(void)
{
struct cblock *cblockp;
cblockp = cfreelist;
if (cblockp == NULL)
panic("clist reservation botch");
- cfreelist = cblockp->c_next;
- cblockp->c_next = NULL;
+ KKASSERT(cblockp->c_head.ch_magic == CLIST_MAGIC_FREE);
+ cfreelist = cblockp->c_head.ch_next;
+ cblockp->c_head.ch_next = NULL;
+ cblockp->c_head.ch_magic = CLIST_MAGIC_USED;
cfreecount -= CBSIZE;
return (cblockp);
}
/*
* Add a cblock to the cfreelist queue.
+ *
+ * May not block, must be called in a critical section
*/
-static __inline void
-cblock_free(cblockp)
- struct cblock *cblockp;
+static void
+cblock_free(struct cblock *cblockp)
{
if (isset(cblockp->c_quote, CBQSIZE * NBBY - 1))
bzero(cblockp->c_quote, sizeof cblockp->c_quote);
- cblockp->c_next = cfreelist;
+ KKASSERT(cblockp->c_head.ch_magic == CLIST_MAGIC_USED);
+ cblockp->c_head.ch_next = cfreelist;
+ cblockp->c_head.ch_magic = CLIST_MAGIC_FREE;
cfreelist = cblockp;
cfreecount += CBSIZE;
}
/*
* Allocate some cblocks for the cfreelist queue.
+ *
+ * This routine my block, but still must be called in a critical section
*/
static void
-cblock_alloc_cblocks(number)
- int number;
+cblock_alloc_cblocks(int number)
{
int i;
struct cblock *cbp;
@@ -139,25 +158,23 @@
"clist_alloc_cblocks: M_NOWAIT malloc failed, trying M_WAITOK\n");
cbp = malloc(sizeof *cbp, M_TTYS, M_WAITOK);
}
+ KKASSERT(((intptr_t)cbp & CROUND) == 0);
/*
* Freed cblocks have zero quotes and garbage elsewhere.
* Set the may-have-quote bit to force zeroing the quotes.
*/
setbit(cbp->c_quote, CBQSIZE * NBBY - 1);
+ cbp->c_head.ch_magic = CLIST_MAGIC_USED;
cblock_free(cbp);
}
ctotcount += number;
}
/*
- * Set the cblock allocation policy for a a clist.
- * Must be called in process context at spltty().
+ * Set the cblock allocation policy for a clist.
*/
void
-clist_alloc_cblocks(clistp, ccmax, ccreserved)
- struct clist *clistp;
- int ccmax;
- int ccreserved;
+clist_alloc_cblocks(struct clist *clistp, int ccmax, int ccreserved)
{
int dcbr;
@@ -169,25 +186,31 @@
if (ccreserved != 0)
ccreserved += CBSIZE - 1;
+ crit_enter();
clistp->c_cbmax = roundup(ccmax, CBSIZE) / CBSIZE;
dcbr = roundup(ccreserved, CBSIZE) / CBSIZE - clistp->c_cbreserved;
- if (dcbr >= 0)
- cblock_alloc_cblocks(dcbr);
- else {
+ if (dcbr >= 0) {
+ clistp->c_cbreserved += dcbr; /* atomic w/c_cbmax */
+ cblock_alloc_cblocks(dcbr); /* may block */
+ } else {
+ KKASSERT(clistp->c_cbcount <= clistp->c_cbreserved);
if (clistp->c_cbreserved + dcbr < clistp->c_cbcount)
dcbr = clistp->c_cbcount - clistp->c_cbreserved;
- cblock_free_cblocks(-dcbr);
+ clistp->c_cbreserved += dcbr; /* atomic w/c_cbmax */
+ cblock_free_cblocks(-dcbr); /* may block */
}
- clistp->c_cbreserved += dcbr;
+ KKASSERT(clistp->c_cbreserved >= 0);
+ crit_exit();
}
/*
* Free some cblocks from the cfreelist queue back to the
* system malloc pool.
+ *
+ * Must be called from within a critical section. May block.
*/
static void
-cblock_free_cblocks(number)
- int number;
+cblock_free_cblocks(int number)
{
int i;
@@ -198,34 +221,34 @@
/*
* Free the cblocks reserved for a clist.
- * Must be called at spltty().
*/
void
-clist_free_cblocks(clistp)
- struct clist *clistp;
+clist_free_cblocks(struct clist *clistp)
{
+ int cbreserved;
+
+ crit_enter();
if (clistp->c_cbcount != 0)
panic("freeing active clist cblocks");
- cblock_free_cblocks(clistp->c_cbreserved);
+ cbreserved = clistp->c_cbreserved;
clistp->c_cbmax = 0;
clistp->c_cbreserved = 0;
+ cblock_free_cblocks(cbreserved); /* may block */
+ crit_exit();
}
/*
* Get a character from the head of a clist.
*/
int
-getc(clistp)
- struct clist *clistp;
+getc(struct clist *clistp)
{
int chr = -1;
- int s;
struct cblock *cblockp;
- s = spltty();
-
- /* If there are characters in the list, get one */
+ crit_enter();
if (clistp->c_cc) {
+ KKASSERT(((intptr_t)clistp->c_cf & CROUND) != 0);
cblockp = (struct cblock *)((intptr_t)clistp->c_cf & ~CROUND);
chr = (u_char)*clistp->c_cf;
@@ -247,9 +270,11 @@
* last pointers to NULL. In either case, free the
* current cblock.
*/
- if ((clistp->c_cf >= (char *)(cblockp+1)) || (clistp->c_cc == 0)) {
+ KKASSERT(clistp->c_cf <= (char *)(cblockp + 1));
+ if ((clistp->c_cf == (char *)(cblockp + 1)) ||
+ (clistp->c_cc == 0)) {
if (clistp->c_cc > 0) {
- clistp->c_cf = cblockp->c_next->c_info;
+ clistp->c_cf = cblockp->c_head.ch_next->c_info;
} else {
clistp->c_cf = clistp->c_cl = NULL;
}
@@ -258,8 +283,7 @@
++cslushcount;
}
}
-
- splx(s);
+ crit_exit();
return (chr);
}
@@ -269,20 +293,16 @@
* actually copied.
*/
int
-q_to_b(clistp, dest, amount)
- struct clist *clistp;
- char *dest;
- int amount;
+q_to_b(struct clist *clistp, char *dest, int amount)
{
struct cblock *cblockp;
struct cblock *cblockn;
char *dest_orig = dest;
int numc;
- int s;
-
- s = spltty();
+ crit_enter();
while (clistp && amount && (clistp->c_cc > 0)) {
+ KKASSERT(((intptr_t)clistp->c_cf & CROUND) != 0);
cblockp = (struct cblock *)((intptr_t)clistp->c_cf & ~CROUND);
cblockn = cblockp + 1; /* pointer arithmetic! */
numc = min(amount, (char *)cblockn - clistp->c_cf);
@@ -298,9 +318,11 @@
* and last pointer to NULL. In either case, free the
* current cblock.
*/
- if ((clistp->c_cf >= (char *)cblockn) || (clistp->c_cc == 0)) {
+ KKASSERT(clistp->c_cf <= (char *)cblockn);
+ if ((clistp->c_cf == (char *)cblockn) || (clistp->c_cc == 0)) {
if (clistp->c_cc > 0) {
- clistp->c_cf = cblockp->c_next->c_info;
+ KKASSERT(cblockp->c_head.ch_next != NULL);
+ clistp->c_cf = cblockp->c_head.ch_next->c_info;
} else {
clistp->c_cf = clistp->c_cl = NULL;
}
@@ -309,8 +331,7 @@
++cslushcount;
}
}
-
- splx(s);
+ crit_exit();
return (dest - dest_orig);
}
@@ -318,18 +339,15 @@
* Flush 'amount' of chars, beginning at head of clist 'clistp'.
*/
void
-ndflush(clistp, amount)
- struct clist *clistp;
- int amount;
+ndflush(struct clist *clistp, int amount)
{
struct cblock *cblockp;
struct cblock *cblockn;
int numc;
- int s;
-
- s = spltty();
+ crit_enter();
while (amount && (clistp->c_cc > 0)) {
+ KKASSERT(((intptr_t)clistp->c_cf & CROUND) != 0);
cblockp = (struct cblock *)((intptr_t)clistp->c_cf & ~CROUND);
cblockn = cblockp + 1; /* pointer arithmetic! */
numc = min(amount, (char *)cblockn - clistp->c_cf);
@@ -343,9 +361,11 @@
* and last pointer to NULL. In either case, free the
* current cblock.
*/
- if ((clistp->c_cf >= (char *)cblockn) || (clistp->c_cc == 0)) {
+ KKASSERT(clistp->c_cf <= (char *)cblockn);
+ if (clistp->c_cf == (char *)cblockn || clistp->c_cc == 0) {
if (clistp->c_cc > 0) {
- clistp->c_cf = cblockp->c_next->c_info;
+ KKASSERT(cblockp->c_head.ch_next != NULL);
+ clistp->c_cf = cblockp->c_head.ch_next->c_info;
} else {
clistp->c_cf = clistp->c_cl = NULL;
}
@@ -354,8 +374,7 @@
++cslushcount;
}
}
-
- splx(s);
+ crit_exit();
}
/*
@@ -363,18 +382,20 @@
* more clists, or 0 for success.
*/
int
-putc(chr, clistp)
- int chr;
- struct clist *clistp;
+putc(int chr, struct clist *clistp)
{
struct cblock *cblockp;
- int s;
- s = spltty();
+ crit_enter();
+ /*
+ * Note: this section may point c_cl at the base of a cblock. This
+ * is a temporary violation of the requirements for c_cl, we
+ * increment it before returning.
+ */
if (clistp->c_cl == NULL) {
if (clistp->c_cbreserved < 1) {
- splx(s);
+ crit_exit();
printf("putc to a clist with no reserved cblocks\n");
return (-1); /* nothing done */
}
@@ -390,14 +411,14 @@
if (clistp->c_cbcount >= clistp->c_cbreserved) {
if (clistp->c_cbcount >= clistp->c_cbmax
|| cslushcount <= 0) {
- splx(s);
+ crit_exit();
return (-1);
}
--cslushcount;
}
cblockp = cblock_alloc();
clistp->c_cbcount++;
- prev->c_next = cblockp;
+ prev->c_head.ch_next = cblockp;
clistp->c_cl = cblockp->c_info;
}
}
@@ -412,13 +433,14 @@
* may be quoted.
*/
setbit(cblockp->c_quote, CBQSIZE * NBBY - 1);
- } else
+ } else {
clrbit(cblockp->c_quote, clistp->c_cl - (char *)cblockp->c_info);
+ }
*clistp->c_cl++ = chr;
clistp->c_cc++;
- splx(s);
+ crit_exit();
return (0);
}
@@ -427,16 +449,12 @@
* number of characters not copied.
*/
int
-b_to_q(src, amount, clistp)
- char *src;
- int amount;
- struct clist *clistp;
+b_to_q(char *src, int amount, struct clist *clistp)
{
struct cblock *cblockp;
char *firstbyte, *lastbyte;
u_char startmask, endmask;
int startbit, endbit, num_between, numc;
- int s;
/*
* Avoid allocating an initial cblock and then not using it.
@@ -445,15 +463,16 @@
if (amount <= 0)
return (amount);
- s = spltty();
+ crit_enter();
/*
- * If there are no cblocks assigned to this clist yet,
- * then get one.
+ * Note: this section may point c_cl at the base of a cblock. This
+ * is a temporary violation of the requirements for c_cl. Since
+ * amount is non-zero we will not return with it in that state.
*/
if (clistp->c_cl == NULL) {
if (clistp->c_cbreserved < 1) {
- splx(s);
+ crit_exit();
printf("b_to_q to a clist with no reserved cblocks.\n");
return (amount); /* nothing done */
}
@@ -462,6 +481,10 @@
clistp->c_cf = clistp->c_cl = cblockp->c_info;
clistp->c_cc = 0;
} else {
+ /*
+ * c_cl may legally point past the end of the block, which
+ * falls through to the 'get another cblock' code below.
+ */
cblockp = (struct cblock *)((intptr_t)clistp->c_cl & ~CROUND);
}
@@ -475,14 +498,14 @@
if (clistp->c_cbcount >= clistp->c_cbreserved) {
if (clistp->c_cbcount >= clistp->c_cbmax
|| cslushcount <= 0) {
- splx(s);
+ crit_exit();
return (amount);
}
--cslushcount;
}
cblockp = cblock_alloc();
clistp->c_cbcount++;
- prev->c_next = cblockp;
+ prev->c_head.ch_next = cblockp;
clistp->c_cl = cblockp->c_info;
}
@@ -540,11 +563,9 @@
* cblock) we prepare for the assignment of 'prev'
* above.
*/
- cblockp += 1;
-
+ ++cblockp;
}
-
- splx(s);
+ crit_exit();
return (amount);
}
@@ -552,12 +573,12 @@
* Get the next character in the clist. Store it at dst. Don't
* advance any clist pointers, but return a pointer to the next
* character position.
+ *
+ * Must be called at spltty(). This routine may not run in a critical
+ * section and so may not call the cblock allocator/deallocator.
*/
char *
-nextc(clistp, cp, dst)
- struct clist *clistp;
- char *cp;
- int *dst;
+nextc(struct clist *clistp, char *cp, int *dst)
{
struct cblock *cblockp;
@@ -572,7 +593,7 @@
* cblock, advance to the next cblock.
*/
if (((intptr_t)cp & CROUND) == 0)
- cp = ((struct cblock *)cp - 1)->c_next->c_info;
+ cp = ((struct cblock *)cp - 1)->c_head.ch_next->c_info;
cblockp = (struct cblock *)((intptr_t)cp & ~CROUND);
/*
@@ -591,17 +612,20 @@
* "Unput" a character from a clist.
*/
int
-unputc(clistp)
- struct clist *clistp;
+unputc(struct clist *clistp)
{
struct cblock *cblockp = 0, *cbp = 0;
- int s;
int chr = -1;
-
- s = spltty();
+ crit_enter();
if (clistp->c_cc) {
+ /*
+ * note that clistp->c_cl will never point at the base
+ * of a cblock (cblock->c_info) (see assert this later on),
+ * but it may point past the end of one. We temporarily
+ * violate this in the decrement below but then we fix it up.
+ */
--clistp->c_cc;
--clistp->c_cl;
@@ -619,30 +643,39 @@
* If all of the characters have been unput in this
* one.
+ *
+ * if c_cc is 0 clistp->c_cl may end up pointing at
+ * cblockp->c_info, which is illegal, but the case will be
+ * taken care of near the end of the routine. Otherwise
+ * there *MUST* be another cblock, find it.
*/
- if (clistp->c_cc && (clistp->c_cl <= (char *)cblockp->c_info)) {
+ KKASSERT(clistp->c_cl >= (char *)cblockp->c_info);
+ if (clistp->c_cc && (clistp->c_cl == (char *)cblockp->c_info)) {
cbp = (struct cblock *)((intptr_t)clistp->c_cf & ~CROUND);
- while (cbp->c_next != cblockp)
- cbp = cbp->c_next;
+ while (cbp->c_head.ch_next != cblockp)
+ cbp = cbp->c_head.ch_next;
+ cbp->c_head.ch_next = NULL;
/*
* When the previous cblock is at the end, the 'last'
* pointer always points (invalidly) one past.
*/
- clistp->c_cl = (char *)(cbp+1);
+ clistp->c_cl = (char *)(cbp + 1);
cblock_free(cblockp);
if (--clistp->c_cbcount >= clistp->c_cbreserved)
++cslushcount;
- cbp->c_next = NULL;
}
}
/*
* If there are no more characters on the list, then
- * free the last cblock.
+ * free the last cblock. It should not be possible for c->cl
+ * to be pointing past the end of a block due to our decrement
+ * of it way above.
*/
- if ((clistp->c_cc == 0) && clistp->c_cl) {
+ if (clistp->c_cc == 0 && clistp->c_cl) {
+ KKASSERT(((intptr_t)clistp->c_cl & CROUND) != 0);
cblockp = (struct cblock *)((intptr_t)clistp->c_cl & ~CROUND);
cblock_free(cblockp);
if (--clistp->c_cbcount >= clistp->c_cbreserved)
@@ -650,7 +683,7 @@
clistp->c_cf = clistp->c_cl = NULL;
}
- splx(s);
+ crit_exit();
return (chr);
}
@@ -659,12 +692,11 @@
* preserving quote bits.
*/
void
-catq(src_clistp, dest_clistp)
- struct clist *src_clistp, *dest_clistp;
+catq(struct clist *src_clistp, struct clist *dest_clistp)
{
- int chr, s;
+ int chr;
- s = spltty();
+ crit_enter();
/*
* If the destination clist is empty (has no cblocks atttached),
* and there are no possible complications with the resource counters,
@@ -682,11 +714,10 @@
dest_clistp->c_cbcount = src_clistp->c_cbcount;
src_clistp->c_cbcount = 0;
- splx(s);
+ crit_exit();
return;
}
-
- splx(s);
+ crit_exit();
/*
* XXX This should probably be optimized to more than one
Index: sys/clist.h
===================================================================
RCS file: /cvs/src/sys/sys/clist.h,v
retrieving revision 1.2
diff -u -r1.2 clist.h
--- sys/clist.h 17 Jun 2003 04:28:58 -0000 1.2
+++ sys/clist.h 5 Oct 2004 20:32:08 -0000
@@ -38,8 +38,22 @@
#ifndef _SYS_CLIST_H_
#define _SYS_CLIST_H_
+#define CBLOCK 128 /* Clist block size, must be a power of 2. */
+#define CBQSIZE (CBLOCK/NBBY) /* Quote bytes/cblock - can do better. */
+ /* Data chars/clist. */
+#define CBSIZE (CBLOCK - sizeof(struct cblockhead) - CBQSIZE)
+#define CROUND (CBLOCK - 1) /* Clist rounding. */
+
+struct cblockhead {
+ struct cblock *ch_next;
+ int ch_magic;
+};
+
+#define CLIST_MAGIC_FREE 0x434c0102
+#define CLIST_MAGIC_USED 0x434c8182
+
struct cblock {
- struct cblock *c_next; /* next cblock in queue */
+ struct cblockhead c_head; /* header */
unsigned char c_quote[CBQSIZE]; /* quoted characters */
unsigned char c_info[CBSIZE]; /* characters */
};
Index: sys/param.h
===================================================================
RCS file: /cvs/src/sys/sys/param.h,v
retrieving revision 1.17
diff -u -r1.17 param.h
--- sys/param.h 23 Aug 2004 16:03:44 -0000 1.17
+++ sys/param.h 5 Oct 2004 21:33:54 -0000
@@ -146,21 +146,6 @@
#endif
/*
- * Clustering of hardware pages on machines with ridiculously small
- * page sizes is done here. The paging subsystem deals with units of
- * CLSIZE pte's describing PAGE_SIZE (from machine/machparam.h) pages each.
- */
-#if 0
-#define CLBYTES (CLSIZE*PAGE_SIZE)
-#endif
-
-#define CBLOCK 128 /* Clist block size, must be a power of 2. */
-#define CBQSIZE (CBLOCK/NBBY) /* Quote bytes/cblock - can do better. */
- /* Data chars/clist. */
-#define CBSIZE (CBLOCK - sizeof(struct cblock *) - CBQSIZE)
-#define CROUND (CBLOCK - 1) /* Clist rounding. */
-
-/*
* File system parameters and macros.
*
* MAXBSIZE - Filesystems are made out of blocks of at most MAXBSIZE bytes