From owner-svn-src-user@FreeBSD.ORG Tue Nov 6 23:46:09 2012 Return-Path: Delivered-To: svn-src-user@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 06DF4F35; Tue, 6 Nov 2012 23:46:09 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id 60C5E8FC0C; Tue, 6 Nov 2012 23:46:07 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id 980347300A; Wed, 7 Nov 2012 01:07:07 +0100 (CET) Date: Wed, 7 Nov 2012 01:07:07 +0100 From: Luigi Rizzo To: Jack Vogel Subject: Re: svn commit: r242670 - user/andre/tcp_workqueue/sys/dev/e1000 Message-ID: <20121107000707.GB32652@onelab2.iet.unipi.it> References: <201211061954.qA6JsOP3038450@svn.freebsd.org> <20121106220952.GA32652@onelab2.iet.unipi.it> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i Cc: src-committers@freebsd.org, Andre Oppermann , svn-src-user@freebsd.org X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Nov 2012 23:46:09 -0000 On Tue, Nov 06, 2012 at 02:07:39PM -0800, Jack Vogel wrote: > Its his own branch :) true, and i totally missed this important detail :) But i do hope the removal lands into HEAD so i'd prefer that device drivers be left unchanged when this happens. cheers luigi > Jack > > > On Tue, Nov 6, 2012 at 2:09 PM, Luigi Rizzo wrote: > > > 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) > >