Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 29 Apr 2014 14:07:21 -0700
From:      Rui Paulo <rpaulo@felyko.com>
To:        Kevin Lo <kevlo@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r264864 - head/sys/dev/usb/wlan
Message-ID:  <1D92F901-019B-436A-9926-7D7AB4F4C858@felyko.com>
In-Reply-To: <201404240316.s3O3GlUl093950@svn.freebsd.org>
References:  <201404240316.s3O3GlUl093950@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

On Apr 23, 2014, at 20:16, Kevin Lo <kevlo@freebsd.org> wrote:

> Author: kevlo
> Date: Thu Apr 24 03:16:47 2014
> New Revision: 264864
> URL: http://svnweb.freebsd.org/changeset/base/264864
>=20
> Log:
>  Fix panic by adding mtx_assert() to urtwn_init_locked() and
>  urtwn_stop_locked().

This was not a panic, but a LOR.

> Modified:
>  head/sys/dev/usb/wlan/if_urtwn.c
>=20
> Modified: head/sys/dev/usb/wlan/if_urtwn.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=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
> --- head/sys/dev/usb/wlan/if_urtwn.c	Thu Apr 24 02:16:23 2014	=
(r264863)
> +++ head/sys/dev/usb/wlan/if_urtwn.c	Thu Apr 24 03:16:47 2014	=
(r264864)
> @@ -2054,6 +2054,7 @@ urtwn_load_firmware(struct urtwn_softc *
> 	uint32_t reg;
> 	int mlen, ntries, page, error;
>=20
> +	URTWN_UNLOCK(sc);

You didn't need to unlock this early.  The problem was in =
firmware_get().

> 	/* Read firmware image from the filesystem. */
> 	if ((sc->chip & (URTWN_CHIP_UMC_A_CUT | URTWN_CHIP_92C)) =3D=3D
> 	    URTWN_CHIP_UMC_A_CUT)
> @@ -2062,6 +2063,7 @@ urtwn_load_firmware(struct urtwn_softc *
> 		imagename =3D "urtwn-rtl8192cfwT";
>=20
> 	fw =3D firmware_get(imagename);
> +	URTWN_LOCK(sc);
> 	if (fw =3D=3D NULL) {
> 		device_printf(sc->sc_dev,
> 		    "failed loadfirmware of file %s\n", imagename);
> @@ -2816,6 +2818,8 @@ urtwn_init_locked(void *arg)
> 	uint32_t reg;
> 	int error;
>=20
> +	URTWN_ASSERT_LOCKED(sc);
> +
> 	if (ifp->if_drv_flags & IFF_DRV_RUNNING)
> 		urtwn_stop_locked(ifp);
>=20
> @@ -2979,6 +2983,8 @@ urtwn_stop_locked(struct ifnet *ifp)
> {
> 	struct urtwn_softc *sc =3D ifp->if_softc;
>=20
> +	URTWN_ASSERT_LOCKED(sc);
> +
> 	ifp->if_drv_flags &=3D ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE);
>=20
> 	callout_stop(&sc->sc_watchdog_ch);

This is fine, but I wonder if the firmware API should be changed.  The =
problem with this approach, and all USB WiFi drivers, is that by =
unlocking the mutex, the mutual exclusion protection is lost...=20

--
Rui Paulo






Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1D92F901-019B-436A-9926-7D7AB4F4C858>