Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 28 Aug 2009 20:07:38 +0000 (UTC)
From:      Robert Watson <rwatson@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org
Subject:   svn commit: r196627 - in stable/8/sys: . amd64/include/xen cddl/contrib/opensolaris contrib/dev/acpica contrib/pf dev/xen/xenpci net
Message-ID:  <200908282007.n7SK7cxk035166@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: rwatson
Date: Fri Aug 28 20:07:38 2009
New Revision: 196627
URL: http://svn.freebsd.org/changeset/base/196627

Log:
  Merge r196482 from head to stable/8:
  
    Rather than using IFNET_RLOCK() when iterating over (and modifying) the
    ifnet list during if_ef load, directly acquire the ifnet_sxlock
    exclusively.  That way when if_alloc() recurses the lock, it's a write
    recursion rather than a read->write recursion.
  
    This code structure is arguably a bug, so add a comment indicating that
    this is the case.  Post-8.0, we should fix this, but this commit
    resolves panic-on-load for if_ef.
  
    Discussed with:       bz, julian
    Reported by:  phk
  
  Approved by:	re (kib)

Modified:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)
  stable/8/sys/dev/xen/xenpci/   (props changed)
  stable/8/sys/net/if_ef.c

Modified: stable/8/sys/net/if_ef.c
==============================================================================
--- stable/8/sys/net/if_ef.c	Fri Aug 28 20:06:02 2009	(r196626)
+++ stable/8/sys/net/if_ef.c	Fri Aug 28 20:07:38 2009	(r196627)
@@ -492,7 +492,20 @@ ef_load(void)
 	VNET_LIST_RLOCK();
 	VNET_FOREACH(vnet_iter) {
 		CURVNET_SET(vnet_iter);
-		IFNET_RLOCK();
+
+		/*
+		 * XXXRW: The following loop walks the ifnet list while
+		 * modifying it, something not well-supported by ifnet
+		 * locking.  To avoid lock upgrade/recursion issues, manually
+		 * acquire a write lock of ifnet_sxlock here, rather than a
+		 * read lock, so that when if_alloc() recurses the lock, we
+		 * don't panic.  This structure, in which if_ef automatically
+		 * attaches to all ethernet interfaces, should be replaced
+		 * with a model like that found in if_vlan, in which
+		 * interfaces are explicitly configured, which would avoid
+		 * this (and other) problems.
+		 */
+		sx_xlock(&ifnet_sxlock);
 		TAILQ_FOREACH(ifp, &V_ifnet, if_link) {
 			if (ifp->if_type != IFT_ETHER) continue;
 			EFDEBUG("Found interface %s\n", ifp->if_xname);
@@ -523,7 +536,7 @@ ef_load(void)
 			efcount++;
 			SLIST_INSERT_HEAD(&efdev, efl, el_next);
 		}
-		IFNET_RUNLOCK();
+		sx_xunlock(&ifnet_sxlock);
 		CURVNET_RESTORE();
 	}
 	VNET_LIST_RUNLOCK();



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