From: Dongpo Li <lidongpo@hisilicon.com>
To: Arnd Bergmann <arnd@arndb.de>
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 16:33:46 +0800 [thread overview]
Message-ID: <578359EA.3060407@hisilicon.com> (raw)
In-Reply-To: <6408034.HDp0l1gLKO@wuerfel>
On 2016/7/11 16:16, Arnd Bergmann wrote:
> 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.
>
Okay, many thanks for your review.
> Arnd
>
> .
>
Regards,
Dongpo
.
next prev parent reply other threads:[~2016-07-11 8:41 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-13 6:07 [PATCH 0/3] Add Hisilicon MDIO bus driver and FEMAC driver Dongpo Li
2016-06-13 6:07 ` [PATCH 1/3] net: Add MDIO bus driver for the Hisilicon FEMAC Dongpo Li
2016-06-13 13:32 ` Andrew Lunn
2016-06-14 8:24 ` Li Dongpo
2016-06-14 22:27 ` Rob Herring
2016-06-15 7:49 ` Li Dongpo
2016-06-13 6:07 ` [PATCH 2/3] ethtool: Add common functions for get and set settings Dongpo Li
2016-06-13 7:36 ` kbuild test robot
2016-06-13 8:11 ` kbuild test robot
2016-06-13 8:38 ` kbuild test robot
2016-06-13 6:07 ` [PATCH 3/3] net: hisilicon: Add Fast Ethernet MAC driver Dongpo Li
2016-06-13 7:12 ` kbuild test robot
2016-06-13 9:06 ` Arnd Bergmann
2016-06-14 13:17 ` Li Dongpo
2016-06-14 21:20 ` Arnd Bergmann
2016-06-15 9:56 ` Dongpo Li
2016-06-28 9:21 ` Dongpo Li
2016-06-28 9:34 ` Arnd Bergmann
2016-07-11 3:44 ` Dongpo Li
2016-07-11 8:16 ` Arnd Bergmann
2016-07-11 8:33 ` Dongpo Li [this message]
2016-06-14 22:31 ` Rob Herring
2016-06-15 10:08 ` Dongpo Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=578359EA.3060407@hisilicon.com \
--to=lidongpo@hisilicon.com \
--cc=arnd@arndb.de \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=f.fainelli@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=netdev@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=xuejiancheng@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).