i had a fault in usb/kb this evening, and have a proposed fix.
it also seemed like a good time to address the goto soup in repeatproc.
by moving the timer into its own proc, it was possible to turn
repeatproc into a case statement. the downside is there's
a tiny bit of busy work going on between the timer and repeat proc.
minooka; apatch/diffemail
email
quans...@quanstro.net
readme
as there was no interlock for the repeat process shutdown, it was
possible
to free the channel before the repeat process had a chance to shut down,
so if a repeating keyboard were unplugged, a crash could result. this
was
seen as in this
bootes 152 0:00 0:00 4096K Broken usbd [kbd repeat]
with this console output:
chula# ````````````````````````````kb: /dev/usb/ep8.1: read:
crc/timeout error
kb: exiting
`152: usbd: bootes: fault addr=0xcafebac2 kpc=0x20854e
usbd 152: suicide: sys: trap: fault read addr=0xcafebac2 pc=0x20854e
removed
files
/sys/src/cmd/usb/kb/kb.c kb.c
/sys/src/cmd/usb/kb/kb.c kb.c
kb.c.orig:19,32 - kb.c:19,50
enum
{
- Awakemsg= 0xdeaddead,
- Diemsg = 0xbeefbeef,
+ Stoprpt = -2,
+ Tick = -3,
+ Exiting = -4,
+
+ Msec = 1000*1000, /* msec per ns */
+
Dwcidle = 8,
};
typedef struct KDev KDev;
+ typedef struct Kbd Kbd;
+ typedef struct Mouse Mouse;
typedef struct Kin Kin;
+ struct Kbd
+ {
+ Channel* repeatc;
+ Channel* exitc;
+ long nproc;
+ };
+
+ struct Mouse
+ {
+ int accel; /* only for mouse */
+ };
+
struct KDev
{
Dev* dev; /* usb device*/
kb.c.orig:33,42 - kb.c:51,60
Dev* ep; /* endpoint to get events */
Kin* in; /* used to send events to kernel */
int idle; /* min time between reports (× 4ms) */
- Channel*repeatc; /* only for keyboard */
- int accel; /* only for mouse */
int bootp; /* has associated keyboard */
int debug;
+ Kbd;
+ Mouse;
HidRepTempl templ;
int (*ptrvals)(KDev *kd, Chain *ch, int *px, int *py, int *pb);
};
kb.c.orig:212,219 - kb.c:230,244
fprint(2, "kb: fatal: %s\n", sts);
else
fprint(2, "kb: exiting\n");
- if(kd->repeatc != nil)
- nbsendul(kd->repeatc, Diemsg);
+ if(kd->repeatc != nil){
+ chanclose(kd->repeatc);
+ for(; kd->nproc != 0; kd->nproc--)
+ recvul(kd->exitc);
+ chanfree(kd->repeatc);
+ chanfree(kd->exitc);
+ kd->repeatc = nil;
+ kd->exitc = nil;
+ }
dev = kd->dev;
kd->dev = nil;
if(kd->ep != nil)
kb.c.orig:392,398 - kb.c:417,423
static void
stoprepeat(KDev *f)
{
- sendul(f->repeatc, Awakemsg);
+ sendul(f->repeatc, Stoprpt);
}
static void
kb.c.orig:432,478 - kb.c:457,518
}
static void
+ repeattimerproc(void *a)
+ {
+ Channel *c;
+ KDev *f;
+
+ threadsetname("kbd reptimer");
+ f = a;
+ c = f->repeatc;
+
+ for(;;){
+ sleep(30);
+ if(sendul(c, Tick) == -1)
+ break;
+ }
+ sendul(f->exitc, Exiting);
+ threadexits("aborted");
+ }
+
+ static void
repeatproc(void* a)
{
KDev *f;
- Channel *repeatc;
- ulong l, t, i;
- uchar esc1, sc;
+ Channel *c;
+ int code;
+ uint l;
+ uvlong timeout;
threadsetname("kbd repeat");
- /*
- * too many jumps here.
- * Rewrite instead of debug, if needed.
- */
f = a;
- repeatc = f->repeatc;
- l = Awakemsg;
- Repeat:
- if(l == Diemsg)
- goto Abort;
- while(l == Awakemsg)
- l = recvul(repeatc);
- if(l == Diemsg)
- goto Abort;
- esc1 = l >> 8;
- sc = l;
- t = 500;
+ c = f->repeatc;
+
+ timeout = 0;
+ code = -1;
for(;;){
- for(i = 0; i < t; i += 5){
- if(l = nbrecvul(repeatc))
- goto Repeat;
- sleep(5);
+ switch(l = recvul(c)){
+ default:
+ code = l;
+ timeout = nsec() + 500*Msec;
+ break;
+ case ~0ul:
+ sendul(f->exitc, Exiting);
+ threadexits("aborted");
+ break;
+ case Stoprpt:
+ code = -1;
+ break;
+ case Tick:
+ if(code == -1 || nsec() < timeout)
+ continue;
+ putscan(f, (uchar)(code>>8), (uchar)code);
+ timeout = nsec() + 30*Msec;
+ break;
}
- putscan(f, esc1, sc);
- t = 30;
}
- Abort:
- chanfree(repeatc);
- threadexits("aborted");
-
}
-
#define hasesc1(sc) (((sc) >= 0x47) || ((sc) == 0x38))
static void
kb.c.orig:560,570 - kb.c:600,617
if(f->ep->maxpkt < 3 || f->ep->maxpkt > sizeof buf)
kbfatal(f, "weird maxpkt");
+ f->exitc = chancreate(sizeof(ulong), 0);
+ if(f->exitc == nil)
+ kbfatal(f, "chancreate failed");
f->repeatc = chancreate(sizeof(ulong), 0);
- if(f->repeatc == nil)
+ if(f->repeatc == nil){
+ chanfree(f->exitc);
kbfatal(f, "chancreate failed");
+ }
+ f->nproc = 2;
proccreate(repeatproc, f, Stack);
+ proccreate(repeattimerproc, f, Stack);
memset(lbuf, 0, sizeof lbuf);
dk = nerrs = 0;
for(;;){
