From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030482AbcGKINu (ORCPT ); Mon, 11 Jul 2016 04:13:50 -0400 Received: from mout.kundenserver.de ([217.72.192.74]:56434 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030249AbcGKINr (ORCPT ); Mon, 11 Jul 2016 04:13:47 -0400 From: Arnd Bergmann To: Dongpo Li Cc: f.fainelli@gmail.com, robh+dt@kernel.org, mark.rutland@arm.com, davem@davemloft.net, xuejiancheng@hisilicon.com, netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] net: hisilicon: Add Fast Ethernet MAC driver Date: Mon, 11 Jul 2016 10:16:25 +0200 Message-ID: <6408034.HDp0l1gLKO@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-22-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <57831617.6080506@hisilicon.com> References: <1465798076-176393-1-git-send-email-lidongpo@hisilicon.com> <4787248.HAfgOdBKVY@wuerfel> <57831617.6080506@hisilicon.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:wR4brYKPfQQmnkBXcUuEpvQAQuHNNqnt9kDJ/DyU/gcX4v1r426 gXoYTTMN2hJbtZjPrL2/klnauVO/Ow6pfLHD14RoaG5nMocjDtU3kukCEPX5mMEuiVgIthG CHmA/Wsz2ojV4wbZPar0osQ5FlH9yTFFuCqlzKIAWJgUV4KAQW01OQgEge3vmiS+0HwmIIF eQKlWm7waS0JXB+d6WQOA== X-UI-Out-Filterresults: notjunk:1;V01:K0:rsUDNfo1o5c=:bNgC2fqbqAtNDSqaMVf7tw 2uCUAwmSIPLthR1WS7JwaqwLG31UK+cbAc2Yi/IjLobs/cwyTrjknUAiq9z/yI14kggTZDQ5W 31zq9zGPRhjYudyTPg2Cj4Dhb/nPP90lzD0bjPxRv8Y2z/yMZG0hRws6T7MZKwmpFSuNwtwhZ yptZKZGhyIq5KdEVZMtH3lXUFKBorbY3/VVZlmPgyZ4e3zAYODOh8vCRcUTjGX1yJfefd0dGE HgWw9k5SigqEOH5a06xjb5xtp62ssihUifz/LrHsi7l4t9xeBZiXHj7TzG4Gj2TS91oXG/0rC e7UrSxfr+S/OUQvmaVvLtNEGb3e03gDvKpOuVNNgcCu0xtFU6OjptmTj/CrC9eQMGA0n/XZuT 3K2dDd+pFtlR3SIvJm8TGsaOY+0keWSRF4vsar7OKsvlKq5JRrH4esEOl/+SEg3MLaKJUnJuW sVYKwrpdqhPjU+sipDWx4hj9wT8v+ueFpmvb0t2V2HfR8VCsI5xbrD5PZ9f+qTtWROwtv2OYF ea+QW+vIsH5I1t+2lpsADE64q0qh+OJCFVMqRRE1lRzkbgQyfqlB7LoiSfn6t1JeSf5KccMpf Vmp1SAQWFvET60VgkoqLmpv95V9k8B7BexElPedzankWex4/CRIbhv+N8kwnthBC7v+Zv3sy/ BgTE6Kd/s5VbYSKfielF6yQAz2xqkTUGWqJobo3VcUHZUBu3SDyKkS/oEAG4DjMLoHWDf6+2r nM/kAeMK1dNPeEkq Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday, July 11, 2016 11:44:23 AM CEST Dongpo Li wrote: > Hi Arnd, > > On 2016/6/28 17:34, Arnd Bergmann wrote: > > On Tuesday, June 28, 2016 5:21:19 PM CEST Dongpo Li wrote: > >> On 2016/6/15 5:20, Arnd Bergmann wrote: > >>> On Tuesday, June 14, 2016 9:17:44 PM CEST Li Dongpo wrote: > >>>> On 2016/6/13 17:06, Arnd Bergmann wrote: > >>>>> On Monday, June 13, 2016 2:07:56 PM CEST Dongpo Li wrote: > >>>>> You tx function uses BQL to optimize the queue length, and that > >>>>> is great. You also check xmit reclaim for rx interrupts, so > >>>>> as long as you have both rx and tx traffic, this should work > >>>>> great. > >>>>> > >>>>> However, I notice that you only have a 'tx fifo empty' > >>>>> interrupt triggering the napi poll, so I guess on a tx-only > >>>>> workload you will always end up pushing packets into the > >>>>> queue until BQL throttles tx, and then get the interrupt > >>>>> after all packets have been sent, which will cause BQL to > >>>>> make the queue longer up to the maximum queue size, and that > >>>>> negates the effect of BQL. > >>>>> > >>>>> Is there any way you can get a tx interrupt earlier than > >>>>> this in order to get a more balanced queue, or is it ok > >>>>> to just rely on rx packets to come in occasionally, and > >>>>> just use the tx fifo empty interrupt as a fallback? > >>>>> > >>>> In tx direction, there are only two kinds of interrupts, 'tx fifo empty' > >>>> and 'tx one packet finish'. I didn't use 'tx one packet finish' because > >>>> it would lead to high hardware interrupts rate. This has been verified in > >>>> our chips. It's ok to just use tx fifo empty interrupt. > >>> > >>> I'm not convinced by the explanation, I don't think that has anything > >>> to do with the hardware design, but instead is about the correctness > >>> of the BQL logic with your driver. > >>> > >>> Maybe your xmit function can do something like > >>> > >>> if (dql_avail(netdev_get_tx_queue(dev, 0)->dql) < 0) > >>> enable per-packet interrupt > >>> else > >>> use only fifo-empty interrupt > >>> > >>> That way, you don't get a lot of interrupts when the system is > >>> in a state of packets being received and sent continuously, > >>> but if you get to the point where your tx queue fills up > >>> and no rx interrupts arrive, you don't have to wait for it > >>> to become completely empty before adding new packets, and > >>> BQL won't keep growing the queue. > >>> > >> Hi, Arnd > >> I tried enable per-packet interrupt when tx queue full in xmit function > >> and disable it in NAPI poll. But the number of interrupts are a little > >> bigger than only using fifo-empty interrupt. > > > > Right, I'd expect that to be the case, it basically means that the > > algorithm works as expected. > > > > Just to be sure you didn't have extra interrupts: you only enable the > > per-packet interrupts if interrupts are currently enabled, not in > > NAPI polling mode, right? > > > Sorry so long to reply to you. I use the per-packet interrupt like this: > In my xmit function, > if (hardware tx fifo is full) { > enable tx per-packet interrupt; > netif_stop_queue(dev); > return NETDEV_TX_BUSY; > } > > In interrupt handle function, > if (interrupt is tx per-packet or tx fifo-empty or rx) { > disable tx per-packet interrupt; > napi_schedule(&priv->napi); > } > We disable tx per-packet interrupt anyway because the NAPI poll will reclaim > the tx fifo. > When the NAPI poll completed, it will only enable the tx fifo-empty interrupt > and rx interrupt except the tx per-packet interrupt. > > Is this solution okay? Yes, this looks good to me. Arnd