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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 13 Apr 2015 17:44:04 +0200
From:	Felix Fietkau <nbd@...nwrt.org>
To:	Rafał Miłecki <zajec5@...il.com>
CC:	Network Development <netdev@...r.kernel.org>,
	Hauke Mehrtens <hauke@...ke-m.de>
Subject: Re: [PATCH v4 2/9] bgmac: leave interrupts disabled as long as there
 is work to do

On 2015-04-13 17:06, Rafał Miłecki wrote:
> On 13 April 2015 at 17:03, Felix Fietkau <nbd@...nwrt.org> wrote:
>> On 2015-04-13 16:34, Rafał Miłecki wrote:
>>> On 13 April 2015 at 15:52, Felix Fietkau <nbd@...nwrt.org> 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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html