NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: kern/44144 (ifconfig causes assertion failure in vfs_lookup.c)
The following reply was made to PR kern/44144; it has been noted by GNATS.
From: David Holland <dholland-bugs%netbsd.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc:
Subject: Re: kern/44144 (ifconfig causes assertion failure in vfs_lookup.c)
Date: Sun, 26 Dec 2010 19:10:37 +0000
On Sun, Dec 26, 2010 at 06:55:01PM +0000, David Holland wrote:
> If I guard the firmware_free() call after label fail1 at the end of
> the wpi_cache_firmware() function with "if (wpi_firmware_imaeg != NULL)"
> then I get this:
It'd probably also be good not to free the image twice in the fail2
case. (sheesh)
> # ifconfig wpi0 up
> wpi0: ERROR: could not read firmware file
> wpi0: ERROR: could not load firmware file
> wpi0: cannot assign link-local address
> wpi0: ERROR: could not read firmware file
> wpi0: ERROR: could not load firmware file
> wpi0: ERROR: could not read firmware file
> wpi0: ERROR: could not load firmware file
> wpi0: cannot assign link-local address
> #
>
> There's no panic, which is good, but I think that the first error should
> have been handled in such a way that the other errors did not occur.
Yeah, but that will take some restructuring; as things are it's going
to attempt wpi_cache_firmware every time it goes through wpi_init
until it succeeds, and each successive call is going to print a
message. Just suppressing the message after the first time seems bad,
because then if you try it once, move the firmware into place, then
try again it'll fail silently.
it looks like it's going to print the message every time it
> It would also be nice if the "could not read firmware file" message
> included the file name.
It doesn't look possible to get the path, but the name I think can be
arranged.
This patch should cover more of the bases. (But note that I don't have
one of these, so I haven't tested it at all...)
Index: pci/if_wpi.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_wpi.c,v
retrieving revision 1.48
diff -u -p -r1.48 if_wpi.c
--- pci/if_wpi.c 15 Nov 2010 05:57:39 -0000 1.48
+++ pci/if_wpi.c 26 Dec 2010 19:09:23 -0000
@@ -418,8 +418,12 @@ wpi_detach(device_t self, int flags __un
if (sc->fw_used) {
mutex_enter(&wpi_firmware_mutex);
- if (--wpi_firmware_users == 0)
+ if (--wpi_firmware_users == 0) {
+ KASSERT(wpi_firmware_image != NULL);
firmware_free(wpi_firmware_image, wpi_firmware_size);
+ wpi_firmware_image = NULL;
+ wpi_firmware_size = 0;
+ }
mutex_exit(&wpi_firmware_mutex);
}
@@ -1134,6 +1138,7 @@ wpi_load_microcode(struct wpi_softc *sc,
static int
wpi_cache_firmware(struct wpi_softc *sc)
{
+ static const char fwname[] = "iwlwifi-3945.ucode";
firmware_handle_t fw;
int error;
@@ -1147,9 +1152,10 @@ wpi_cache_firmware(struct wpi_softc *sc)
}
/* load firmware image from disk */
- if ((error = firmware_open("if_wpi","iwlwifi-3945.ucode", &fw) != 0)) {
- aprint_error_dev(sc->sc_dev, "could not read firmware file\n");
- goto fail1;
+ if ((error = firmware_open("if_wpi", fwname, &fw) != 0)) {
+ aprint_error_dev(sc->sc_dev,
+ "Could not open firmware file %s\n", fwname);
+ goto fail0;
}
wpi_firmware_size = firmware_get_size(fw);
@@ -1158,29 +1164,33 @@ wpi_cache_firmware(struct wpi_softc *sc)
WPI_FW_MAIN_TEXT_MAXSZ + WPI_FW_MAIN_DATA_MAXSZ +
WPI_FW_INIT_TEXT_MAXSZ + WPI_FW_INIT_DATA_MAXSZ +
WPI_FW_BOOT_TEXT_MAXSZ) {
- aprint_error_dev(sc->sc_dev, "invalid firmware file\n");
+ aprint_error_dev(sc->sc_dev, "Invalid firmware file %s\n",
+ fwname);
error = EFBIG;
goto fail1;
}
if (wpi_firmware_size < sizeof (struct wpi_firmware_hdr)) {
aprint_error_dev(sc->sc_dev,
- "truncated firmware header: %zu bytes\n",
- wpi_firmware_size);
+ "Truncated firmware header in %s: %zu bytes\n",
+ fwname, wpi_firmware_size);
error = EINVAL;
- goto fail2;
+ goto fail1;
}
wpi_firmware_image = firmware_malloc(wpi_firmware_size);
if (wpi_firmware_image == NULL) {
- aprint_error_dev(sc->sc_dev, "not enough memory to stock
firmware\n");
+ aprint_error_dev(sc->sc_dev,
+ "Not enough memory for firmware %s\n", fwname);
error = ENOMEM;
goto fail1;
}
- if ((error = firmware_read(fw, 0, wpi_firmware_image,
wpi_firmware_size)) != 0) {
- aprint_error_dev(sc->sc_dev, "can't get firmware\n");
- goto fail2;
+ error = firmware_read(fw, 0, wpi_firmware_image, wpi_firmware_size);
+ if (error != 0) {
+ aprint_error_dev(sc->sc_dev,
+ "Error reading firmware file %s\n", fwname);
+ goto fail1;
}
sc->fw_used = true;
@@ -1189,12 +1199,16 @@ wpi_cache_firmware(struct wpi_softc *sc)
return 0;
-fail2:
- firmware_free(wpi_firmware_image, wpi_firmware_size);
fail1:
firmware_close(fw);
- if (--wpi_firmware_users == 0)
+fail0:
+ --wpi_firmware_users;
+ KASSERT(wpi_firmware_users == 0);
+ if (wpi_firmware_image != NULL) {
firmware_free(wpi_firmware_image, wpi_firmware_size);
+ wpi_firmware_image = NULL;
+ wpi_firmware_size = 0;
+ }
mutex_exit(&wpi_firmware_mutex);
return error;
}
@@ -3092,7 +3106,8 @@ wpi_init(struct ifnet *ifp)
WPI_WRITE(sc, WPI_UCODE_CLR, WPI_RADIO_OFF);
if ((error = wpi_load_firmware(sc)) != 0) {
- aprint_error_dev(sc->sc_dev, "could not load firmware\n");
+ /* wpi_load_firmware already prints a message on error */
+ /*aprint_error_dev(sc->sc_dev, "could not load firmware\n");*/
goto fail1;
}
--
David A. Holland
dholland%netbsd.org@localhost
Home |
Main Index |
Thread Index |
Old Index