cvs commit: src/sys/dev/usb usb.c

[Ian Dowse]

In message <20050327173421.6F6C616A4CE at hub.freebsd.org>, wpaul at FreeBSD.org writ
es:
>uhub2: at uhub1 port 1 (addr 2) disconnected
>ukbd0: at uhub2 port 1 (addr 3) disconnected
>ukbd0: detached
>
>Fatal trap 12: page fault while in kernel mode
>cpuid = 0; apic id = 00
>fault virtual address   = 0xc
>fault code              = supervisor read, page not present

>It looks as if the ukbd_timeout() routine is not always disabled when
>the ukbd driver is detached. I suspect there is a race condition somewhere
>that only manifests on SMP, but I haven't been able to track it down.
>This is with the 6.0 SNAP002 CD from March 18th. It also happened with
>the SNAP001 CD.

The ukbd driver uses the timeout()/untimeout() API, which makes it
pretty much impossible to cancel timeouts reliably. That may not
be the real problem here, but could be related - does the patch
below make any difference?

Ian

Index: ukbd.c
===================================================================
RCS file: /dump/FreeBSD-CVS/src/sys/dev/usb/ukbd.c,v
retrieving revision 1.51
diff -u -r1.51 ukbd.c
--- ukbd.c	6 Jan 2005 01:43:28 -0000	1.51
+++ ukbd.c	27 Mar 2005 19:05:16 -0000
@@ -344,7 +344,7 @@
 #define	INTRENABLED	(1 << 0)
 #define	DISCONNECTED	(1 << 1)
 
-	struct callout_handle ks_timeout_handle;
+	usb_callout_t ks_timeout_handle;
 
 	int		ks_mode;	/* input mode (K_XLATE,K_RAW,K_CODE) */
 	int		ks_flags;	/* flags */
@@ -405,7 +405,7 @@
 /* local functions */
 Static int		ukbd_enable_intr(keyboard_t *kbd, int on,
 					 usbd_intr_t *func);
-Static timeout_t	ukbd_timeout;
+Static void		ukbd_timeout(void *arg);
 
 Static int		ukbd_getc(ukbd_state_t *state);
 Static int		probe_keyboard(struct usb_attach_arg *uaa, int flags);
@@ -573,7 +573,7 @@
 		state->ks_iface = uaa->iface;
 		state->ks_uaa = uaa;
 		state->ks_ifstate = 0;
-		callout_handle_init(&state->ks_timeout_handle);
+		usb_callout_init(state->ks_timeout_handle);
 		/*
 		 * FIXME: set the initial value for lock keys in ks_state
 		 * according to the BIOS data?
@@ -643,8 +643,7 @@
 	state = (ukbd_state_t *)kbd->kb_data;
 	DPRINTF(("ukbd_term: ks_ifstate=0x%x\n", state->ks_ifstate));
 
-	untimeout(ukbd_timeout, (void *)kbd, state->ks_timeout_handle);
-	callout_handle_init(&state->ks_timeout_handle);
+	usb_uncallout(state->ks_timeout_handle, ukbd_timeout, kbd);
 
 	if (state->ks_ifstate & INTRENABLED)
 		ukbd_enable_intr(kbd, FALSE, NULL);
@@ -685,7 +684,7 @@
 	state = (ukbd_state_t *)kbd->kb_data;
 	s = splusb();
 	(*kbdsw[kbd->kb_index]->intr)(kbd, (void *)USBD_NORMAL_COMPLETION);
-	state->ks_timeout_handle = timeout(ukbd_timeout, arg, hz/40);
+	usb_callout(state->ks_timeout_handle, hz / 40, ukbd_timeout, arg);
 	splx(s);
 }