From owner-svn-src-user@FreeBSD.ORG Tue Nov 6 22:07:40 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 B0FBF210; Tue, 6 Nov 2012 22:07:40 +0000 (UTC) (envelope-from jfvogel@gmail.com) Received: from mail-vb0-f54.google.com (mail-vb0-f54.google.com [209.85.212.54]) by mx1.freebsd.org (Postfix) with ESMTP id 39C5D8FC18; Tue, 6 Nov 2012 22:07:39 +0000 (UTC) Received: by mail-vb0-f54.google.com with SMTP id l1so1183043vba.13 for ; Tue, 06 Nov 2012 14:07:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=924A7yxp5BydW0m2S9LABSZNbZDo+2xbvsw5/QNgQ6A=; b=VCs9gjBHuEYJ7y1jl6M8roDodcF/Op3i0QBHa5BOfxD2iuCgoJHP1RdSF557p5p6LW t/ix4PXVjIDwOd3x6E6WpY2f3u6UUcgdJqUkihIZZkVpVo2tLnr+fj1s2yBChpTRhkfa i5Bsj6xkt/DTsDetyqhQn2RrotxinSjL6RASf1rROcMjx0iAwpgY2VVH5Ll6Oqz1HlJM 5RCg6M709vewUM0CrilhH/5Ewih7cnKCVofZFQm6MJmEB8Y6ZSN87ZfdZpc3/UMR1kK9 IPmrtn95Bt9FXrYLk+YRqMyj9ZmvnAHg/kPtmo/dOG7kze7COwlQ7Eu9yw6KHo8XbziN DblQ== MIME-Version: 1.0 Received: by 10.52.180.40 with SMTP id dl8mr1976020vdc.51.1352239659168; Tue, 06 Nov 2012 14:07:39 -0800 (PST) Received: by 10.59.3.165 with HTTP; Tue, 6 Nov 2012 14:07:39 -0800 (PST) In-Reply-To: <20121106220952.GA32652@onelab2.iet.unipi.it> References: <201211061954.qA6JsOP3038450@svn.freebsd.org> <20121106220952.GA32652@onelab2.iet.unipi.it> Date: Tue, 6 Nov 2012 14:07:39 -0800 Message-ID: Subject: Re: svn commit: r242670 - user/andre/tcp_workqueue/sys/dev/e1000 From: Jack Vogel To: Luigi Rizzo Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.14 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 22:07:40 -0000 Its his own branch :) 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) >