Date: Thu, 16 Feb 2006 10:15:34 +1300 From: Andrew Thompson <thompsa@freebsd.org> To: FreeBSD Current <current@freebsd.org> Subject: rwlock patch for bridge Message-ID: <20060215211534.GA78376@heff.fud.org.nz>
next in thread | raw e-mail | index | archive | help
--W/nzBZO5zC0uMSeA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Hi,
Here is a patch that changes if_bridge to use rwlock(9) rather than the
handrolled ref counting. Can I please get it reviewed to ensure I have
the changes correct. I pondered if the order of unlocking the softc
mutex and grabbing the rlock mattered but decided it didn't.
It has passed a runtime test.
cheers,
Andrew
--W/nzBZO5zC0uMSeA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="bridge_rwlock.diff"
Index: if_bridge.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_bridge.c,v
retrieving revision 1.54
diff -u -p -r1.54 if_bridge.c
--- if_bridge.c 3 Feb 2006 23:03:07 -0000 1.54
+++ if_bridge.c 15 Feb 2006 01:35:52 -0000
@@ -786,9 +786,9 @@ bridge_delete_member(struct bridge_softc
}
ifs->if_bridge = NULL;
- BRIDGE_XLOCK(sc);
+ BRIDGE_WLOCK(sc);
LIST_REMOVE(bif, bif_next);
- BRIDGE_XDROP(sc);
+ BRIDGE_WUNLOCK(sc);
bridge_rtdelete(sc, ifs, IFBF_FLUSHALL);
@@ -1595,13 +1595,10 @@ bridge_output(struct ifnet *ifp, struct
if (dst_if == NULL) {
struct bridge_iflist *bif;
struct mbuf *mc;
- int error = 0, used = 0;
+ int used = 0;
- BRIDGE_LOCK2REF(sc, error);
- if (error) {
- m_freem(m);
- return (0);
- }
+ BRIDGE_UNLOCK(sc);
+ BRIDGE_RLOCK(sc);
bridge_span(sc, m);
@@ -1644,7 +1641,7 @@ bridge_output(struct ifnet *ifp, struct
}
if (used == 0)
m_freem(m);
- BRIDGE_UNREF(sc);
+ BRIDGE_RUNLOCK(sc);
return (0);
}
@@ -2045,14 +2042,10 @@ bridge_broadcast(struct bridge_softc *sc
struct bridge_iflist *bif;
struct mbuf *mc;
struct ifnet *dst_if;
- int error = 0, used = 0;
+ int used = 0;
- BRIDGE_LOCK_ASSERT(sc);
- BRIDGE_LOCK2REF(sc, error);
- if (error) {
- m_freem(m);
- return;
- }
+ BRIDGE_UNLOCK(sc);
+ BRIDGE_RLOCK(sc);
/* Filter on the bridge interface before broadcasting */
if (runfilt && (PFIL_HOOKED(&inet_pfil_hook)
@@ -2119,7 +2112,7 @@ bridge_broadcast(struct bridge_softc *sc
m_freem(m);
out:
- BRIDGE_UNREF(sc);
+ BRIDGE_RUNLOCK(sc);
}
/*
Index: if_bridgevar.h
===================================================================
RCS file: /home/ncvs/src/sys/net/if_bridgevar.h,v
retrieving revision 1.10
diff -u -p -r1.10 if_bridgevar.h
--- if_bridgevar.h 14 Jan 2006 03:51:30 -0000 1.10
+++ if_bridgevar.h 15 Feb 2006 20:38:33 -0000
@@ -75,8 +75,7 @@
*/
#include <sys/callout.h>
-#include <sys/queue.h>
-#include <sys/condvar.h>
+#include <sys/rwlock.h>
/*
* Commands used in the SIOCSDRVSPEC ioctl. Note the lookup of the
@@ -270,7 +269,7 @@ struct bridge_softc {
struct ifnet *sc_ifp; /* make this an interface */
LIST_ENTRY(bridge_softc) sc_list;
struct mtx sc_mtx;
- struct cv sc_cv;
+ struct rwlock sc_rwlock;
uint64_t sc_designated_root;
uint64_t sc_bridge_id;
struct bridge_iflist *sc_root_port;
@@ -294,8 +293,6 @@ struct bridge_softc {
uint32_t sc_brttimeout; /* rt timeout in seconds */
struct callout sc_brcallout; /* bridge callout */
struct callout sc_bstpcallout; /* STP callout */
- uint32_t sc_iflist_ref; /* refcount for sc_iflist */
- uint32_t sc_iflist_xcnt; /* refcount for sc_iflist */
LIST_HEAD(, bridge_iflist) sc_iflist; /* member interface list */
LIST_HEAD(, bridge_rtnode) *sc_rthash; /* our forwarding table */
LIST_HEAD(, bridge_rtnode) sc_rtlist; /* list version of above */
@@ -305,41 +302,20 @@ struct bridge_softc {
#define BRIDGE_LOCK_INIT(_sc) do { \
mtx_init(&(_sc)->sc_mtx, "if_bridge", NULL, MTX_DEF); \
- cv_init(&(_sc)->sc_cv, "if_bridge_cv"); \
+ rw_init(&(_sc)->sc_rwlock, "if_bridge_rw"); \
} while (0)
#define BRIDGE_LOCK_DESTROY(_sc) do { \
mtx_destroy(&(_sc)->sc_mtx); \
- cv_destroy(&(_sc)->sc_cv); \
+ rw_destroy(&(_sc)->sc_rwlock); \
} while (0)
#define BRIDGE_LOCK(_sc) mtx_lock(&(_sc)->sc_mtx)
#define BRIDGE_UNLOCK(_sc) mtx_unlock(&(_sc)->sc_mtx)
#define BRIDGE_LOCKED(_sc) mtx_owned(&(_sc)->sc_mtx)
#define BRIDGE_LOCK_ASSERT(_sc) mtx_assert(&(_sc)->sc_mtx, MA_OWNED)
-#define BRIDGE_LOCK2REF(_sc, _err) do { \
- mtx_assert(&(_sc)->sc_mtx, MA_OWNED); \
- if ((_sc)->sc_iflist_xcnt > 0) \
- (_err) = EBUSY; \
- else \
- (_sc)->sc_iflist_ref++; \
- mtx_unlock(&(_sc)->sc_mtx); \
-} while (0)
-#define BRIDGE_UNREF(_sc) do { \
- mtx_lock(&(_sc)->sc_mtx); \
- (_sc)->sc_iflist_ref--; \
- if (((_sc)->sc_iflist_xcnt > 0) && ((_sc)->sc_iflist_ref == 0)) \
- cv_broadcast(&(_sc)->sc_cv); \
- mtx_unlock(&(_sc)->sc_mtx); \
-} while (0)
-#define BRIDGE_XLOCK(_sc) do { \
- mtx_assert(&(_sc)->sc_mtx, MA_OWNED); \
- (_sc)->sc_iflist_xcnt++; \
- while ((_sc)->sc_iflist_ref > 0) \
- cv_wait(&(_sc)->sc_cv, &(_sc)->sc_mtx); \
-} while (0)
-#define BRIDGE_XDROP(_sc) do { \
- mtx_assert(&(_sc)->sc_mtx, MA_OWNED); \
- (_sc)->sc_iflist_xcnt--; \
-} while (0)
+#define BRIDGE_RLOCK(_sc) rw_rlock(&(_sc)->sc_rwlock)
+#define BRIDGE_WLOCK(_sc) rw_wlock(&(_sc)->sc_rwlock)
+#define BRIDGE_RUNLOCK(_sc) rw_runlock(&(_sc)->sc_rwlock)
+#define BRIDGE_WUNLOCK(_sc) rw_wunlock(&(_sc)->sc_rwlock)
#define BRIDGE_INPUT(_ifp, _m) do { \
KASSERT(bridge_input_p != NULL, \
--W/nzBZO5zC0uMSeA--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060215211534.GA78376>