From owner-svn-src-stable-7@FreeBSD.ORG Fri Jan 8 21:19:41 2010 Return-Path: Delivered-To: svn-src-stable-7@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B57671065672; Fri, 8 Jan 2010 21:19:41 +0000 (UTC) (envelope-from yongari@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id A38F98FC17; Fri, 8 Jan 2010 21:19:41 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o08LJfBO093998; Fri, 8 Jan 2010 21:19:41 GMT (envelope-from yongari@svn.freebsd.org) Received: (from yongari@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o08LJfAD093995; Fri, 8 Jan 2010 21:19:41 GMT (envelope-from yongari@svn.freebsd.org) Message-Id: <201001082119.o08LJfAD093995@svn.freebsd.org> From: Pyun YongHyeon Date: Fri, 8 Jan 2010 21:19:41 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org X-SVN-Group: stable-7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r201824 - stable/7/sys/dev/vge X-BeenThere: svn-src-stable-7@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for only the 7-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Jan 2010 21:19:41 -0000 Author: yongari Date: Fri Jan 8 21:19:41 2010 New Revision: 201824 URL: http://svn.freebsd.org/changeset/base/201824 Log: MFC r198987,199414,199543,200422 Partial merge r198987: Use device_printf() and if_printf() instead of printf() with an explicit unit number and remove 'unit' members from softc. Partial merge r199414: Use the bus_*() routines rather than bus_space_*() for register operations. r199543: Several fixes to this driver: - Overhaul the locking to avoid recursion and add missing locking in a few places. - Don't schedule a task to call vge_start() from contexts that are safe to call vge_start() directly. Just invoke the routine directly instead (this is what all of the other NIC drivers I am familiar with do). Note that vge(4) does not use an interrupt filter handler which is the primary reason some other drivers use tasks. - Add a new private timer to drive the watchdog timer instead of using if_watchdog and if_timer. - Fixup detach by calling ether_ifdetach() before stopping the interface. r200422: Remove driver lock assertion in MII register access. This change was made in r199543 to remove MTX_RECURSE. These routines can be called in device attach phase(e.g. mii_phy_probe()) so checking assertion here is not right as caller does not hold a driver lock. Modified: stable/7/sys/dev/vge/if_vge.c stable/7/sys/dev/vge/if_vgevar.h Directory Properties: stable/7/sys/ (props changed) stable/7/sys/cddl/contrib/opensolaris/ (props changed) stable/7/sys/contrib/dev/acpica/ (props changed) stable/7/sys/contrib/pf/ (props changed) Modified: stable/7/sys/dev/vge/if_vge.c ============================================================================== --- stable/7/sys/dev/vge/if_vge.c Fri Jan 8 21:15:09 2010 (r201823) +++ stable/7/sys/dev/vge/if_vge.c Fri Jan 8 21:19:41 2010 (r201824) @@ -93,7 +93,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include @@ -160,12 +159,13 @@ static void vge_rxeof (struct vge_softc static void vge_txeof (struct vge_softc *); static void vge_intr (void *); static void vge_tick (void *); -static void vge_tx_task (void *, int); static void vge_start (struct ifnet *); +static void vge_start_locked (struct ifnet *); static int vge_ioctl (struct ifnet *, u_long, caddr_t); static void vge_init (void *); +static void vge_init_locked (struct vge_softc *); static void vge_stop (struct vge_softc *); -static void vge_watchdog (struct ifnet *); +static void vge_watchdog (void *); static int vge_suspend (device_t); static int vge_resume (device_t); static int vge_shutdown (device_t); @@ -379,7 +379,6 @@ vge_miibus_readreg(dev, phy, reg) if (phy != (CSR_READ_1(sc, VGE_MIICFG) & 0x1F)) return(0); - VGE_LOCK(sc); vge_miipoll_stop(sc); /* Specify the register we want to read. */ @@ -401,7 +400,6 @@ vge_miibus_readreg(dev, phy, reg) rval = CSR_READ_2(sc, VGE_MIIDATA); vge_miipoll_start(sc); - VGE_UNLOCK(sc); return (rval); } @@ -419,7 +417,6 @@ vge_miibus_writereg(dev, phy, reg, data) if (phy != (CSR_READ_1(sc, VGE_MIICFG) & 0x1F)) return(0); - VGE_LOCK(sc); vge_miipoll_stop(sc); /* Specify the register we want to write. */ @@ -444,7 +441,6 @@ vge_miibus_writereg(dev, phy, reg, data) } vge_miipoll_start(sc); - VGE_UNLOCK(sc); return (rval); } @@ -932,7 +928,9 @@ vge_attach(dev) sc->vge_dev = dev; mtx_init(&sc->vge_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK, - MTX_DEF | MTX_RECURSE); + MTX_DEF); + callout_init_mtx(&sc->vge_watchdog, &sc->vge_mtx, 0); + /* * Map control/status registers. */ @@ -948,9 +946,6 @@ vge_attach(dev) goto fail; } - sc->vge_btag = rman_get_bustag(sc->vge_res); - sc->vge_bhandle = rman_get_bushandle(sc->vge_res); - /* Allocate interrupt */ rid = 0; sc->vge_irq = bus_alloc_resource(dev, SYS_RES_IRQ, &rid, @@ -970,8 +965,6 @@ vge_attach(dev) */ vge_read_eeprom(sc, (caddr_t)eaddr, VGE_EE_EADDR, 3, 0); - sc->vge_unit = unit; - /* * Allocate the parent bus DMA tag appropriate for PCI. */ @@ -996,7 +989,7 @@ vge_attach(dev) ifp = sc->vge_ifp = if_alloc(IFT_ETHER); if (ifp == NULL) { - printf("vge%d: can not if_alloc()\n", sc->vge_unit); + device_printf(dev, "can not if_alloc()\n"); error = ENOSPC; goto fail; } @@ -1004,7 +997,7 @@ vge_attach(dev) /* Do MII setup */ if (mii_phy_probe(dev, &sc->vge_miibus, vge_ifmedia_upd, vge_ifmedia_sts)) { - printf("vge%d: MII without any phy!\n", sc->vge_unit); + device_printf(dev, "MII without any phy!\n"); error = ENXIO; goto fail; } @@ -1022,14 +1015,11 @@ vge_attach(dev) #ifdef DEVICE_POLLING ifp->if_capabilities |= IFCAP_POLLING; #endif - ifp->if_watchdog = vge_watchdog; ifp->if_init = vge_init; IFQ_SET_MAXLEN(&ifp->if_snd, VGE_IFQ_MAXLEN); ifp->if_snd.ifq_drv_maxlen = VGE_IFQ_MAXLEN; IFQ_SET_READY(&ifp->if_snd); - TASK_INIT(&sc->vge_txtask, 0, vge_tx_task, ifp); - /* * Call MI attach routine. */ @@ -1078,21 +1068,11 @@ vge_detach(dev) /* These should only be active if attach succeeded */ if (device_is_attached(dev)) { - vge_stop(sc); - /* - * Force off the IFF_UP flag here, in case someone - * still had a BPF descriptor attached to this - * interface. If they do, ether_ifattach() will cause - * the BPF code to try and clear the promisc mode - * flag, which will bubble down to vge_ioctl(), - * which will try to call vge_init() again. This will - * turn the NIC back on and restart the MII ticker, - * which will panic the system when the kernel tries - * to invoke the vge_tick() function that isn't there - * anymore. - */ - ifp->if_flags &= ~IFF_UP; ether_ifdetach(ifp); + VGE_LOCK(sc); + vge_stop(sc); + VGE_UNLOCK(sc); + callout_drain(&sc->vge_watchdog); } if (sc->vge_miibus) device_delete_child(dev, sc->vge_miibus); @@ -1531,7 +1511,7 @@ vge_txeof(sc) if (idx != sc->vge_ldata.vge_tx_considx) { sc->vge_ldata.vge_tx_considx = idx; ifp->if_drv_flags &= ~IFF_DRV_OACTIVE; - ifp->if_timer = 0; + sc->vge_timer = 0; } /* @@ -1557,7 +1537,7 @@ vge_tick(xsc) sc = xsc; ifp = sc->vge_ifp; - VGE_LOCK(sc); + VGE_LOCK_ASSERT(sc); mii = device_get_softc(sc->vge_miibus); mii_tick(mii); @@ -1574,13 +1554,10 @@ vge_tick(xsc) if_link_state_change(sc->vge_ifp, LINK_STATE_UP); if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) - taskqueue_enqueue(taskqueue_swi, - &sc->vge_txtask); + vge_start_locked(ifp); } } - VGE_UNLOCK(sc); - return; } @@ -1599,7 +1576,7 @@ vge_poll (struct ifnet *ifp, enum poll_c vge_txeof(sc); if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) - taskqueue_enqueue(taskqueue_swi, &sc->vge_txtask); + vge_start_locked(ifp); if (cmd == POLL_AND_CHECK_STATUS) { /* also check status register */ u_int32_t status; @@ -1615,7 +1592,7 @@ vge_poll (struct ifnet *ifp, enum poll_c if (status & VGE_ISR_TXDMA_STALL || status & VGE_ISR_RXDMA_STALL) - vge_init(sc); + vge_init_locked(sc); if (status & (VGE_ISR_RXOFLOW|VGE_ISR_RXNODESC)) { vge_rxeof(sc); @@ -1687,7 +1664,7 @@ vge_intr(arg) vge_txeof(sc); if (status & (VGE_ISR_TXDMA_STALL|VGE_ISR_RXDMA_STALL)) - vge_init(sc); + vge_init_locked(sc); if (status & VGE_ISR_LINKSTS) vge_tick(sc); @@ -1696,10 +1673,10 @@ vge_intr(arg) /* Re-enable interrupts */ CSR_WRITE_1(sc, VGE_CRS3, VGE_CR3_INT_GMSK); - VGE_UNLOCK(sc); - if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) - taskqueue_enqueue(taskqueue_swi, &sc->vge_txtask); + vge_start_locked(ifp); + + VGE_UNLOCK(sc); return; } @@ -1737,8 +1714,7 @@ vge_encap(sc, m_head, idx) m_head, vge_dma_map_tx_desc, &arg, BUS_DMA_NOWAIT); if (error && error != EFBIG) { - printf("vge%d: can't map mbuf (error %d)\n", - sc->vge_unit, error); + if_printf(sc->vge_ifp, "can't map mbuf (error %d)\n", error); return (ENOBUFS); } @@ -1759,8 +1735,8 @@ vge_encap(sc, m_head, idx) error = bus_dmamap_load_mbuf(sc->vge_ldata.vge_mtag, map, m_head, vge_dma_map_tx_desc, &arg, BUS_DMA_NOWAIT); if (error) { - printf("vge%d: can't map mbuf (error %d)\n", - sc->vge_unit, error); + if_printf(sc->vge_ifp, "can't map mbuf (error %d)\n", + error); return (EFBIG); } } @@ -1781,19 +1757,6 @@ vge_encap(sc, m_head, idx) return (0); } -static void -vge_tx_task(arg, npending) - void *arg; - int npending; -{ - struct ifnet *ifp; - - ifp = arg; - vge_start(ifp); - - return; -} - /* * Main transmit routine. */ @@ -1803,21 +1766,29 @@ vge_start(ifp) struct ifnet *ifp; { struct vge_softc *sc; + + sc = ifp->if_softc; + VGE_LOCK(sc); + vge_start_locked(ifp); + VGE_UNLOCK(sc); +} + +static void +vge_start_locked(ifp) + struct ifnet *ifp; +{ + struct vge_softc *sc; struct mbuf *m_head = NULL; int idx, pidx = 0; sc = ifp->if_softc; - VGE_LOCK(sc); + VGE_LOCK_ASSERT(sc); - if (!sc->vge_link || ifp->if_drv_flags & IFF_DRV_OACTIVE) { - VGE_UNLOCK(sc); + if (!sc->vge_link || ifp->if_drv_flags & IFF_DRV_OACTIVE) return; - } - if (IFQ_DRV_IS_EMPTY(&ifp->if_snd)) { - VGE_UNLOCK(sc); + if (IFQ_DRV_IS_EMPTY(&ifp->if_snd)) return; - } idx = sc->vge_ldata.vge_tx_prodidx; @@ -1850,10 +1821,8 @@ vge_start(ifp) ETHER_BPF_MTAP(ifp, m_head); } - if (idx == sc->vge_ldata.vge_tx_prodidx) { - VGE_UNLOCK(sc); + if (idx == sc->vge_ldata.vge_tx_prodidx) return; - } /* Flush the TX descriptors */ @@ -1877,12 +1846,10 @@ vge_start(ifp) */ CSR_WRITE_1(sc, VGE_CRS1, VGE_CR1_TIMER0_ENABLE); - VGE_UNLOCK(sc); - /* * Set a timeout in case the chip goes out to lunch. */ - ifp->if_timer = 5; + sc->vge_timer = 5; return; } @@ -1892,11 +1859,20 @@ vge_init(xsc) void *xsc; { struct vge_softc *sc = xsc; + + VGE_LOCK(sc); + vge_init_locked(sc); + VGE_UNLOCK(sc); +} + +static void +vge_init_locked(struct vge_softc *sc) +{ struct ifnet *ifp = sc->vge_ifp; struct mii_data *mii; int i; - VGE_LOCK(sc); + VGE_LOCK_ASSERT(sc); mii = device_get_softc(sc->vge_miibus); /* @@ -2052,12 +2028,11 @@ vge_init(xsc) ifp->if_drv_flags |= IFF_DRV_RUNNING; ifp->if_drv_flags &= ~IFF_DRV_OACTIVE; + callout_reset(&sc->vge_watchdog, hz, vge_watchdog, sc); sc->vge_if_flags = 0; sc->vge_link = 0; - VGE_UNLOCK(sc); - return; } @@ -2094,7 +2069,9 @@ vge_ifmedia_sts(ifp, ifmr) sc = ifp->if_softc; mii = device_get_softc(sc->vge_miibus); + VGE_LOCK(sc); mii_pollstat(mii); + VGE_UNLOCK(sc); ifmr->ifm_active = mii->mii_media_active; ifmr->ifm_status = mii->mii_media_status; @@ -2169,6 +2146,7 @@ vge_ioctl(ifp, command, data) ifp->if_mtu = ifr->ifr_mtu; break; case SIOCSIFFLAGS: + VGE_LOCK(sc); if (ifp->if_flags & IFF_UP) { if (ifp->if_drv_flags & IFF_DRV_RUNNING && ifp->if_flags & IFF_PROMISC && @@ -2183,16 +2161,19 @@ vge_ioctl(ifp, command, data) VGE_RXCTL_RX_PROMISC); vge_setmulti(sc); } else - vge_init(sc); + vge_init_locked(sc); } else { if (ifp->if_drv_flags & IFF_DRV_RUNNING) vge_stop(sc); } sc->vge_if_flags = ifp->if_flags; + VGE_UNLOCK(sc); break; case SIOCADDMULTI: case SIOCDELMULTI: + VGE_LOCK(sc); vge_setmulti(sc); + VGE_UNLOCK(sc); break; case SIOCGIFMEDIA: case SIOCSIFMEDIA: @@ -2226,6 +2207,7 @@ vge_ioctl(ifp, command, data) } } #endif /* DEVICE_POLLING */ + VGE_LOCK(sc); if ((mask & IFCAP_TXCSUM) != 0 && (ifp->if_capabilities & IFCAP_TXCSUM) != 0) { ifp->if_capenable ^= IFCAP_TXCSUM; @@ -2237,6 +2219,7 @@ vge_ioctl(ifp, command, data) if ((mask & IFCAP_RXCSUM) != 0 && (ifp->if_capabilities & IFCAP_RXCSUM) != 0) ifp->if_capenable ^= IFCAP_RXCSUM; + VGE_UNLOCK(sc); } break; default: @@ -2248,22 +2231,25 @@ vge_ioctl(ifp, command, data) } static void -vge_watchdog(ifp) - struct ifnet *ifp; +vge_watchdog(void *arg) { - struct vge_softc *sc; + struct vge_softc *sc; + struct ifnet *ifp; - sc = ifp->if_softc; - VGE_LOCK(sc); - printf("vge%d: watchdog timeout\n", sc->vge_unit); + sc = arg; + VGE_LOCK_ASSERT(sc); + callout_reset(&sc->vge_watchdog, hz, vge_watchdog, sc); + if (sc->vge_timer == 0 || --sc->vge_timer > 0) + return; + + ifp = sc->vge_ifp; + if_printf(ifp, "watchdog timeout\n"); ifp->if_oerrors++; vge_txeof(sc); vge_rxeof(sc); - vge_init(sc); - - VGE_UNLOCK(sc); + vge_init_locked(sc); return; } @@ -2279,9 +2265,10 @@ vge_stop(sc) register int i; struct ifnet *ifp; - VGE_LOCK(sc); + VGE_LOCK_ASSERT(sc); ifp = sc->vge_ifp; - ifp->if_timer = 0; + sc->vge_timer = 0; + callout_stop(&sc->vge_watchdog); ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE); @@ -2319,8 +2306,6 @@ vge_stop(sc) } } - VGE_UNLOCK(sc); - return; } @@ -2337,9 +2322,11 @@ vge_suspend(dev) sc = device_get_softc(dev); + VGE_LOCK(sc); vge_stop(sc); sc->suspended = 1; + VGE_UNLOCK(sc); return (0); } @@ -2364,10 +2351,12 @@ vge_resume(dev) pci_enable_io(dev, SYS_RES_MEMORY); /* reinitialize interface if necessary */ + VGE_LOCK(sc); if (ifp->if_flags & IFF_UP) - vge_init(sc); + vge_init_locked(sc); sc->suspended = 0; + VGE_UNLOCK(sc); return (0); } @@ -2384,7 +2373,9 @@ vge_shutdown(dev) sc = device_get_softc(dev); + VGE_LOCK(sc); vge_stop(sc); + VGE_UNLOCK(sc); return (0); } Modified: stable/7/sys/dev/vge/if_vgevar.h ============================================================================== --- stable/7/sys/dev/vge/if_vgevar.h Fri Jan 8 21:15:09 2010 (r201823) +++ stable/7/sys/dev/vge/if_vgevar.h Fri Jan 8 21:19:41 2010 (r201824) @@ -100,22 +100,20 @@ struct vge_list_data { struct vge_softc { struct ifnet *vge_ifp; /* interface info */ device_t vge_dev; - bus_space_handle_t vge_bhandle; /* bus space handle */ - bus_space_tag_t vge_btag; /* bus space tag */ struct resource *vge_res; struct resource *vge_irq; void *vge_intrhand; device_t vge_miibus; bus_dma_tag_t vge_parent_tag; bus_dma_tag_t vge_tag; - u_int8_t vge_unit; /* interface number */ u_int8_t vge_type; int vge_if_flags; int vge_rx_consumed; int vge_link; int vge_camidx; - struct task vge_txtask; struct mtx vge_mtx; + struct callout vge_watchdog; + int vge_timer; struct mbuf *vge_head; struct mbuf *vge_tail; @@ -135,20 +133,20 @@ struct vge_softc { * register space access macros */ #define CSR_WRITE_STREAM_4(sc, reg, val) \ - bus_space_write_stream_4(sc->vge_btag, sc->vge_bhandle, reg, val) + bus_write_stream_4(sc->vge_res, reg, val) #define CSR_WRITE_4(sc, reg, val) \ - bus_space_write_4(sc->vge_btag, sc->vge_bhandle, reg, val) + bus_write_4(sc->vge_res, reg, val) #define CSR_WRITE_2(sc, reg, val) \ - bus_space_write_2(sc->vge_btag, sc->vge_bhandle, reg, val) + bus_write_2(sc->vge_res, reg, val) #define CSR_WRITE_1(sc, reg, val) \ - bus_space_write_1(sc->vge_btag, sc->vge_bhandle, reg, val) + bus_write_1(sc->vge_res, reg, val) #define CSR_READ_4(sc, reg) \ - bus_space_read_4(sc->vge_btag, sc->vge_bhandle, reg) + bus_read_4(sc->vge_res, reg) #define CSR_READ_2(sc, reg) \ - bus_space_read_2(sc->vge_btag, sc->vge_bhandle, reg) + bus_read_2(sc->vge_res, reg) #define CSR_READ_1(sc, reg) \ - bus_space_read_1(sc->vge_btag, sc->vge_bhandle, reg) + bus_read_1(sc->vge_res, reg) #define CSR_SETBIT_1(sc, reg, x) \ CSR_WRITE_1(sc, reg, CSR_READ_1(sc, reg) | (x))