From owner-svn-src-head@FreeBSD.ORG Tue Apr 21 10:51:11 2009 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 132321065674; Tue, 21 Apr 2009 10:51:11 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail11.syd.optusnet.com.au (mail11.syd.optusnet.com.au [211.29.132.192]) by mx1.freebsd.org (Postfix) with ESMTP id A03908FC16; Tue, 21 Apr 2009 10:51:10 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from besplex.bde.org (c122-107-120-227.carlnfd1.nsw.optusnet.com.au [122.107.120.227]) by mail11.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id n3LAp71H007956 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 21 Apr 2009 20:51:08 +1000 Date: Tue, 21 Apr 2009 20:51:07 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Robert Watson In-Reply-To: Message-ID: <20090421201148.V3341@besplex.bde.org> References: <200904210034.n3L0YV9i062814@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Pyun YongHyeon Subject: Re: svn commit: r191344 - head/sys/dev/xl X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Apr 2009 10:51:11 -0000 On Tue, 21 Apr 2009, Robert Watson wrote: > On Tue, 21 Apr 2009, Pyun YongHyeon wrote: > >> Clear IFF_DRV_OACTIVE flag if one of queued packets was transmitted. >> Previously it used to clear the flag only when the transmit queue >> is empty which may slow down Tx performance. >> While I'm here check whether driver is running and whether we can >> queue more packets in if_start handler. This fixes occasional >> watchdog timeouts. > > Historically, IFF_DRV_OACTIVE was present to allow the driver to prevent > calls to if_start when the driver was already in the transmit steady state > (i.e., interrupt-driven transmit). When the transmit interrupt saw it was > out of queued packets and interrupt-driven transmission was ending, it would > clear the flag so that future enqueues to the now-empty queue would generate > an if_start to trigger interrupt-driven transmission again. With that in > mind -- are you sure this is the right change? Shouldn't the descriptor ring > be refilled when an interrupt comes in from the device after an appropriate > interval? Yes. It was mostly or entirely broken drivers that didn't try to refill the tx ring _as soon as possible_ after a tx interrupt arrives. The tx interrupt should not arrive until the ring reaches a low watermark, which implies both that the ring should be refilled as soon as possible and that there is plenty of space in it so that attempts to refill it will not be completely wasted. The lowest-end hardware and/or drivers that have inadequate watermark handling are probably the ones that are harmed most by not refilling ASAP, since they probably have a tiny tx ring which gives tinier effective low watermark so the tx is more likely to run dry if you don't refill it ASAP. Another point here is that calling if_start() from the interrupt handler is fairly efficient. rxeof() routines could be smarter and only clear IFF_DRV_OACTIVE when the low watermark has been reached. I tried this in a couple of drivers and found it to be a waste of time, because for the hardware wasn't completely-lowest-end so the low watermark was reached in the usual case. The broken drivers here are ones that are dumber and only clear IFF_DRV_OACTIVE when their nonsense idea of a low watermark (0) is reached. Bruce