Date: Sun, 18 Feb 2007 16:20:18 +0100 From: Max Laier <max@love2party.net> To: Luigi Rizzo <rizzo@icir.org> Cc: Jeremie Le Hen <jeremie@le-hen.org>, freebsd-net@freebsd.org, "V.Chukharev" <chukharev@mail.ru>, cognet@freebsd.org Subject: Re: iwi leaks memory? Message-ID: <200702181620.26610.max@love2party.net> In-Reply-To: <20070216085132.A7944@xorpc.icir.org> References: <op.tns7i21z0g54sc@localhost> <200702161738.35142.max@love2party.net> <20070216085132.A7944@xorpc.icir.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart1505776.Wia5angaEs Content-Type: multipart/mixed; boundary="Boundary-01=_16G2FC0/L+z46iA" Content-Transfer-Encoding: 7bit Content-Disposition: inline --Boundary-01=_16G2FC0/L+z46iA Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On Friday 16 February 2007 17:51, Luigi Rizzo wrote: > On Fri, Feb 16, 2007 at 05:38:28PM +0100, Max Laier wrote: > ... > > > I'm under the impression that this is more a problem of increasing > > fragmentation until we can't get a big enough unfragmented chunk. I > > don't have any proof of this assumption yet. > > makes sense. > As a matter of fact i wonder whether it wouldn't be smarter > to allocate the dma-ble memory on the first request and > keep it around until the driver is unloaded. > > If i read the code in iwi_load_firmware() correctly, > the contiguous chunks cannot be longer than 8191 bytes, > so a single contiguous buffer is not mandatory. > I just don't know if we can write the firmware to the adapter > with multiple operations (lists of command blocks) or > it needs to be just a single list ? The linux driver uses one continuous buffer as well. I tried to hand in=20 separate buffers, but it failed to initialize the firmware. Attached is=20 a diff (for HEAD and RELENG_6) to allocate the DMA buffer once and keep=20 it around. As I said earlier: As long as the firmware is as (un)stable=20 as it is now, this might be the only sensible way. All in all it's not=20 that bad, as we "only" throw away 212966 bytes. Could you please confirm that this works for you? =2D-=20 /"\ Best regards, | mlaier@freebsd.org \ / Max Laier | ICQ #67774661 X http://pf4freebsd.love2party.net/ | mlaier@EFnet / \ ASCII Ribbon Campaign | Against HTML Mail and News --Boundary-01=_16G2FC0/L+z46iA Content-Type: text/x-diff; charset="iso-8859-1"; name="iwi.r6.diff" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="iwi.r6.diff" Index: if_iwi.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /usr/store/mlaier/fcvs/src/sys/dev/iwi/if_iwi.c,v retrieving revision 1.8.2.12 diff -u -r1.8.2.12 if_iwi.c =2D-- if_iwi.c 23 Jan 2007 22:17:48 -0000 1.8.2.12 +++ if_iwi.c 18 Feb 2007 15:05:01 -0000 @@ -118,6 +118,8 @@ }; =20 static void iwi_dma_map_addr(void *, bus_dma_segment_t *, int, int); +static int iwi_alloc_fw_cb(struct iwi_softc *); +static void iwi_free_fw_cb(struct iwi_softc *); static int iwi_alloc_cmd_ring(struct iwi_softc *, struct iwi_cmd_ring *, int); static void iwi_reset_cmd_ring(struct iwi_softc *, struct iwi_cmd_ring *); @@ -322,6 +324,12 @@ goto fail; } =20 + if (iwi_alloc_fw_cb(sc) !=3D 0) { + device_printf(dev, + "could not allocate firmware command block\n"); + goto fail; + } + /* * Allocate rings. */ @@ -496,6 +504,7 @@ } iwi_put_firmware(sc); =20 + iwi_free_fw_cb(sc); iwi_free_cmd_ring(sc, &sc->cmdq); iwi_free_tx_ring(sc, &sc->txq[0]); iwi_free_tx_ring(sc, &sc->txq[1]); @@ -536,6 +545,57 @@ } =20 static int +iwi_alloc_fw_cb(struct iwi_softc *sc) +{ + int error; + + sc->fw_dma_size =3D IWI_CB_MAXDATALEN * IWI_FW_CB_MAX; + + error =3D bus_dma_tag_create(bus_get_dma_tag(sc->sc_dev), 4, 0, + BUS_SPACE_MAXADDR_32BIT, BUS_SPACE_MAXADDR, NULL, NULL, + sc->fw_dma_size, 1, sc->fw_dma_size, 0, NULL, NULL, + &sc->fw_dmat); + if (error !=3D 0) { + device_printf(sc->sc_dev, + "could not create firmware DMA tag\n"); + goto fail; + } + + error =3D bus_dmamem_alloc(sc->fw_dmat, &sc->fw_virtaddr, 0, + &sc->fw_map); + if (error !=3D 0) { + device_printf(sc->sc_dev, + "could not allocate firmware DMA memory\n"); + goto fail; + } + + error =3D bus_dmamap_load(sc->fw_dmat, sc->fw_map, sc->fw_virtaddr, + sc->fw_dma_size, iwi_dma_map_addr, &sc->fw_physaddr, 0); + if (error !=3D 0) { + device_printf(sc->sc_dev, "could not load firmware DMA map\n"); + goto fail; + } + + return (0); +fail: iwi_free_fw_cb(sc); + return (error); +} + +static void +iwi_free_fw_cb(struct iwi_softc *sc) +{ + if (sc->fw_virtaddr !=3D NULL) { + bus_dmamap_sync(sc->fw_dmat, sc->fw_map, + BUS_DMASYNC_POSTWRITE); + bus_dmamap_unload(sc->fw_dmat, sc->fw_map); + bus_dmamem_free(sc->fw_dmat, sc->fw_virtaddr, sc->fw_map); + } + + if (sc->fw_dmat !=3D NULL) + bus_dma_tag_destroy(sc->fw_dmat); +} + +static int iwi_alloc_cmd_ring(struct iwi_softc *sc, struct iwi_cmd_ring *ring, int co= unt) { int error; @@ -2433,6 +2493,14 @@ uint32_t sentinel, ctl, src, dst, sum, len, mlen, tmp; int ntries, error; =20 + if (fw->size > sc->fw_dma_size) { + device_printf(sc->sc_dev, + "to less dma memory for %s firmware (%d < %d)\n", + fw->name, sc->fw_dma_size, fw->size); + error =3D ENOMEM; + goto fail5; + } + /* copy firmware image to DMA memory */ memcpy(sc->fw_virtaddr, fw->data, fw->size); =20 @@ -3099,47 +3167,18 @@ goto fail; } =20 =2D /* allocate DMA memory for mapping firmware image */ =2D if (sc->fw_boot.size > sc->fw_dma_size) =2D sc->fw_dma_size =3D sc->fw_boot.size; =2D if (sc->fw_fw.size > sc->fw_dma_size) =2D sc->fw_dma_size =3D sc->fw_fw.size; =2D if (sc->fw_uc.size > sc->fw_dma_size) =2D sc->fw_dma_size =3D sc->fw_uc.size; =2D =2D if (bus_dma_tag_create(NULL, 4, 0, BUS_SPACE_MAXADDR_32BIT, =2D BUS_SPACE_MAXADDR, NULL, NULL, sc->fw_dma_size, 1, sc->fw_dma_size, =2D 0, NULL, NULL, &sc->fw_dmat) !=3D 0) { =2D device_printf(sc->sc_dev, =2D "could not create firmware DMA tag\n"); =2D IWI_LOCK(sc); =2D goto fail; =2D } =2D if (bus_dmamem_alloc(sc->fw_dmat, &sc->fw_virtaddr, 0, =2D &sc->fw_map) !=3D 0) { =2D device_printf(sc->sc_dev, =2D "could not allocate firmware DMA memory\n"); =2D IWI_LOCK(sc); =2D goto fail2; =2D } =2D if (bus_dmamap_load(sc->fw_dmat, sc->fw_map, sc->fw_virtaddr, =2D sc->fw_dma_size, iwi_dma_map_addr, &sc->fw_physaddr, 0) !=3D 0) { =2D device_printf(sc->sc_dev, "could not load firmware DMA map\n"); =2D IWI_LOCK(sc); =2D goto fail3; =2D } IWI_LOCK(sc); =20 if (iwi_load_firmware(sc, &sc->fw_boot) !=3D 0) { device_printf(sc->sc_dev, "could not load boot firmware %s\n", sc->fw_boot.name); =2D goto fail4; + goto fail; } =20 if (iwi_load_ucode(sc, &sc->fw_uc) !=3D 0) { device_printf(sc->sc_dev, "could not load microcode %s\n", sc->fw_uc.name); =2D goto fail4; + goto fail; } =20 iwi_stop_master(sc); @@ -3174,15 +3213,10 @@ if (iwi_load_firmware(sc, &sc->fw_fw) !=3D 0) { device_printf(sc->sc_dev, "could not load main firmware %s\n", sc->fw_fw.name); =2D goto fail4; + goto fail; } sc->flags |=3D IWI_FLAG_FW_INITED; =20 =2D bus_dmamap_sync(sc->fw_dmat, sc->fw_map, BUS_DMASYNC_POSTWRITE); =2D bus_dmamap_unload(sc->fw_dmat, sc->fw_map); =2D bus_dmamem_free(sc->fw_dmat, sc->fw_virtaddr, sc->fw_map); =2D bus_dma_tag_destroy(sc->fw_dmat); =2D if (iwi_config(sc) !=3D 0) { device_printf(sc->sc_dev, "device configuration failed\n"); goto fail; @@ -3206,10 +3240,6 @@ sc->flags &=3D ~IWI_FLAG_FW_LOADING; return; =20 =2Dfail4: bus_dmamap_sync(sc->fw_dmat, sc->fw_map, BUS_DMASYNC_POSTWRITE); =2D bus_dmamap_unload(sc->fw_dmat, sc->fw_map); =2Dfail3: bus_dmamem_free(sc->fw_dmat, sc->fw_virtaddr, sc->fw_map); =2Dfail2: bus_dma_tag_destroy(sc->fw_dmat); fail: ifp->if_flags &=3D ~IFF_UP; sc->flags &=3D ~IWI_FLAG_FW_LOADING; iwi_stop(sc); Index: if_iwireg.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /usr/store/mlaier/fcvs/src/sys/dev/iwi/if_iwireg.h,v retrieving revision 1.2.2.4 diff -u -r1.2.2.4 if_iwireg.h =2D-- if_iwireg.h 29 Oct 2006 08:29:31 -0000 1.2.2.4 +++ if_iwireg.h 18 Feb 2007 15:04:28 -0000 @@ -144,6 +144,9 @@ #define IWI_FW_GET_MAJOR(ver) ((ver) & 0xff) #define IWI_FW_GET_MINOR(ver) (((ver) & 0xff00) >> 8) =20 +/* max firmware size in command blocks (0x1fff) */ +#define IWI_FW_CB_MAX 26 + #define IWI_FW_MODE_UCODE 0 #define IWI_FW_MODE_BOOT 0 #define IWI_FW_MODE_BSS 0 --Boundary-01=_16G2FC0/L+z46iA Content-Type: text/x-diff; charset="iso-8859-1"; name="iwi.diff" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="iwi.diff" Index: if_iwi.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /usr/store/mlaier/fcvs/src/sys/dev/iwi/if_iwi.c,v retrieving revision 1.46 diff -u -r1.46 if_iwi.c =2D-- if_iwi.c 15 Feb 2007 17:21:31 -0000 1.46 +++ if_iwi.c 18 Feb 2007 15:16:00 -0000 @@ -119,6 +119,8 @@ }; =20 static void iwi_dma_map_addr(void *, bus_dma_segment_t *, int, int); +static int iwi_alloc_fw_cb(struct iwi_softc *); +static void iwi_free_fw_cb(struct iwi_softc *); static int iwi_alloc_cmd_ring(struct iwi_softc *, struct iwi_cmd_ring *, int); static void iwi_reset_cmd_ring(struct iwi_softc *, struct iwi_cmd_ring *); @@ -323,6 +325,12 @@ goto fail; } =20 + if (iwi_alloc_fw_cb(sc) !=3D 0) { + device_printf(dev, + "could not allocate firmware command block\n"); + goto fail; + } + /* * Allocate rings. */ @@ -497,6 +505,7 @@ } iwi_put_firmware(sc); =20 + iwi_free_fw_cb(sc); iwi_free_cmd_ring(sc, &sc->cmdq); iwi_free_tx_ring(sc, &sc->txq[0]); iwi_free_tx_ring(sc, &sc->txq[1]); @@ -537,6 +546,57 @@ } =20 static int +iwi_alloc_fw_cb(struct iwi_softc *sc) +{ + int error; + + sc->fw_dma_size =3D IWI_CB_MAXDATALEN * IWI_FW_CB_MAX; + + error =3D bus_dma_tag_create(bus_get_dma_tag(sc->sc_dev), 4, 0, + BUS_SPACE_MAXADDR_32BIT, BUS_SPACE_MAXADDR, NULL, NULL, + sc->fw_dma_size, 1, sc->fw_dma_size, 0, NULL, NULL, + &sc->fw_dmat); + if (error !=3D 0) { + device_printf(sc->sc_dev, + "could not create firmware DMA tag\n"); + goto fail; + } + + error =3D bus_dmamem_alloc(sc->fw_dmat, &sc->fw_virtaddr, 0, + &sc->fw_map); + if (error !=3D 0) { + device_printf(sc->sc_dev, + "could not allocate firmware DMA memory\n"); + goto fail; + } + + error =3D bus_dmamap_load(sc->fw_dmat, sc->fw_map, sc->fw_virtaddr, + sc->fw_dma_size, iwi_dma_map_addr, &sc->fw_physaddr, 0); + if (error !=3D 0) { + device_printf(sc->sc_dev, "could not load firmware DMA map\n"); + goto fail; + } + + return (0); +fail: iwi_free_fw_cb(sc); + return (error); +} + +static void +iwi_free_fw_cb(struct iwi_softc *sc) +{ + if (sc->fw_virtaddr !=3D NULL) { + bus_dmamap_sync(sc->fw_dmat, sc->fw_map, + BUS_DMASYNC_POSTWRITE); + bus_dmamap_unload(sc->fw_dmat, sc->fw_map); + bus_dmamem_free(sc->fw_dmat, sc->fw_virtaddr, sc->fw_map); + } + + if (sc->fw_dmat !=3D NULL) + bus_dma_tag_destroy(sc->fw_dmat); +} + +static int iwi_alloc_cmd_ring(struct iwi_softc *sc, struct iwi_cmd_ring *ring, int co= unt) { int error; @@ -2439,6 +2499,14 @@ uint32_t sentinel, ctl, src, dst, sum, len, mlen, tmp; int ntries, error; =20 + if (fw->size > sc->fw_dma_size) { + device_printf(sc->sc_dev, + "to less dma memory for %s firmware (%d < %d)\n", + fw->name, sc->fw_dma_size, fw->size); + error =3D ENOMEM; + goto fail5; + } + /* copy firmware image to DMA memory */ memcpy(sc->fw_virtaddr, fw->data, fw->size); =20 @@ -3105,48 +3173,18 @@ goto fail; } =20 =2D /* allocate DMA memory for mapping firmware image */ =2D if (sc->fw_boot.size > sc->fw_dma_size) =2D sc->fw_dma_size =3D sc->fw_boot.size; =2D if (sc->fw_fw.size > sc->fw_dma_size) =2D sc->fw_dma_size =3D sc->fw_fw.size; =2D if (sc->fw_uc.size > sc->fw_dma_size) =2D sc->fw_dma_size =3D sc->fw_uc.size; =2D =2D if (bus_dma_tag_create(bus_get_dma_tag(sc->sc_dev), 4, 0,=20 =2D BUS_SPACE_MAXADDR_32BIT, BUS_SPACE_MAXADDR, NULL, NULL,=20 =2D sc->fw_dma_size, 1, sc->fw_dma_size, 0, NULL, NULL,=20 =2D &sc->fw_dmat) !=3D 0) { =2D device_printf(sc->sc_dev, =2D "could not create firmware DMA tag\n"); =2D IWI_LOCK(sc); =2D goto fail; =2D } =2D if (bus_dmamem_alloc(sc->fw_dmat, &sc->fw_virtaddr, 0, =2D &sc->fw_map) !=3D 0) { =2D device_printf(sc->sc_dev, =2D "could not allocate firmware DMA memory\n"); =2D IWI_LOCK(sc); =2D goto fail2; =2D } =2D if (bus_dmamap_load(sc->fw_dmat, sc->fw_map, sc->fw_virtaddr, =2D sc->fw_dma_size, iwi_dma_map_addr, &sc->fw_physaddr, 0) !=3D 0) { =2D device_printf(sc->sc_dev, "could not load firmware DMA map\n"); =2D IWI_LOCK(sc); =2D goto fail3; =2D } IWI_LOCK(sc); =20 if (iwi_load_firmware(sc, &sc->fw_boot) !=3D 0) { device_printf(sc->sc_dev, "could not load boot firmware %s\n", sc->fw_boot.name); =2D goto fail4; + goto fail; } =20 if (iwi_load_ucode(sc, &sc->fw_uc) !=3D 0) { device_printf(sc->sc_dev, "could not load microcode %s\n", sc->fw_uc.name); =2D goto fail4; + goto fail; } =20 iwi_stop_master(sc); @@ -3181,15 +3219,10 @@ if (iwi_load_firmware(sc, &sc->fw_fw) !=3D 0) { device_printf(sc->sc_dev, "could not load main firmware %s\n", sc->fw_fw.name); =2D goto fail4; + goto fail; } sc->flags |=3D IWI_FLAG_FW_INITED; =20 =2D bus_dmamap_sync(sc->fw_dmat, sc->fw_map, BUS_DMASYNC_POSTWRITE); =2D bus_dmamap_unload(sc->fw_dmat, sc->fw_map); =2D bus_dmamem_free(sc->fw_dmat, sc->fw_virtaddr, sc->fw_map); =2D bus_dma_tag_destroy(sc->fw_dmat); =2D if (iwi_config(sc) !=3D 0) { device_printf(sc->sc_dev, "device configuration failed\n"); goto fail; @@ -3213,10 +3246,6 @@ sc->flags &=3D ~IWI_FLAG_FW_LOADING; return; =20 =2Dfail4: bus_dmamap_sync(sc->fw_dmat, sc->fw_map, BUS_DMASYNC_POSTWRITE); =2D bus_dmamap_unload(sc->fw_dmat, sc->fw_map); =2Dfail3: bus_dmamem_free(sc->fw_dmat, sc->fw_virtaddr, sc->fw_map); =2Dfail2: bus_dma_tag_destroy(sc->fw_dmat); fail: ifp->if_flags &=3D ~IFF_UP; sc->flags &=3D ~IWI_FLAG_FW_LOADING; iwi_stop(sc); Index: if_iwireg.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /usr/store/mlaier/fcvs/src/sys/dev/iwi/if_iwireg.h,v retrieving revision 1.13 diff -u -r1.13 if_iwireg.h =2D-- if_iwireg.h 23 Oct 2006 00:34:07 -0000 1.13 +++ if_iwireg.h 18 Feb 2007 15:15:37 -0000 @@ -144,6 +144,9 @@ #define IWI_FW_GET_MAJOR(ver) ((ver) & 0xff) #define IWI_FW_GET_MINOR(ver) (((ver) & 0xff00) >> 8) =20 +/* max firmware size in command blocks (0x1fff) */ +#define IWI_FW_CB_MAX 26 + #define IWI_FW_MODE_UCODE 0 #define IWI_FW_MODE_BOOT 0 #define IWI_FW_MODE_BSS 0 --Boundary-01=_16G2FC0/L+z46iA-- --nextPart1505776.Wia5angaEs Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (FreeBSD) iD8DBQBF2G66XyyEoT62BG0RAvJwAJ99xKCWQJoEOLKds+8e7COPmLuD2wCcDuQJ meb+m9sKDwbiy8H+aY62iME= =e93m -----END PGP SIGNATURE----- --nextPart1505776.Wia5angaEs--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200702181620.26610.max>