Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 26 Sep 2012 07:52:04 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Ed Maste <emaste@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r240938 - head/sys/net
Message-ID:  <20120926045204.GG35915@deviant.kiev.zoral.com.ua>
In-Reply-To: <201209252210.q8PMAEx5003950@svn.freebsd.org>
References:  <201209252210.q8PMAEx5003950@svn.freebsd.org>

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

--Y+xroYBkGM9OatJL
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Sep 25, 2012 at 10:10:14PM +0000, Ed Maste wrote:
> Author: emaste
> Date: Tue Sep 25 22:10:14 2012
> New Revision: 240938
> URL: http://svn.freebsd.org/changeset/base/240938
>=20
> Log:
>   Avoid INVARIANTS panic destroying an in-use tap(4)
>  =20
>   The requirement (implied by the KASSERT in tap_destroy) that the tap is
>   closed isn't valid; destroy_dev will block in devdrn while other threads
>   are in d_* functions.
Are you sure ?

The device may be opened, but no threads could be in any cdevsw
methods. destroy_dev(9) only waits for threads to leave cdevsw methods,
and not for the close to happen.

This is normal situation, but driver shall be aware of it and
handle it properly.

>  =20
>   Note: if_tun had the same issue, addressed in SVN revisions r186391,
>   r186483 and r186497.  The use of the condvar there appears to be
>   redundant with the functionality provided by destroy_dev.
>  =20
>   Sponsored by:	ADARA Networks
>   Reviewed by:	dwhite
>   MFC after:	2 weeks
>=20
> Modified:
>   head/sys/net/if_tap.c
>   head/sys/net/if_tapvar.h
>=20
> Modified: head/sys/net/if_tap.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/net/if_tap.c	Tue Sep 25 21:33:36 2012	(r240937)
> +++ head/sys/net/if_tap.c	Tue Sep 25 22:10:14 2012	(r240938)
> @@ -213,14 +213,10 @@ tap_destroy(struct tap_softc *tp)
>  {
>  	struct ifnet *ifp =3D tp->tap_ifp;
> =20
> -	/* Unlocked read. */
> -	KASSERT(!(tp->tap_flags & TAP_OPEN),
> -		("%s flags is out of sync", ifp->if_xname));
> -
>  	CURVNET_SET(ifp->if_vnet);
> +	destroy_dev(tp->tap_dev);
>  	seldrain(&tp->tap_rsel);
>  	knlist_destroy(&tp->tap_rsel.si_note);
> -	destroy_dev(tp->tap_dev);
>  	ether_ifdetach(ifp);
>  	if_free(ifp);
> =20
>=20
> Modified: head/sys/net/if_tapvar.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=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> --- head/sys/net/if_tapvar.h	Tue Sep 25 21:33:36 2012	(r240937)
> +++ head/sys/net/if_tapvar.h	Tue Sep 25 22:10:14 2012	(r240938)
> @@ -64,6 +64,7 @@ struct tap_softc {
>  	SLIST_ENTRY(tap_softc)	tap_next;	/* next device in chain      */
>  	struct cdev *tap_dev;
>  	struct mtx	 tap_mtx;		/* per-softc mutex */
> +	struct cv	 tap_cv;		/* protect ref'd dev destroy */=20
>  };
> =20
>  #endif /* !_NET_IF_TAPVAR_H_ */

--Y+xroYBkGM9OatJL
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (FreeBSD)

iEYEARECAAYFAlBiifQACgkQC3+MBN1Mb4h2vQCfRZkdCRvUiAtluuB+UzbYfZgv
1h8An0bkf+q1VEsLaKDgaIHa7MbygL4A
=Vbpb
-----END PGP SIGNATURE-----

--Y+xroYBkGM9OatJL--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120926045204.GG35915>