Subject: Re: [PATCH v4 2/9] bgmac: leave interrupts disabled as long as there is work to do

From: Felix Fietkau <[email-masked]>
Date: Mon, 13 Apr 2015 17:44:04 +0200

On 2015-04-13 17:06, Rafał Miłecki wrote:
> On 13 April 2015 at 17:03, Felix Fietkau <[email-masked]> wrote:
>> On 2015-04-13 16:34, Rafał Miłecki wrote:
>>> On 13 April 2015 at 15:52, Felix Fietkau <[email-masked]> wrote:
>>>> Always poll rx and tx during NAPI poll instead of relying on the status
>>>> of the first interrupt. This prevents bgmac_poll from leaving unfinished
>>>> work around until the next IRQ.
>>>> In my tests this makes bridging/routing throughput under heavy load more
>>>> stable and ensures that no new IRQs arrive as long as bgmac_poll uses up
>>>> the entire budget.
>>>
>>> What do you think about keeping u32 int_status; and just updating it
>>> at the end of bgmac_poll? In case you decide to implement multiple TX
>>> queues, it may be cheaper to just check a single bit in memory instead
>>> reading DMA ring status.
>> Events might arrive in the mean time. I ran some tests, and not checking
>> the irq status for processing rx/tx gave me fewer total IRQs under load.
> 
> But you do check the status, I mean the following line:
> if (bgmac_read(bgmac, BGMAC_INT_STATUS) & (BGMAC_IS_TX0 | BGMAC_IS_RX))
> 
> Would it make sense to do
> bgmac->int_status = bgmac_read(bgmac, BGMAC_INT_STATUS);
I don't really see the point in storing the status from the initial IRQ.

The check there is only to decide whether the poll function should run
at all. By the time bgmac_poll is called, more events may have arrived
already, making the initial status useless.

- Felix