From owner-freebsd-bugs@FreeBSD.ORG Thu Aug 18 11:40:14 2005 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 14EFB16A420 for ; Thu, 18 Aug 2005 11:40:14 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 538A243D48 for ; Thu, 18 Aug 2005 11:40:13 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.3/8.13.3) with ESMTP id j7IBeDep098513 for ; Thu, 18 Aug 2005 11:40:13 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.3/8.13.1/Submit) id j7IBeDRW098512; Thu, 18 Aug 2005 11:40:13 GMT (envelope-from gnats) Resent-Date: Thu, 18 Aug 2005 11:40:13 GMT Resent-Message-Id: <200508181140.j7IBeDRW098512@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, "Wojciech A. Koszek" Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id A732F16A420 for ; Thu, 18 Aug 2005 11:34:11 +0000 (GMT) (envelope-from dunstan@freebsd.czest.pl) Received: from freebsd.czest.pl (silver.iplus.pl [80.48.250.4]) by mx1.FreeBSD.org (Postfix) with ESMTP id B8D4243D69 for ; Thu, 18 Aug 2005 11:34:05 +0000 (GMT) (envelope-from dunstan@freebsd.czest.pl) Received: from freebsd.czest.pl (freebsd.czest.pl [80.48.250.4]) by freebsd.czest.pl (8.12.10/8.12.9) with ESMTP id j7IBn4GW083543 for ; Thu, 18 Aug 2005 11:49:04 GMT (envelope-from dunstan@freebsd.czest.pl) Received: (from dunstan@localhost) by freebsd.czest.pl (8.12.10/8.12.9/Submit) id j7IBn4s2083542; Thu, 18 Aug 2005 11:49:04 GMT (envelope-from dunstan) Message-Id: <200508181149.j7IBn4s2083542@freebsd.czest.pl> Date: Thu, 18 Aug 2005 11:49:04 GMT From: "Wojciech A. Koszek" To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: Subject: kern/85086: [PATCH] Locking fixes for ef(4) (+removes mem. leak) X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: "Wojciech A. Koszek" List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Aug 2005 11:40:14 -0000 >Number: 85086 >Category: kern >Synopsis: [PATCH] Locking fixes for ef(4) (+removes mem. leak) >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Aug 18 11:40:12 GMT 2005 >Closed-Date: >Last-Modified: >Originator: Wojciech A. Koszek >Release: FreeBSD 7.0-CURRENT i386 >Organization: >Environment: System: FreeBSD laptop.freebsd.czest.pl 7.0-CURRENT FreeBSD 7.0-CURRENT #18: Tue Aug 16 12:29:31 CEST 2005 dunstan@laptop.freebsd.czest.pl:/usr/obj/usr/src/sys/LAPTOP i386 >Description: Patch against FreeBSD 7.0-CURRENT, kern.osreldate: 700002. Makes ef(4) MPSAFE, fixes memory leak, removes one unused macro and one unused variable. Brings also corrections which David Brooks has sent me. >How-To-Repeat: >Fix: --- diff.locking.if_ef.c begins here --- (c) 2005 Wojciech A. Koszek dunstan@FreeBSD.czest.pl diff -upr /usr/src/sys/net/if_ef.c src/sys/net/if_ef.c --- /usr/src/sys/net/if_ef.c Sun Aug 14 14:38:51 2005 +++ src/sys/net/if_ef.c Thu Aug 18 12:57:07 2005 @@ -39,6 +39,9 @@ #include #include #include +#include +#include +#include #include #include @@ -83,8 +86,6 @@ #define EFDEBUG(format, args...) #endif -#define EFERROR(format, args...) printf("%s: "format, __func__ ,## args) - struct efnet { struct ifnet *ef_ifp; struct ifnet *ef_pifp; @@ -98,7 +99,12 @@ struct ef_link { }; static SLIST_HEAD(ef_link_head, ef_link) efdev = {NULL}; -static int efcount; +static struct mtx ef_mtx; + +#define EF_MTX_INIT() mtx_init(&ef_mtx, "efmtx", NULL, MTX_DEF) +#define EF_MTX_DESTROY() mtx_destroy(&ef_mtx) +#define EF_LOCK() mtx_lock(&ef_mtx) +#define EF_UNLOCK() mtx_unlock(&ef_mtx) extern int (*ef_inputp)(struct ifnet*, struct ether_header *eh, struct mbuf *m); extern int (*ef_outputp)(struct ifnet *ifp, struct mbuf **mp, @@ -156,14 +162,10 @@ static int ef_detach(struct efnet *sc) { struct ifnet *ifp = sc->ef_ifp; - int s; - - s = splimp(); - + ether_ifdetach(ifp); if_free(ifp); - splx(s); return 0; } @@ -177,11 +179,10 @@ ef_ioctl(struct ifnet *ifp, u_long cmd, { struct efnet *sc = ifp->if_softc; struct ifaddr *ifa = (struct ifaddr*)data; - int s, error; + int error; EFDEBUG("IOCTL %ld for %s\n", cmd, ifp->if_xname); error = 0; - s = splimp(); switch (cmd) { case SIOCSIFFLAGS: error = 0; @@ -198,7 +199,6 @@ ef_ioctl(struct ifnet *ifp, u_long cmd, error = ether_ioctl(ifp, cmd, data); break; } - splx(s); return error; } @@ -355,12 +355,14 @@ ef_input(struct ifnet *ifp, struct ether * Check if interface configured for the given frame */ efp = NULL; + EF_LOCK(); SLIST_FOREACH(efl, &efdev, el_next) { if (efl->el_ifp == ifp) { efp = efl->el_units[ft]; break; } } + EF_UNLOCK(); if (efp == NULL) { EFDEBUG("Can't find if for %d\n", ft); return EPROTONOSUPPORT; @@ -477,7 +479,7 @@ ef_clone(struct ef_link *efl, int ft) efp->ef_pifp = ifp; efp->ef_frametype = ft; eifp = efp->ef_ifp = if_alloc(IFT_ETHER); - if (ifp == NULL) + if (eifp == NULL) return (ENOSPC); snprintf(eifp->if_xname, IFNAMSIZ, "%sf%d", ifp->if_xname, efp->ef_frametype); @@ -500,7 +502,8 @@ ef_load(void) IFNET_RLOCK(); TAILQ_FOREACH(ifp, &ifnet, if_link) { - if (ifp->if_type != IFT_ETHER) continue; + if (ifp->if_type != IFT_ETHER) + continue; EFDEBUG("Found interface %s\n", ifp->if_xname); efl = (struct ef_link*)malloc(sizeof(struct ef_link), M_IFADDR, M_WAITOK | M_ZERO); @@ -508,7 +511,7 @@ ef_load(void) error = ENOMEM; break; } - + efl->el_ifp = ifp; #ifdef ETHER_II error = ef_clone(efl, ETHER_FT_EII); @@ -526,31 +529,37 @@ ef_load(void) error = ef_clone(efl, ETHER_FT_SNAP); if (error) break; #endif - efcount++; + EF_LOCK(); SLIST_INSERT_HEAD(&efdev, efl, el_next); + EF_UNLOCK(); } IFNET_RUNLOCK(); if (error) { - if (efl) + EF_LOCK(); + if (efl != NULL) SLIST_INSERT_HEAD(&efdev, efl, el_next); SLIST_FOREACH(efl, &efdev, el_next) { for (d = 0; d < EF_NFT; d++) - if (efl->el_units[d]) { - if (efl->el_units[d]->ef_pifp != NULL) - if_free(efl->el_units[d]->ef_pifp); + efp = efl->el_units[d]; + if (efp != NULL) { + if (efp->ef_ifp != NULL) { + if_free(efp->ef_ifp); + } free(efl->el_units[d], M_IFADDR); } - free(efl, M_IFADDR); } + EF_UNLOCK(); return error; } + EF_LOCK(); SLIST_FOREACH(efl, &efdev, el_next) { for (d = 0; d < EF_NFT; d++) { efp = efl->el_units[d]; - if (efp) + if (efp != NULL) ef_attach(efp); } } + EF_UNLOCK(); ef_inputp = ef_input; ef_outputp = ef_output; EFDEBUG("Loaded\n"); @@ -566,14 +575,19 @@ ef_unload(void) ef_inputp = NULL; ef_outputp = NULL; - SLIST_FOREACH(efl, &efdev, el_next) { + EF_LOCK(); + while ((efl = SLIST_FIRST(&efdev)) != NULL) { + SLIST_REMOVE_HEAD(&efdev, el_next); for (d = 0; d < EF_NFT; d++) { efp = efl->el_units[d]; - if (efp) { + if (efp != NULL) { ef_detach(efp); + free(efp, M_IFADDR); } } + free(efl, M_IFADDR); } + EF_UNLOCK(); EFDEBUG("Unloaded\n"); return 0; } @@ -581,15 +595,21 @@ ef_unload(void) static int if_ef_modevent(module_t mod, int type, void *data) { + int error = 0; + switch ((modeventtype_t)type) { - case MOD_LOAD: - return ef_load(); - case MOD_UNLOAD: - return ef_unload(); - default: - return EOPNOTSUPP; + case MOD_LOAD: + EF_MTX_INIT(); + error = ef_load(); + break; + case MOD_UNLOAD: + error = ef_unload(); + EF_MTX_DESTROY(); + break; + default: + error = EOPNOTSUPP; } - return 0; + return (error); } static moduledata_t if_ef_mod = { --- diff.locking.if_ef.c ends here --- >Release-Note: >Audit-Trail: >Unformatted: