Re: NET_MPSAFE support for ifmedia / mii

[Jason Thorpe <thorpej%me.com@localhost>]

> On Feb 5, 2020, at 10:25 PM, Jason Thorpe <thorpej%me.com@localhost> wrote:
> 
> Please provide feedback on the changes.  I still plan to update some more comments, and do some additional testing, before checking this in, but I would like some eyeballs on this now.

Updated patch.  I fixed some bugs, and have been using this under LOCKDEBUG with both NET_MPSAFE and without for a couple of days with no ill effects (using converted "wm" driver and unconverted "pcn" driver).  I also had to convert the "usbnet" code, because it can't use the "non-converted driver compatibility" mechanism I provided (it sleeps, frequently).  I did, however, make its locking protocol a lot simpler.  I need help testing the usbnet changes -- if you have a USB Ethernet dongle, ping me off-list.

Index: dev/pci/if_wm.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_wm.c,v
retrieving revision 1.658
diff -u -p -r1.658 if_wm.c
--- dev/pci/if_wm.c	13 Dec 2019 02:03:46 -0000	1.658
+++ dev/pci/if_wm.c	16 Jan 2020 04:49:16 -0000
@@ -3398,13 +3398,7 @@ wm_ioctl(struct ifnet *ifp, u_long cmd, 
 			sc->sc_flowflags = ifr->ifr_media & IFM_ETH_FMASK;
 		}
 		WM_CORE_UNLOCK(sc);
-#ifdef WM_MPSAFE
-		s = splnet();
-#endif
 		error = ifmedia_ioctl(ifp, ifr, &sc->sc_mii.mii_media, cmd);
-#ifdef WM_MPSAFE
-		splx(s);
-#endif
 		break;
 	case SIOCINITIFADDR:
 		WM_CORE_LOCK(sc);
Index: net/if_media.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_media.c,v
retrieving revision 1.48
diff -u -p -r1.48 if_media.c
--- net/if_media.c	1 Oct 2019 17:45:25 -0000	1.48
+++ net/if_media.c	16 Jan 2020 04:49:16 -0000
@@ -78,6 +78,10 @@
 #include <sys/cdefs.h>
 __KERNEL_RCSID(0, "$NetBSD: if_media.c,v 1.48 2019/10/01 17:45:25 chs Exp $");
 
+#ifdef _KERNEL_OPT
+#include "opt_net_mpsafe.h"
+#endif
+
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/errno.h>
@@ -361,13 +365,32 @@ ifmedia_ioctl(struct ifnet *ifp, struct 
 {
 	int e;
 
+#ifdef NET_MPSAFE
 	/*
-	 * If if_is_mpsafe(ifp), KERNEL_LOCK isn't held here,
-	 * but ifmedia_ioctl_locked isn't MP-safe yet, so we must hold the lock.
+	 * If if_is_mpsafe(ifp), KERNEL_LOCK isn't held here and
+	 * ipl will not have been raised (well, maybe it has, but
+	 * we can safely raise it regardless), but ifmedia_ioctl_locked
+	 * isn't MP-safe yet, so we go to splnet and hold the lock.
+	 *
+	 * In the non-mpsafe case, the interface's ioctl routine
+	 * will already be running at splnet.
 	 */
-	KERNEL_LOCK_IF_IFP_MPSAFE(ifp);
+	int s;
+	if (if_is_mpsafe(ifp)) {
+		s = splnet();
+		KERNEL_LOCK(1, NULL);
+	}
+#endif /* NET_MPSAFE */
+
 	e = ifmedia_ioctl_locked(ifp, ifr, ifm, cmd);
-	KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp);
+
+#ifdef NET_MPSAFE
+	if (if_is_mpsafe(ifp)) {
+		KERNEL_UNLOCK_ONE(NULL);
+		splx(s);
+	}
+#endif /* NET_MPSAFE */
+
 	return e;
 }
 

-- thorpej