From mboxrd@z Thu Jan  1 00:00:00 1970
From: erik quanstrom <quanstro@quanstro.net>
Date: Sat, 21 Sep 2013 21:39:36 -0400
To: 9fans@9fans.net
Message-ID: <af2feafb08c779ce1518d1d44f76a2ec@brasstown.quanstro.net>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: [9fans] usb/kb fault ... and fix
Topicbox-Message-UUID: 80d16bfc-ead8-11e9-9d60-3106f5b1d025

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
	quanstro@quanstro.net
readme
	as there was no interlock for the repeat process shutdown, it was possib=
le
	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 w=
as
	seen as in this
	bootes          152    0:00   0:00     4096K Broken   usbd [kbd repeat]
	with this console output:
=09
	chula# ````````````````````````````kb: /dev/usb/ep8.1: read: crc/timeout=
 error
	kb: exiting
	`152: usbd: bootes: fault addr=3D0xcafebac2 kpc=3D0x20854e
	usbd 152: suicide: sys: trap: fault read addr=3D0xcafebac2 pc=3D0x20854e
=09
removed
=09
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
 =20
  enum
  {
- 	Awakemsg=3D 0xdeaddead,
- 	Diemsg	=3D 0xbeefbeef,
+ 	Stoprpt	=3D -2,
+ 	Tick	=3D -3,
+ 	Exiting	=3D -4,
+=20
+ 	Msec	=3D 1000*1000,		/* msec per ns */
+=20
  	Dwcidle	=3D 8,
  };
 =20
  typedef struct KDev KDev;
+ typedef struct Kbd Kbd;
+ typedef struct Mouse Mouse;
  typedef struct Kin Kin;
 =20
+ struct Kbd
+ {
+ 	Channel*	repeatc;
+ 	Channel*	exitc;
+ 	long		nproc;
+ };
+=20
+ struct Mouse
+ {
+ 	int	accel;		/* only for mouse */
+ };
+=20
  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 (=C3=97 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 !=3D nil)
- 		nbsendul(kd->repeatc, Diemsg);
+ 	if(kd->repeatc !=3D nil){
+ 		chanclose(kd->repeatc);
+ 		for(; kd->nproc !=3D 0; kd->nproc--)
+ 			recvul(kd->exitc);
+ 		chanfree(kd->repeatc);
+ 		chanfree(kd->exitc);
+ 		kd->repeatc =3D nil;
+ 		kd->exitc =3D nil;
+ 	}
  	dev =3D kd->dev;
  	kd->dev =3D nil;
  	if(kd->ep !=3D nil)
kb.c.orig:392,398 - kb.c:417,423
  static void
  stoprepeat(KDev *f)
  {
- 	sendul(f->repeatc, Awakemsg);
+ 	sendul(f->repeatc, Stoprpt);
  }
 =20
  static void
kb.c.orig:432,478 - kb.c:457,518
  }
 =20
  static void
+ repeattimerproc(void *a)
+ {
+ 	Channel *c;
+ 	KDev *f;
+=20
+ 	threadsetname("kbd reptimer");
+ 	f =3D a;
+ 	c =3D f->repeatc;
+=20
+ 	for(;;){
+ 		sleep(30);
+ 		if(sendul(c, Tick) =3D=3D -1)
+ 			break;
+ 	}
+ 	sendul(f->exitc, Exiting);
+ 	threadexits("aborted");
+ }
+=20
+ static void
  repeatproc(void* a)
  {
  	KDev *f;
- 	Channel *repeatc;
- 	ulong l, t, i;
- 	uchar esc1, sc;
+ 	Channel *c;
+ 	int code;
+ 	uint l;
+ 	uvlong timeout;
 =20
  	threadsetname("kbd repeat");
- 	/*
- 	 * too many jumps here.
- 	 * Rewrite instead of debug, if needed.
- 	 */
  	f =3D a;
- 	repeatc =3D f->repeatc;
- 	l =3D Awakemsg;
- Repeat:
- 	if(l =3D=3D Diemsg)
- 		goto Abort;
- 	while(l =3D=3D Awakemsg)
- 		l =3D recvul(repeatc);
- 	if(l =3D=3D Diemsg)
- 		goto Abort;
- 	esc1 =3D l >> 8;
- 	sc =3D l;
- 	t =3D 500;
+ 	c =3D f->repeatc;
+=20
+ 	timeout =3D 0;
+ 	code =3D -1;
  	for(;;){
- 		for(i =3D 0; i < t; i +=3D 5){
- 			if(l =3D nbrecvul(repeatc))
- 				goto Repeat;
- 			sleep(5);
+ 		switch(l =3D recvul(c)){
+ 		default:
+ 			code =3D l;
+ 			timeout =3D nsec() + 500*Msec;
+ 			break;
+ 		case ~0ul:
+ 			sendul(f->exitc, Exiting);
+ 			threadexits("aborted");
+ 			break;
+ 		case Stoprpt:
+ 			code =3D -1;
+ 			break;
+ 		case Tick:
+ 			if(code =3D=3D -1 || nsec() < timeout)
+ 				continue;
+ 			putscan(f, (uchar)(code>>8), (uchar)code);
+ 			timeout =3D nsec() + 30*Msec;
+ 			break;
  		}
- 		putscan(f, esc1, sc);
- 		t =3D 30;
  	}
- Abort:
- 	chanfree(repeatc);
- 	threadexits("aborted");
-=20
  }
 =20
-=20
  #define hasesc1(sc)	(((sc) >=3D 0x47) || ((sc) =3D=3D 0x38))
 =20
  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");
 =20
+ 	f->exitc =3D chancreate(sizeof(ulong), 0);
+ 	if(f->exitc =3D=3D nil)
+ 		kbfatal(f, "chancreate failed");
  	f->repeatc =3D chancreate(sizeof(ulong), 0);
- 	if(f->repeatc =3D=3D nil)
+ 	if(f->repeatc =3D=3D nil){
+ 		chanfree(f->exitc);
  		kbfatal(f, "chancreate failed");
+ 	}
 =20
+ 	f->nproc =3D 2;
  	proccreate(repeatproc, f, Stack);
+ 	proccreate(repeattimerproc, f, Stack);
  	memset(lbuf, 0, sizeof lbuf);
  	dk =3D nerrs =3D 0;
  	for(;;){