Date: Wed, 19 Dec 2007 14:33:13 -0500 From: John Baldwin <jhb@freebsd.org> To: Hans Petter Selasky <hselasky@freebsd.org> Cc: Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 131191 for review Message-ID: <200712191433.13837.jhb@freebsd.org> In-Reply-To: <200712190137.lBJ1b8sJ006586@repoman.freebsd.org> References: <200712190137.lBJ1b8sJ006586@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200712191433.13837.jhb>