Date: Tue, 6 Nov 2012 23:09:52 +0100 From: Luigi Rizzo <rizzo@iet.unipi.it> To: Andre Oppermann <andre@FreeBSD.org> Cc: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: Re: svn commit: r242670 - user/andre/tcp_workqueue/sys/dev/e1000 Message-ID: <20121106220952.GA32652@onelab2.iet.unipi.it> In-Reply-To: <201211061954.qA6JsOP3038450@svn.freebsd.org> References: <201211061954.qA6JsOP3038450@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
One thing: while i believe device polling should go, removing the conditional blocks from device drivers is both useless and a mistake as it gratuitously introduces disalignment between device drivers. I suggest to revert this (and similar) commits. The code is already conditional, and it suffices to remove the option from files/options and if you want to remove the DEVICE_POLLING from the main code. cheers luigi On Tue, Nov 06, 2012 at 07:54:24PM +0000, Andre Oppermann wrote: > Author: andre > Date: Tue Nov 6 19:54:24 2012 > New Revision: 242670 > URL: http://svnweb.freebsd.org/changeset/base/242670 > > Log: > Remove polling support from em in preparation to try a different > approach. > > Modified: > user/andre/tcp_workqueue/sys/dev/e1000/if_em.c > > Modified: user/andre/tcp_workqueue/sys/dev/e1000/if_em.c > ============================================================================== > --- user/andre/tcp_workqueue/sys/dev/e1000/if_em.c Tue Nov 6 19:51:54 2012 (r242669) > +++ user/andre/tcp_workqueue/sys/dev/e1000/if_em.c Tue Nov 6 19:54:24 2012 (r242670) > @@ -33,7 +33,6 @@ > /*$FreeBSD$*/ > > #ifdef HAVE_KERNEL_OPTION_HEADERS > -#include "opt_device_polling.h" > #include "opt_inet.h" > #include "opt_inet6.h" > #endif > @@ -293,10 +292,6 @@ static int em_sysctl_eee(SYSCTL_HANDLER_ > > static __inline void em_rx_discard(struct rx_ring *, int); > > -#ifdef DEVICE_POLLING > -static poll_handler_t em_poll; > -#endif /* POLLING */ > - > /********************************************************************* > * FreeBSD Device Interface Entry Points > *********************************************************************/ > @@ -772,11 +767,6 @@ em_detach(device_t dev) > return (EBUSY); > } > > -#ifdef DEVICE_POLLING > - if (ifp->if_capenable & IFCAP_POLLING) > - ether_poll_deregister(ifp); > -#endif > - > if (adapter->led_dev != NULL) > led_destroy(adapter->led_dev); > > @@ -1162,10 +1152,7 @@ em_ioctl(struct ifnet *ifp, u_long comma > EM_CORE_LOCK(adapter); > em_disable_intr(adapter); > em_set_multi(adapter); > -#ifdef DEVICE_POLLING > - if (!(ifp->if_capenable & IFCAP_POLLING)) > -#endif > - em_enable_intr(adapter); > + em_enable_intr(adapter); > EM_CORE_UNLOCK(adapter); > } > break; > @@ -1192,26 +1179,7 @@ em_ioctl(struct ifnet *ifp, u_long comma > IOCTL_DEBUGOUT("ioctl rcv'd: SIOCSIFCAP (Set Capabilities)"); > reinit = 0; > mask = ifr->ifr_reqcap ^ ifp->if_capenable; > -#ifdef DEVICE_POLLING > - if (mask & IFCAP_POLLING) { > - if (ifr->ifr_reqcap & IFCAP_POLLING) { > - error = ether_poll_register(em_poll, ifp); > - if (error) > - return (error); > - EM_CORE_LOCK(adapter); > - em_disable_intr(adapter); > - ifp->if_capenable |= IFCAP_POLLING; > - EM_CORE_UNLOCK(adapter); > - } else { > - error = ether_poll_deregister(ifp); > - /* Enable interrupt even in error case */ > - EM_CORE_LOCK(adapter); > - em_enable_intr(adapter); > - ifp->if_capenable &= ~IFCAP_POLLING; > - EM_CORE_UNLOCK(adapter); > - } > - } > -#endif > + > if (mask & IFCAP_HWCSUM) { > ifp->if_capenable ^= IFCAP_HWCSUM; > reinit = 1; > @@ -1373,16 +1341,7 @@ em_init_locked(struct adapter *adapter) > E1000_WRITE_REG(&adapter->hw, E1000_IVAR, adapter->ivars); > } > > -#ifdef DEVICE_POLLING > - /* > - * Only enable interrupts if we are not polling, make sure > - * they are off otherwise. > - */ > - if (ifp->if_capenable & IFCAP_POLLING) > - em_disable_intr(adapter); > - else > -#endif /* DEVICE_POLLING */ > - em_enable_intr(adapter); > + em_enable_intr(adapter); > > /* AMT based hardware can now take control from firmware */ > if (adapter->has_manage && adapter->has_amt) > @@ -1399,58 +1358,6 @@ em_init(void *arg) > EM_CORE_UNLOCK(adapter); > } > > - > -#ifdef DEVICE_POLLING > -/********************************************************************* > - * > - * Legacy polling routine: note this only works with single queue > - * > - *********************************************************************/ > -static int > -em_poll(struct ifnet *ifp, enum poll_cmd cmd, int count) > -{ > - struct adapter *adapter = ifp->if_softc; > - struct tx_ring *txr = adapter->tx_rings; > - struct rx_ring *rxr = adapter->rx_rings; > - u32 reg_icr; > - int rx_done; > - > - EM_CORE_LOCK(adapter); > - if ((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) { > - EM_CORE_UNLOCK(adapter); > - return (0); > - } > - > - if (cmd == POLL_AND_CHECK_STATUS) { > - reg_icr = E1000_READ_REG(&adapter->hw, E1000_ICR); > - if (reg_icr & (E1000_ICR_RXSEQ | E1000_ICR_LSC)) { > - callout_stop(&adapter->timer); > - adapter->hw.mac.get_link_status = 1; > - em_update_link_status(adapter); > - callout_reset(&adapter->timer, hz, > - em_local_timer, adapter); > - } > - } > - EM_CORE_UNLOCK(adapter); > - > - em_rxeof(rxr, count, &rx_done); > - > - EM_TX_LOCK(txr); > - em_txeof(txr); > -#ifdef EM_MULTIQUEUE > - if (!drbr_empty(ifp, txr->br)) > - em_mq_start_locked(ifp, txr, NULL); > -#else > - if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) > - em_start_locked(ifp, txr); > -#endif > - EM_TX_UNLOCK(txr); > - > - return (rx_done); > -} > -#endif /* DEVICE_POLLING */ > - > - > /********************************************************************* > * > * Fast Legacy/MSI Combined Interrupt Service routine > @@ -2256,10 +2163,8 @@ em_local_timer(void *arg) > > adapter->pause_frames = 0; > callout_reset(&adapter->timer, hz, em_local_timer, adapter); > -#ifndef DEVICE_POLLING > /* Trigger an RX interrupt to guarantee mbuf refresh */ > E1000_WRITE_REG(&adapter->hw, E1000_ICS, trigger); > -#endif > return; > hung: > /* Looks like we're hung */ > @@ -2980,10 +2885,6 @@ em_setup_interface(device_t dev, struct > */ > ifp->if_capabilities |= IFCAP_VLAN_HWFILTER; > > -#ifdef DEVICE_POLLING > - ifp->if_capabilities |= IFCAP_POLLING; > -#endif > - > /* Enable only WOL MAGIC by default */ > if (adapter->wol) { > ifp->if_capabilities |= IFCAP_WOL; > @@ -4388,7 +4289,6 @@ em_initialize_receive_unit(struct adapte > * We loop at most count times if count is > 0, or until done if > * count < 0. > * > - * For polling we also now return the number of cleaned packets > *********************************************************************/ > static bool > em_rxeof(struct rx_ring *rxr, int count, int *done)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121106220952.GA32652>