PERFORCE change 131191 for review

[John Baldwin]

On Tuesday 18 December 2007 08:37:08 pm Hans Petter Selasky wrote:
> http://perforce.freebsd.org/chv.cgi?CH=131191
> 
> Change 131191 by hselasky at hselasky_laptop001 on 2007/12/19 01:36:11
> 
> 	
> 	Bugfix:
> 	It looks like you need to call "intr_event_destroy" and
> 	not just "swi_remove". This was not well documented.
> 	Else you get a memory leak. Looked through the sources
> 	and I think others have made the same mistake aswell.

That's because all other SWI's are owned by the system.  You probably should 
just create plain old kernel threads via kthread_*() or kproc_*() instead of 
using the swi stuff.  swi_add/swi_remove are for adding and removing 
handlers, not for managing the backing threads.  swi_add() just happens to 
create the thread if it doesn't already exist.

> Affected files ...
> 
> .. //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#76 edit
> 
> Differences ...
> 
> ==== //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#76 (text+ko) ====
> 
> @@ -835,9 +835,11 @@
>  			LIST_INIT(&(info->done_head));
>  
>  			/* create our interrupt thread */
> -			if (swi_add(NULL, "usbcb", &usbd_callback_intr_td,
> -			    info, SWI_CAMBIO, INTR_MPSAFE, &(info->done_cookie))) {
> +			if (swi_add(&(info->done_event), "usbcb",
> +			    &usbd_callback_intr_td, info, SWI_CAMBIO,
> +			    INTR_MPSAFE, &(info->done_cookie))) {
>  				info->done_cookie = NULL;
> +				info->done_event = NULL;
>  				parm.err = USBD_NO_INTR_THREAD;
>  				goto done;
>  			}
> @@ -1073,6 +1075,7 @@
>  	/* teardown the interrupt thread, if any */
>  	if (info->done_cookie) {
>  		swi_remove(info->done_cookie);
> +		intr_event_destroy(info->done_event);
>  	}
>  	/*
>  	 * free the "memory_base" last,
> @@ -2434,7 +2437,7 @@
>  		type = (xfer->pipe->edesc->bmAttributes & UE_XFERTYPE);
>  		if (type != UE_ISOCHRONOUS) {
>  			xfer->pipe->is_stalled = 1;
> -			(xfer->pipe->methods->set_stall) (
> +			(xfer->udev->bus->methods->set_stall) (
>  			    xfer->udev, NULL, xfer->pipe);
>  			goto done;
>  		}
> @@ -2823,11 +2826,11 @@
>  		 * complete the USB transfer like in case of a timeout
>  		 * setting the error code "USBD_STALLED".
>  		 */
> -		(pipe->methods->set_stall) (udev, xfer, pipe);
> +		(udev->bus->methods->set_stall) (udev, xfer, pipe);
>  	} else {
>  		pipe->toggle_next = 0;	/* reset data toggle */
>  
> -		(pipe->methods->clear_stall) (udev, pipe);
> +		(udev->bus->methods->clear_stall) (udev, pipe);
>  
>  		/* start up the first transfer, if any */
>  		if (xfer) {
> 



-- 
John Baldwin