Date: Tue, 17 Aug 2004 00:24:38 -0700 From: John-Mark Gurney <gurney_j@resnet.uoregon.edu> To: freebsd-current@freebsd.org Subject: new if_sk locking patch... Message-ID: <20040817072438.GA99980@funkthat.com>
next in thread | raw e-mail | index | archive | help
--YZ5djTAD1cGYuMQK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Ok, I just happen to pick up a Belkin F5D5005 Gige card. (I couldn't resist at $25.) They were nice enough to include Linux drivers on the CD (yeh!) which made it much easier for me to find out what driver it was compatible with. It didn't take much time to get the card working. Of course, once I had the card working I was getting soooo many LOR's that my serial console was overloaded. I took Doug White's patch and modified it a little. I left in the recursion since I wanted to get it working first. So, here it is attached, and availabe at: http://people.freebsd.org/~jmg/if_sk.diff I don't get any LOR's w/ this patch and debug.mpsafenet=1. I didn't try using it as a module though. (Hence I don't know if kldload/kldunload works.) Test it out and let me know how it works for you. Thanks! -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not." --YZ5djTAD1cGYuMQK Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="if_sk.diff" Index: if_sk.c =================================================================== RCS file: /home/ncvs/src/sys/pci/if_sk.c,v retrieving revision 1.83 diff -u -r1.83 if_sk.c --- if_sk.c 2004/06/28 20:07:03 1.83 +++ if_sk.c 2004/08/17 07:13:23 @@ -156,6 +156,11 @@ "Marvell Gigabit Ethernet" }, { + VENDORID_MARVELL, + DEVICEID_BELKIN_5005, + "Belkin F5D5005 Gigabit Ethernet" + }, + { VENDORID_3COM, DEVICEID_3COM_3C940, "3Com 3C940 Gigabit Ethernet" @@ -1332,7 +1337,6 @@ error = 0; sc_if = device_get_softc(dev); sc = device_get_softc(device_get_parent(dev)); - SK_LOCK(sc); port = *(int *)device_get_ivars(dev); free(device_get_ivars(dev), M_DEVBUF); device_set_ivars(dev, NULL); @@ -1347,6 +1351,40 @@ if (port == SK_PORT_B) sc_if->sk_tx_bmu = SK_BMU_TXS_CSR1; + /* Allocate the descriptor queues. */ + sc_if->sk_rdata = contigmalloc(sizeof(struct sk_ring_data), M_DEVBUF, + M_NOWAIT, 0, 0xffffffff, PAGE_SIZE, 0); + + if (sc_if->sk_rdata == NULL) { + printf("sk%d: no memory for list buffers!\n", sc_if->sk_unit); + error = ENOMEM; + goto fail; + } + + bzero(sc_if->sk_rdata, sizeof(struct sk_ring_data)); + + /* Try to allocate memory for jumbo buffers. */ + if (sk_alloc_jumbo_mem(sc_if)) { + printf("sk%d: jumbo buffer allocation failed\n", + sc_if->sk_unit); + error = ENOMEM; + goto fail; + } + + ifp = &sc_if->arpcom.ac_if; + ifp->if_softc = sc_if; + if_initname(ifp, device_get_name(dev), device_get_unit(dev)); + ifp->if_mtu = ETHERMTU; + ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST; + ifp->if_ioctl = sk_ioctl; + ifp->if_start = sk_start; + ifp->if_watchdog = sk_watchdog; + ifp->if_init = sk_init; + ifp->if_baudrate = 1000000000; + ifp->if_snd.ifq_maxlen = SK_TX_RING_CNT - 1; + + callout_handle_init(&sc_if->sk_tick_ch); + /* * Get station address for this interface. Note that * dual port cards actually come with three station @@ -1356,6 +1394,7 @@ * are operating in failover mode. Currently we don't * use this extra address. */ + SK_LOCK(sc); for (i = 0; i < ETHER_ADDR_LEN; i++) sc_if->arpcom.ac_enaddr[i] = sk_win_read_1(sc, SK_MAC0_0 + (port * 8) + i); @@ -1409,48 +1448,26 @@ printf("skc%d: unsupported PHY type: %d\n", sc->sk_unit, sc_if->sk_phytype); error = ENODEV; - goto fail; - } - - /* Allocate the descriptor queues. */ - sc_if->sk_rdata = contigmalloc(sizeof(struct sk_ring_data), M_DEVBUF, - M_NOWAIT, 0, 0xffffffff, PAGE_SIZE, 0); - - if (sc_if->sk_rdata == NULL) { - printf("sk%d: no memory for list buffers!\n", sc_if->sk_unit); - error = ENOMEM; - goto fail; - } - - bzero(sc_if->sk_rdata, sizeof(struct sk_ring_data)); - - /* Try to allocate memory for jumbo buffers. */ - if (sk_alloc_jumbo_mem(sc_if)) { - printf("sk%d: jumbo buffer allocation failed\n", - sc_if->sk_unit); - error = ENOMEM; + SK_UNLOCK(sc); goto fail; } - ifp = &sc_if->arpcom.ac_if; - ifp->if_softc = sc_if; - if_initname(ifp, device_get_name(dev), device_get_unit(dev)); - ifp->if_mtu = ETHERMTU; - ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST; - ifp->if_ioctl = sk_ioctl; - ifp->if_start = sk_start; - ifp->if_watchdog = sk_watchdog; - ifp->if_init = sk_init; - ifp->if_baudrate = 1000000000; - ifp->if_snd.ifq_maxlen = SK_TX_RING_CNT - 1; - - callout_handle_init(&sc_if->sk_tick_ch); + /* XXX dwhite + * Drop interface lock around ether_ifattach, which + * cannot be called with locks held. + */ + SK_UNLOCK(sc); /* * Call MI attach routine. */ ether_ifattach(ifp, sc_if->arpcom.ac_enaddr); + /* XXX dwhite + * Pick the lock back up. + */ + SK_LOCK(sc); + /* * Do miibus setup. */ @@ -1463,16 +1480,15 @@ break; } + SK_UNLOCK(sc); if (mii_phy_probe(dev, &sc_if->sk_miibus, sk_ifmedia_upd, sk_ifmedia_sts)) { printf("skc%d: no PHY found!\n", sc_if->sk_unit); ether_ifdetach(ifp); error = ENXIO; - goto fail; } fail: - SK_UNLOCK(sc); if (error) { /* Access should be ok even though lock has been dropped */ sc->sk_if[port] = NULL; @@ -1532,6 +1548,7 @@ sc->sk_type = SK_GENESIS; break; case DEVICEID_SK_V2: + case DEVICEID_BELKIN_5005: case DEVICEID_3COM_3C940: case DEVICEID_LINKSYS_EG1032: case DEVICEID_DLINK_DGE530T: @@ -1623,7 +1640,7 @@ bus_generic_attach(dev); /* Hook interrupt last to avoid having to lock softc */ - error = bus_setup_intr(dev, sc->sk_irq, INTR_TYPE_NET, + error = bus_setup_intr(dev, sc->sk_irq, INTR_TYPE_NET|INTR_MPSAFE, sk_intr, sc, &sc->sk_intrhand); if (error) { @@ -1661,10 +1678,22 @@ /* These should only be active if attach_xmac succeeded */ if (device_is_attached(dev)) { sk_stop(sc_if); + /* XXX dwhite + * Can't hold locks while calling detach + */ + SK_IF_UNLOCK(sc_if); ether_ifdetach(ifp); + SK_IF_LOCK(sc_if); } + /* XXX dwhite + * We're generally called from skc_detach() which is using + * device_delete_child() to get to here. It's already trashed + * miibus for us, so don't do it here or we'll panic. + */ + /* if (sc_if->sk_miibus) device_delete_child(dev, sc_if->sk_miibus); + */ bus_generic_detach(dev); if (sc_if->sk_cdata.sk_jumbo_buf) contigfree(sc_if->sk_cdata.sk_jumbo_buf, SK_JMEM, M_DEVBUF); @@ -1685,7 +1714,6 @@ sc = device_get_softc(dev); KASSERT(mtx_initialized(&sc->sk_mtx), ("sk mutex not initialized")); - SK_LOCK(sc); if (device_is_alive(dev)) { if (sc->sk_devs[SK_PORT_A] != NULL) @@ -1702,7 +1730,6 @@ if (sc->sk_res) bus_release_resource(dev, SK_RES, SK_RID, sc->sk_res); - SK_UNLOCK(sc); mtx_destroy(&sc->sk_mtx); return(0); @@ -2357,7 +2384,8 @@ return; } -static void sk_init_yukon(sc_if) +static void +sk_init_yukon(sc_if) struct sk_if_softc *sc_if; { u_int32_t phy; Index: if_skreg.h =================================================================== RCS file: /home/ncvs/src/sys/pci/if_skreg.h,v retrieving revision 1.20 diff -u -r1.20 if_skreg.h --- if_skreg.h 2004/03/31 12:35:51 1.20 +++ if_skreg.h 2004/08/17 07:13:24 @@ -71,6 +71,11 @@ #define DEVICEID_SK_V2 0x4320 /* + * Belkin F5D5005 + */ +#define DEVICEID_BELKIN_5005 0x5005 + +/* * 3Com PCI vendor ID */ #define VENDORID_3COM 0x10b7 @@ -1436,6 +1441,7 @@ #define SK_LOCK_ASSERT(_sc) mtx_assert(&(_sc)->sk_mtx, MA_OWNED) #define SK_IF_LOCK(_sc) mtx_lock(&(_sc)->sk_softc->sk_mtx) #define SK_IF_UNLOCK(_sc) mtx_unlock(&(_sc)->sk_softc->sk_mtx) +#define SK_IF_LOCK_ASSERT(_sc) mtx_assert(&(_sc)->sk_mtx, MA_OWNED) /* Softc for each logical interface */ struct sk_if_softc { --YZ5djTAD1cGYuMQK--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040817072438.GA99980>