rwlock patch for bridge

[Andrew Thompson]

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
-------------- next part --------------
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,			\