kern/85086: [PATCH] Locking fixes for ef(4) (+removes mem. leak)
Wojciech A. Koszek
dunstan at freebsd.czest.pl
Thu Aug 18 11:40:14 GMT 2005
>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 at 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 at 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 <sys/syslog.h>
#include <sys/kernel.h>
#include <sys/module.h>
+#include <sys/queue.h>
+#include <sys/lock.h>
+#include <sys/mutex.h>
#include <net/ethernet.h>
#include <net/if_llc.h>
@@ -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:
More information about the freebsd-bugs
mailing list