Author: sephe
Date: Thu Oct 13 09:00:44 2016
New Revision: 307209
URL: https://svnweb.freebsd.org/changeset/base/307209

Log:
  MFC 305794
  
      hyperv/hn: Use sx for the main lock.
  
      Sponsored by:   Microsoft
      Differential Revision:  https://reviews.freebsd.org/D7870

Modified:
  stable/10/sys/dev/hyperv/netvsc/hv_net_vsc.h
  stable/10/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/dev/hyperv/netvsc/hv_net_vsc.h
==============================================================================
--- stable/10/sys/dev/hyperv/netvsc/hv_net_vsc.h        Thu Oct 13 08:56:52 
2016        (r307208)
+++ stable/10/sys/dev/hyperv/netvsc/hv_net_vsc.h        Thu Oct 13 09:00:44 
2016        (r307209)
@@ -205,10 +205,7 @@ struct hn_softc {
        device_t        hn_dev;
        int             hn_carrier;
        int             hn_if_flags;
-       struct mtx      hn_lock;
-       int             hn_initdone;
-       /* See hv_netvsc_drv_freebsd.c for rules on how to use */
-       int             temp_unusable;
+       struct sx       hn_lock;
        struct vmbus_channel *hn_prichan;
 
        int             hn_rx_ring_cnt;

Modified: stable/10/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c
==============================================================================
--- stable/10/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c     Thu Oct 13 
08:56:52 2016        (r307208)
+++ stable/10/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c     Thu Oct 13 
09:00:44 2016        (r307209)
@@ -197,21 +197,12 @@ struct hn_txdesc {
 
 #define HN_LRO_ACKCNT_DEF              1
 
-/*
- * Be aware that this sleepable mutex will exhibit WITNESS errors when
- * certain TCP and ARP code paths are taken.  This appears to be a
- * well-known condition, as all other drivers checked use a sleeping
- * mutex to protect their transmit paths.
- * Also Be aware that mutexes do not play well with semaphores, and there
- * is a conflicting semaphore in a certain channel code path.
- */
-#define NV_LOCK_INIT(_sc, _name) \
-           mtx_init(&(_sc)->hn_lock, _name, MTX_NETWORK_LOCK, MTX_DEF)
-#define NV_LOCK(_sc)           mtx_lock(&(_sc)->hn_lock)
-#define NV_LOCK_ASSERT(_sc)    mtx_assert(&(_sc)->hn_lock, MA_OWNED)
-#define NV_UNLOCK(_sc)         mtx_unlock(&(_sc)->hn_lock)
-#define NV_LOCK_DESTROY(_sc)   mtx_destroy(&(_sc)->hn_lock)
-
+#define HN_LOCK_INIT(sc)               \
+       sx_init(&(sc)->hn_lock, device_get_nameunit((sc)->hn_dev))
+#define HN_LOCK_ASSERT(sc)             sx_assert(&(sc)->hn_lock, SA_XLOCKED)
+#define HN_LOCK_DESTROY(sc)            sx_destroy(&(sc)->hn_lock)
+#define HN_LOCK(sc)                    sx_xlock(&(sc)->hn_lock)
+#define HN_UNLOCK(sc)                  sx_xunlock(&(sc)->hn_lock)
 
 /*
  * Globals
@@ -478,6 +469,7 @@ netvsc_attach(device_t dev)
 
        sc->hn_dev = dev;
        sc->hn_prichan = vmbus_get_channel(dev);
+       HN_LOCK_INIT(sc);
 
        if (hn_tx_taskq == NULL) {
                sc->hn_tx_taskq = taskqueue_create("hn_tx", M_WAITOK,
@@ -500,7 +492,6 @@ netvsc_attach(device_t dev)
        } else {
                sc->hn_tx_taskq = hn_tx_taskq;
        }
-       NV_LOCK_INIT(sc, "NetVSCLock");
 
        ifp = sc->hn_ifp = sc->arpcom.ac_ifp = if_alloc(IFT_ETHER);
        ifp->if_softc = sc;
@@ -681,6 +672,7 @@ netvsc_detach(device_t dev)
                taskqueue_free(sc->hn_tx_taskq);
 
        vmbus_xact_ctx_destroy(sc->hn_xact);
+       HN_LOCK_DESTROY(sc);
        return (0);
 }
 
@@ -1491,39 +1483,27 @@ skip:
        return (0);
 }
 
-/*
- * Rules for using sc->temp_unusable:
- * 1.  sc->temp_unusable can only be read or written while holding NV_LOCK()
- * 2.  code reading sc->temp_unusable under NV_LOCK(), and finding 
- *     sc->temp_unusable set, must release NV_LOCK() and exit
- * 3.  to retain exclusive control of the interface,
- *     sc->temp_unusable must be set by code before releasing NV_LOCK()
- * 4.  only code setting sc->temp_unusable can clear sc->temp_unusable
- * 5.  code setting sc->temp_unusable must eventually clear sc->temp_unusable
- */
-
-/*
- * Standard ioctl entry point.  Called when the user wants to configure
- * the interface.
- */
 static int
 hn_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 {
        struct hn_softc *sc = ifp->if_softc;
        struct ifreq *ifr = (struct ifreq *)data;
        int mask, error = 0;
-       int retry_cnt = 500;
-       
+
        switch (cmd) {
        case SIOCSIFMTU:
-               if (ifp->if_mtu == ifr->ifr_mtu)
-                       break;
-
                if (ifr->ifr_mtu > NETVSC_MAX_CONFIGURABLE_MTU) {
                        error = EINVAL;
                        break;
                }
 
+               HN_LOCK(sc);
+
+               if (ifp->if_mtu == ifr->ifr_mtu) {
+                       HN_UNLOCK(sc);
+                       break;
+               }
+
                /* Obtain and record requested MTU */
                ifp->if_mtu = ifr->ifr_mtu;
 
@@ -1532,40 +1512,18 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, 
                 * Make sure that LRO aggregation length limit is still
                 * valid, after the MTU change.
                 */
-               NV_LOCK(sc);
                if (sc->hn_rx_ring[0].hn_lro.lro_length_lim <
                    HN_LRO_LENLIM_MIN(ifp))
                        hn_set_lro_lenlim(sc, HN_LRO_LENLIM_MIN(ifp));
-               NV_UNLOCK(sc);
 #endif
 
-               do {
-                       NV_LOCK(sc);
-                       if (!sc->temp_unusable) {
-                               sc->temp_unusable = TRUE;
-                               retry_cnt = -1;
-                       }
-                       NV_UNLOCK(sc);
-                       if (retry_cnt > 0) {
-                               retry_cnt--;
-                               DELAY(5 * 1000);
-                       }
-               } while (retry_cnt > 0);
-
-               if (retry_cnt == 0) {
-                       error = EINVAL;
-                       break;
-               }
-
                /* We must remove and add back the device to cause the new
                 * MTU to take effect.  This includes tearing down, but not
                 * deleting the channel, then bringing it back up.
                 */
                error = hv_rf_on_device_remove(sc);
                if (error) {
-                       NV_LOCK(sc);
-                       sc->temp_unusable = FALSE;
-                       NV_UNLOCK(sc);
+                       HN_UNLOCK(sc);
                        break;
                }
 
@@ -1585,29 +1543,11 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, 
 
                hn_init_locked(sc);
 
-               NV_LOCK(sc);
-               sc->temp_unusable = FALSE;
-               NV_UNLOCK(sc);
+               HN_UNLOCK(sc);
                break;
 
        case SIOCSIFFLAGS:
-               do {
-                       NV_LOCK(sc);
-                       if (!sc->temp_unusable) {
-                               sc->temp_unusable = TRUE;
-                               retry_cnt = -1;
-                       }
-                       NV_UNLOCK(sc);
-                       if (retry_cnt > 0) {
-                               retry_cnt--;
-                               DELAY(5 * 1000);
-                       }
-                } while (retry_cnt > 0);
-
-                if (retry_cnt == 0) {
-                       error = EINVAL;
-                       break;
-                }
+               HN_LOCK(sc);
 
                if (ifp->if_flags & IFF_UP) {
                        /*
@@ -1636,14 +1576,13 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, 
                                hn_stop(sc);
                        }
                }
-               NV_LOCK(sc);
-               sc->temp_unusable = FALSE;
-               NV_UNLOCK(sc);
                sc->hn_if_flags = ifp->if_flags;
+
+               HN_UNLOCK(sc);
                break;
 
        case SIOCSIFCAP:
-               NV_LOCK(sc);
+               HN_LOCK(sc);
 
                mask = ifr->ifr_reqcap ^ ifp->if_capenable;
                if (mask & IFCAP_TXCSUM) {
@@ -1679,7 +1618,7 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, 
                                ifp->if_hwassist &= ~CSUM_IP6_TSO;
                }
 
-               NV_UNLOCK(sc);
+               HN_UNLOCK(sc);
                break;
 
        case SIOCADDMULTI:
@@ -1710,6 +1649,8 @@ hn_stop(struct hn_softc *sc)
        struct ifnet *ifp;
        int ret, i;
 
+       HN_LOCK_ASSERT(sc);
+
        ifp = sc->hn_ifp;
 
        if (bootverbose)
@@ -1721,7 +1662,6 @@ hn_stop(struct hn_softc *sc)
                sc->hn_tx_ring[i].hn_oactive = 0;
 
        if_link_state_change(ifp, LINK_STATE_DOWN);
-       sc->hn_initdone = 0;
 
        ret = hv_rf_on_close(sc);
 }
@@ -1790,6 +1730,8 @@ hn_init_locked(struct hn_softc *sc)
        struct ifnet *ifp;
        int ret, i;
 
+       HN_LOCK_ASSERT(sc);
+
        ifp = sc->hn_ifp;
 
        if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
@@ -1799,11 +1741,8 @@ hn_init_locked(struct hn_softc *sc)
        hv_promisc_mode = 1;
 
        ret = hv_rf_on_open(sc);
-       if (ret != 0) {
+       if (ret != 0)
                return;
-       } else {
-               sc->hn_initdone = 1;
-       }
 
        atomic_clear_int(&ifp->if_drv_flags, IFF_DRV_OACTIVE);
        for (i = 0; i < sc->hn_tx_ring_inuse; ++i)
@@ -1813,27 +1752,14 @@ hn_init_locked(struct hn_softc *sc)
        if_link_state_change(ifp, LINK_STATE_UP);
 }
 
-/*
- *
- */
 static void
 hn_init(void *xsc)
 {
        struct hn_softc *sc = xsc;
 
-       NV_LOCK(sc);
-       if (sc->temp_unusable) {
-               NV_UNLOCK(sc);
-               return;
-       }
-       sc->temp_unusable = TRUE;
-       NV_UNLOCK(sc);
-
+       HN_LOCK(sc);
        hn_init_locked(sc);
-
-       NV_LOCK(sc);
-       sc->temp_unusable = FALSE;
-       NV_UNLOCK(sc);
+       HN_UNLOCK(sc);
 }
 
 #ifdef LATER
@@ -1864,13 +1790,15 @@ hn_lro_lenlim_sysctl(SYSCTL_HANDLER_ARGS
        if (error || req->newptr == NULL)
                return error;
 
+       HN_LOCK(sc);
        if (lenlim < HN_LRO_LENLIM_MIN(sc->hn_ifp) ||
-           lenlim > TCP_LRO_LENGTH_MAX)
+           lenlim > TCP_LRO_LENGTH_MAX) {
+               HN_UNLOCK(sc);
                return EINVAL;
-
-       NV_LOCK(sc);
+       }
        hn_set_lro_lenlim(sc, lenlim);
-       NV_UNLOCK(sc);
+       HN_UNLOCK(sc);
+
        return 0;
 }
 
@@ -1897,10 +1825,10 @@ hn_lro_ackcnt_sysctl(SYSCTL_HANDLER_ARGS
         * count limit.
         */
        --ackcnt;
-       NV_LOCK(sc);
+       HN_LOCK(sc);
        for (i = 0; i < sc->hn_rx_ring_inuse; ++i)
                sc->hn_rx_ring[i].hn_lro.lro_ackcnt_lim = ackcnt;
-       NV_UNLOCK(sc);
+       HN_UNLOCK(sc);
        return 0;
 }
 
@@ -1921,7 +1849,7 @@ hn_trust_hcsum_sysctl(SYSCTL_HANDLER_ARG
        if (error || req->newptr == NULL)
                return error;
 
-       NV_LOCK(sc);
+       HN_LOCK(sc);
        for (i = 0; i < sc->hn_rx_ring_inuse; ++i) {
                struct hn_rx_ring *rxr = &sc->hn_rx_ring[i];
 
@@ -1930,7 +1858,7 @@ hn_trust_hcsum_sysctl(SYSCTL_HANDLER_ARG
                else
                        rxr->hn_trust_hcsum &= ~hcsum;
        }
-       NV_UNLOCK(sc);
+       HN_UNLOCK(sc);
        return 0;
 }
 
@@ -1948,7 +1876,9 @@ hn_chim_size_sysctl(SYSCTL_HANDLER_ARGS)
        if (chim_size > sc->hn_chim_szmax || chim_size <= 0)
                return EINVAL;
 
+       HN_LOCK(sc);
        hn_set_chim_size(sc, chim_size);
+       HN_UNLOCK(sc);
        return 0;
 }
 
@@ -2073,12 +2003,12 @@ hn_tx_conf_int_sysctl(SYSCTL_HANDLER_ARG
        if (error || req->newptr == NULL)
                return error;
 
-       NV_LOCK(sc);
+       HN_LOCK(sc);
        for (i = 0; i < sc->hn_tx_ring_inuse; ++i) {
                txr = &sc->hn_tx_ring[i];
                *((int *)((uint8_t *)txr + ofs)) = conf;
        }
-       NV_UNLOCK(sc);
+       HN_UNLOCK(sc);
 
        return 0;
 }
@@ -2732,10 +2662,8 @@ hn_set_chim_size(struct hn_softc *sc, in
 {
        int i;
 
-       NV_LOCK(sc);
        for (i = 0; i < sc->hn_tx_ring_inuse; ++i)
                sc->hn_tx_ring[i].hn_chim_size = chim_size;
-       NV_UNLOCK(sc);
 }
 
 static void
_______________________________________________
svn-src-stable-10@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-stable-10
To unsubscribe, send any mail to "svn-src-stable-10-unsubscr...@freebsd.org"

Reply via email to