From owner-svn-src-all@FreeBSD.ORG Wed Apr 7 05:53:27 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6A021106564A; Wed, 7 Apr 2010 05:53:27 +0000 (UTC) (envelope-from Andre.Albsmeier@siemens.com) Received: from thoth.sbs.de (thoth.sbs.de [192.35.17.2]) by mx1.freebsd.org (Postfix) with ESMTP id DC2EE8FC20; Wed, 7 Apr 2010 05:53:26 +0000 (UTC) Received: from mail1.siemens.de (localhost [127.0.0.1]) by thoth.sbs.de (8.12.11.20060308/8.12.11) with ESMTP id o375YHMu010875; Wed, 7 Apr 2010 07:34:17 +0200 Received: from curry.mchp.siemens.de (curry.mchp.siemens.de [139.25.40.130]) by mail1.siemens.de (8.12.11.20060308/8.12.11) with ESMTP id o375YHl5008315; Wed, 7 Apr 2010 07:34:17 +0200 Received: (from localhost) by curry.mchp.siemens.de (8.14.4/8.14.4) id o375YH7t043189; Date: Wed, 7 Apr 2010 07:34:17 +0200 From: Andre Albsmeier To: Pyun YongHyeon Message-ID: <20100407053417.GB2899@curry.mchp.siemens.de> References: <201003241721.o2OHL5K9063538@svn.freebsd.org> <20100405145937.GA78871@curry.mchp.siemens.de> <20100405180642.GD1225@michelle.cdnetworks.com> <20100406134626.GA1727@curry.mchp.siemens.de> <20100406180027.GA3724@curry.mchp.siemens.de> <20100406184456.GA1087@michelle.cdnetworks.com> <20100406195936.GA48023@curry.mchp.siemens.de> <20100406202949.GB1087@michelle.cdnetworks.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100406202949.GB1087@michelle.cdnetworks.com> X-Echelon: X-Advice: Drop that crappy M$-Outlook, I'm tired of your viruses! User-Agent: Mutt/1.5.20 (2009-06-14) Cc: "svn-src-stable@freebsd.org" , "svn-src-all@freebsd.org" , "Albsmeier, Andre" , Pyun YongHyeon Subject: Re: svn commit: r205614 - stable/7/sys/dev/msk X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Apr 2010 05:53:27 -0000 On Tue, 06-Apr-2010 at 22:29:49 +0200, Pyun YongHyeon wrote: > On Tue, Apr 06, 2010 at 09:59:36PM +0200, Andre Albsmeier wrote: > > On Tue, 06-Apr-2010 at 20:44:56 +0200, Pyun YongHyeon wrote: > > > On Tue, Apr 06, 2010 at 08:00:27PM +0200, Andre Albsmeier wrote: > > > > On Tue, 06-Apr-2010 at 15:46:26 +0200, Andre Albsmeier wrote: > > > > > On Mon, 05-Apr-2010 at 20:06:42 +0200, Pyun YongHyeon wrote: > > > > > > [...] > > > > > > > > > As you know 1.18.2.38 removed taskqueue based interrupt handling so > > > > > > it could be culprit of the issue. But that revision also removed > > > > > > two register accesses in TX path so I'd like to know which one > > > > > > caused the issue. > > > > > > > > > > I have now tried rev. 1.18.2.38 with this patch (no idea if > > > > > this is right ;-)): > > > > > > > > > > --- if_msk.c.1.18.2.38 2010-04-06 15:09:19.000000000 +0200 > > > > > +++ if_msk.c.1.18.2.38.TRY 2010-04-06 15:38:13.000000000 +0200 > > > > > @@ -3327,6 +3327,11 @@ > > > > > uint32_t control, status; > > > > > int cons, len, port, rxprog; > > > > > > > > > > + int idx; > > > > > + idx = CSR_READ_2(sc, STAT_PUT_IDX); > > > > > + if (idx == sc->msk_stat_cons) > > > > > + return (0); > > > > > + > > > > > /* Sync status LEs. */ > > > > > bus_dmamap_sync(sc->msk_stat_tag, sc->msk_stat_map, > > > > > BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE); > > > > > @@ -3407,7 +3412,7 @@ > > > > > if (rxput[MSK_PORT_B] > 0) > > > > > msk_rxput(sc->msk_if[MSK_PORT_B]); > > > > > > > > > > - return (rxprog > sc->msk_process_limit ? EAGAIN : 0); > > > > > + return (sc->msk_stat_cons != CSR_READ_2(sc, STAT_PUT_IDX)); > > > > > } > > > > > > > > > > static void > > > > > > > > > > Now performance seems to be the same as with the older > > > > > driver (at least here at work) and in both directions! > > > > > Some numbers: > > > > > > > > > > em0 writes to rev. 1.18.2.36: 20 seconds > > > > > em0 writes to rev. 1.18.2.38: 50 seconds > > > > > em0 writes to rev. 1.18.2.38 with patch from above: 23 seconds > > > > > same as before but with int_holdoff: 100 -> 1: 20 seconds > > > > > > > > > > rev. 1.18.2.36 writes to em0: 22 seconds > > > > > rev. 1.18.2.38 writes to em0: 40 seconds > > > > > rev. 1.18.2.38 with patch from above writes to em0: 21 seconds > > > > > same as before but with int_holdoff: 100 -> 1: 20 seconds > > > > > > > > > > It seems that these two CSR_READ_2s really help ;-). > > > > > > > > > > As I said, this is at work and with slightly different machines. > > > > > I will try things at home later but I am rather confident of > > > > > receiving good results there as well... > > > > > > > > OK, tests at home show similar good results with the > > > > above patch. When setting int_holdoff to 3, performance > > > > seems equal to the older versions. > > > > > > > > > > Thanks a lot for narrowing down the issue. > > > Because the msk_handle_events() are called in interrupt handler > > > I'd like to remove the two register accesses in fast path. It seems > > > accessing STAT_PUT_IDX triggers status updates. Would you try > > > attached patch and let me know whether the patch makes any > > > difference? > > > > > Index: sys/dev/msk/if_msk.c > > > =================================================================== > > > --- sys/dev/msk/if_msk.c (revision 206204) > > > +++ sys/dev/msk/if_msk.c (working copy) > > > @@ -1470,6 +1470,7 @@ > > > /* WA for dev. #4.18 */ > > > CSR_WRITE_1(sc, STAT_FIFO_WM, 0x21); > > > CSR_WRITE_1(sc, STAT_FIFO_ISR_WM, 0x07); > > > + CSR_WRITE_4(sc, STAT_TX_TIMER_INI, MSK_USECS(sc, 10)); > > > } else { > > > CSR_WRITE_2(sc, STAT_TX_IDX_TH, 0x0a); > > > CSR_WRITE_1(sc, STAT_FIFO_WM, 0x10); > > > @@ -1481,9 +1482,16 @@ > > > CSR_WRITE_4(sc, STAT_ISR_TIMER_INI, 0x0190); > > > } > > > /* > > > - * Use default value for STAT_ISR_TIMER_INI, STAT_LEV_TIMER_INI. > > > + * STAT_TX_TIMER_INI, STAT_ISR_TIMER_INI and STAT_LEV_TIMER_INI > > > + * as well as various water mark registers seem to control when > > > + * controller initiates status LE update. > > > + * It's not clear how these registers interact with interrupt > > > + * state but STAT_TX_TIMER_INI seems to control status update > > > + * time after crossing a threshold(STAT_TX_IDX_TH) value. Due to > > > + * the complexity of various parameters that may affect status > > > + * update just use hardware default until we know better > > > + * the internal things of status updates. > > > */ > > > - CSR_WRITE_4(sc, STAT_TX_TIMER_INI, MSK_USECS(sc, 1000)); > > > > > > /* Enable status unit. */ > > > CSR_WRITE_4(sc, STAT_CTRL, SC_STAT_OP_ON); > > > > After applying this patch against an original 1.18.2.39 > > things became even worse. After 4 minutes of waiting I > > aborted untar'ing the file ;-). Setting int_holdoff=2 > > made things better, now it took 65 seconds for untar'ing... > > > > :-( > > By chance, did you disable MSI(msi_disable=1)? Yes. Do you want me to test again with MSI enabled? -Andre -- "Regression testing? What's that? If it compiles, it is good, if it boots up, it is perfect." - Linus Torvalds