From owner-svn-src-all@freebsd.org Mon Oct 17 08:45:00 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id BC2BAC14BEA; Mon, 17 Oct 2016 08:45:00 +0000 (UTC) (envelope-from sephe@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 99191172E; Mon, 17 Oct 2016 08:45:00 +0000 (UTC) (envelope-from sephe@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u9H8ixw0041220; Mon, 17 Oct 2016 08:44:59 GMT (envelope-from sephe@FreeBSD.org) Received: (from sephe@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u9H8ixG1041218; Mon, 17 Oct 2016 08:44:59 GMT (envelope-from sephe@FreeBSD.org) Message-Id: <201610170844.u9H8ixG1041218@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: sephe set sender to sephe@FreeBSD.org using -f From: Sepherosa Ziehau Date: Mon, 17 Oct 2016 08:44:59 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r307512 - stable/11/sys/dev/hyperv/netvsc X-SVN-Group: stable-11 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Oct 2016 08:45:00 -0000 Author: sephe Date: Mon Oct 17 08:44:59 2016 New Revision: 307512 URL: https://svnweb.freebsd.org/changeset/base/307512 Log: MFC 305794 hyperv/hn: Use sx for the main lock. Sponsored by: Microsoft Differential Revision: https://reviews.freebsd.org/D7870 Modified: stable/11/sys/dev/hyperv/netvsc/hv_net_vsc.h stable/11/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c Directory Properties: stable/11/ (props changed) Modified: stable/11/sys/dev/hyperv/netvsc/hv_net_vsc.h ============================================================================== --- stable/11/sys/dev/hyperv/netvsc/hv_net_vsc.h Mon Oct 17 08:42:54 2016 (r307511) +++ stable/11/sys/dev/hyperv/netvsc/hv_net_vsc.h Mon Oct 17 08:44:59 2016 (r307512) @@ -204,10 +204,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/11/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c ============================================================================== --- stable/11/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c Mon Oct 17 08:42:54 2016 (r307511) +++ stable/11/sys/dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c Mon Oct 17 08:44:59 2016 (r307512) @@ -195,21 +195,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 @@ -463,6 +454,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, @@ -484,7 +476,6 @@ netvsc_attach(device_t dev) } else { sc->hn_tx_taskq = hn_tx_taskq; } - NV_LOCK_INIT(sc, "NetVSCLock"); ifp = sc->hn_ifp = if_alloc(IFT_ETHER); ifp->if_softc = sc; @@ -669,6 +660,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); } @@ -1475,39 +1467,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; @@ -1516,40 +1496,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; } @@ -1569,29 +1527,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) { /* @@ -1620,14 +1560,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) { @@ -1663,7 +1602,7 @@ hn_ioctl(struct ifnet *ifp, u_long cmd, ifp->if_hwassist &= ~CSUM_IP6_TSO; } - NV_UNLOCK(sc); + HN_UNLOCK(sc); break; case SIOCADDMULTI: @@ -1694,6 +1633,8 @@ hn_stop(struct hn_softc *sc) struct ifnet *ifp; int ret, i; + HN_LOCK_ASSERT(sc); + ifp = sc->hn_ifp; if (bootverbose) @@ -1705,7 +1646,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); } @@ -1774,6 +1714,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) { @@ -1783,11 +1725,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) @@ -1797,27 +1736,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 @@ -1848,13 +1774,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; } @@ -1881,10 +1809,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; } @@ -1905,7 +1833,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]; @@ -1914,7 +1842,7 @@ hn_trust_hcsum_sysctl(SYSCTL_HANDLER_ARG else rxr->hn_trust_hcsum &= ~hcsum; } - NV_UNLOCK(sc); + HN_UNLOCK(sc); return 0; } @@ -1932,7 +1860,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; } @@ -2028,12 +1958,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; } @@ -2677,10 +2607,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