NetBSD Problem Report #39356
From www@NetBSD.org Fri Aug 15 09:05:45 2008
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
by narn.NetBSD.org (Postfix) with ESMTP id DBFCD63B853
for <gnats-bugs@gnats.netbsd.org>; Fri, 15 Aug 2008 09:05:44 +0000 (UTC)
Message-Id: <20080815090544.8FA0663B11D@narn.NetBSD.org>
Date: Fri, 15 Aug 2008 09:05:44 +0000 (UTC)
From: andreas.jacobs@lancom.de
Reply-To: andreas.jacobs@lancom.de
To: gnats-bugs@NetBSD.org
Subject: DMA memory leak in Ohci Driver (USB)
X-Send-Pr-Version: www-1.0
>Number: 39356
>Category: kern
>Synopsis: DMA memory leak in Ohci Driver (USB)
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: mrg
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Fri Aug 15 09:10:00 +0000 2008
>Last-Modified: Tue Sep 18 02:33:34 +0000 2018
>Originator: Andreas Jacobs
>Release: CVS Head from 2008-08-14
>Organization:
LANCOM Systems GmbH
>Environment:
I do not use the NetBSD operating system itself but only its USB-Stack ported to another OS.
>Description:
If you attach an Ohci and later detach it again, it does not free all the memory it has allocated (note that this is another leak than in PR 39322):
ohci_alloc_sed() in ohci.c allocates new ohci_soft_ed_t objects, taking them from a linked list rooted at sc_freeeds. If that list is empty, that function allocates a new chunk of memory in line 420 to make further OHCI_SED_CHUCK ohci_soft_ed_t's available. This allocated memory is never released although in my opinion it should be released in ohci_detach at latest.
An equivalent problem exists with the function ohci_alloc_std which allocates memory in line 459.
I've got a patch for that (see below) and I'm curious what you think about it.
Thank you in advance
Andreas Jacobs
>How-To-Repeat:
Attach and detach an Ohci over and over again. This is hard to do, if that chip is part of your PC, but is possible, if you use a UMTS/HSPA modem attached via PCMCIA.
>Fix:
The ohci_soft_ed_t and ohci_soft_td_t structures may be in use until ohci_detach runs, but surely not longer, so in my opinion the blocks that ohci_attach_sed() and ohci_attach_std() allocate should be released exactly in ohci_detach().
Unfortunately, ohci_attach_sed() and ohci_attach_std() do not save pointers to these blocks yet, so there is no easy way for ohci_detach() to release these. Therefore my patch proposal becomes a little bit complicated:
- add a SIMPLEQ to struct ohci_softc to hold the allocated usb_dma structures
- add a SIMPLEQ_ENTRY to usb_dma_t, s.t. usb_dma_t's can be linked
- initialize that queue in ohci_init
- in ohci_attach_sed() and ohci_attach_std() add the allocated usb_dma_t's to the queue; note that the usb_dma_t's must be allocated on the heap instead of the stack for that
- finally in ohci_detach(): iterate through that queue and deallocate all the blocks
Here is my patch as a universal diff:
Index: ohci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ohci.c,v
retrieving revision 1.196
diff -U3 -r1.196 ohci.c
--- ohci.c 13 Aug 2008 09:43:56 -0000 1.196
+++ ohci.c 15 Aug 2008 08:26:37 -0000
@@ -385,6 +385,7 @@
ohci_detach(struct ohci_softc *sc, int flags)
{
int rv = 0;
+ usb_dma_t *dma;
usbd_xfer_handle xfer;
if (sc->sc_child != NULL)
@@ -397,6 +398,11 @@
usb_delay_ms(&sc->sc_bus, 300); /* XXX let stray task complete */
+ while((dma = SIMPLEQ_FIRST(&sc->sc_allocated_dmas)) != NULL) {
+ SIMPLEQ_REMOVE_HEAD(&sc->sc_allocated_dmas, next);
+ usb_freemem(&sc->sc_bus, dma);
+ free(dma, M_USB);
+ }
usb_freemem(&sc->sc_bus, &sc->sc_hccadma);
while((xfer = SIMPLEQ_FIRST(&sc->sc_free_xfers)) != NULL) {
SIMPLEQ_REMOVE_HEAD(&sc->sc_free_xfers, next);
@@ -413,18 +419,24 @@
ohci_soft_ed_t *sed;
usbd_status err;
int i, offs;
- usb_dma_t dma;
+ usb_dma_t *dma;
if (sc->sc_freeeds == NULL) {
DPRINTFN(2, ("ohci_alloc_sed: allocating chunk\n"));
+ dma = malloc(sizeof *dma, M_USB, M_NOWAIT);
+ if(dma == NULL)
+ return (0);
err = usb_allocmem(&sc->sc_bus, OHCI_SED_SIZE * OHCI_SED_CHUNK,
- OHCI_ED_ALIGN, &dma);
- if (err)
+ OHCI_ED_ALIGN, dma);
+ if (err) {
+ free(dma, M_USB);
return (0);
+ }
+ SIMPLEQ_INSERT_HEAD(&sc->sc_allocated_dmas, dma, next);
for(i = 0; i < OHCI_SED_CHUNK; i++) {
offs = i * OHCI_SED_SIZE;
- sed = KERNADDR(&dma, offs);
- sed->physaddr = DMAADDR(&dma, offs);
+ sed = KERNADDR(dma, offs);
+ sed->physaddr = DMAADDR(dma, offs);
sed->dma = dma;
sed->offs = offs;
sed->next = sc->sc_freeeds;
@@ -451,20 +463,26 @@
ohci_soft_td_t *std;
usbd_status err;
int i, offs;
- usb_dma_t dma;
+ usb_dma_t *dma;
int s;
if (sc->sc_freetds == NULL) {
DPRINTFN(2, ("ohci_alloc_std: allocating chunk\n"));
+ dma = malloc(sizeof *dma, M_USB, M_NOWAIT);
+ if(dma == NULL)
+ return (0);
err = usb_allocmem(&sc->sc_bus, OHCI_STD_SIZE * OHCI_STD_CHUNK,
- OHCI_TD_ALIGN, &dma);
- if (err)
+ OHCI_TD_ALIGN, dma);
+ if (err) {
+ free(dma, M_USB);
return (NULL);
+ }
s = splusb();
+ SIMPLEQ_INSERT_HEAD(&sc->sc_allocated_dmas, dma, next);
for(i = 0; i < OHCI_STD_CHUNK; i++) {
offs = i * OHCI_STD_SIZE;
- std = KERNADDR(&dma, offs);
- std->physaddr = DMAADDR(&dma, offs);
+ std = KERNADDR(dma, offs);
+ std->physaddr = DMAADDR(dma, offs);
std->dma = dma;
std->offs = offs;
std->nexttd = sc->sc_freetds;
@@ -708,6 +726,7 @@
for (i = 0; i < OHCI_HASH_SIZE; i++)
LIST_INIT(&sc->sc_hash_itds[i]);
+ SIMPLEQ_INIT(&sc->sc_allocated_dmas);
SIMPLEQ_INIT(&sc->sc_free_xfers);
#ifdef __NetBSD__
Index: ohcivar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ohcivar.h,v
retrieving revision 1.45
diff -U3 -r1.45 ohcivar.h
--- ohcivar.h 28 Jun 2008 17:42:53 -0000 1.45
+++ ohcivar.h 15 Aug 2008 08:26:37 -0000
@@ -117,6 +117,7 @@
ohci_soft_td_t *sc_freetds;
ohci_soft_itd_t *sc_freeitds;
+ SIMPLEQ_HEAD(, usb_dma) sc_allocated_dmas;
SIMPLEQ_HEAD(, usbd_xfer) sc_free_xfers; /* free xfers */
usbd_xfer_handle sc_intrxfer;
Index: usb_port.h
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usb_port.h,v
retrieving revision 1.85
diff -U3 -r1.85 usb_port.h
--- usb_port.h 28 Jun 2008 09:06:20 -0000 1.85
+++ usb_port.h 15 Aug 2008 08:26:37 -0000
@@ -102,9 +102,10 @@
#define DECLARE_USB_DMA_T \
struct usb_dma_block; \
- typedef struct { \
+ typedef struct usb_dma { \
struct usb_dma_block *block; \
u_int offs; \
+ SIMPLEQ_ENTRY(usb_dma) next; \
} usb_dma_t
typedef struct callout usb_callout_t;
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: kern-bug-people->mrg
Responsible-Changed-By: mrg@NetBSD.org
Responsible-Changed-When: Tue, 18 Sep 2018 02:33:34 +0000
Responsible-Changed-Why:
i'll take a look. we can easily force detach/reattach cycle with drvctl today.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.43 2018/01/16 07:36:43 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.