On Wed, Feb 03, 2021 at 01:24:08PM -0500, Willem de Bruijn wrote:

On Wed, Feb 3, 2021 at 5:42 AM Michael S. Tsirkin [off-list ref] wrote:

quoted

On Tue, Feb 02, 2021 at 07:06:53PM -0500, Willem de Bruijn wrote:

quoted

On Tue, Feb 2, 2021 at 6:53 PM Willem de Bruijn [off-list ref] wrote:

quoted

On Tue, Feb 2, 2021 at 6:47 PM Wei Wang [off-list ref] wrote:

quoted

On Tue, Feb 2, 2021 at 3:12 PM Michael S. Tsirkin [off-list ref] wrote:

quoted

On Thu, Jan 28, 2021 at 04:21:36PM -0800, Wei Wang wrote:

quoted

With the implementation of napi-tx in virtio driver, we clean tx
descriptors from rx napi handler, for the purpose of reducing tx
complete interrupts. But this could introduce a race where tx complete
interrupt has been raised, but the handler found there is no work to do
because we have done the work in the previous rx interrupt handler.
This could lead to the following warning msg:
[ 3588.010778] irq 38: nobody cared (try booting with the
"irqpoll" option)
[ 3588.017938] CPU: 4 PID: 0 Comm: swapper/4 Not tainted
5.3.0-19-generic #20~18.04.2-Ubuntu
[ 3588.017940] Call Trace:
[ 3588.017942]  <IRQ>
[ 3588.017951]  dump_stack+0x63/0x85
[ 3588.017953]  __report_bad_irq+0x35/0xc0
[ 3588.017955]  note_interrupt+0x24b/0x2a0
[ 3588.017956]  handle_irq_event_percpu+0x54/0x80
[ 3588.017957]  handle_irq_event+0x3b/0x60
[ 3588.017958]  handle_edge_irq+0x83/0x1a0
[ 3588.017961]  handle_irq+0x20/0x30
[ 3588.017964]  do_IRQ+0x50/0xe0
[ 3588.017966]  common_interrupt+0xf/0xf
[ 3588.017966]  </IRQ>
[ 3588.017989] handlers:
[ 3588.020374] [<000000001b9f1da8>] vring_interrupt
[ 3588.025099] Disabling IRQ #38

This patch adds a new param to struct vring_virtqueue, and we set it for
tx virtqueues if napi-tx is enabled, to suppress the warning in such
case.

Fixes: 7b0411ef4aa6 ("virtio-net: clean tx descriptors from rx napi")
Reported-by: Rick Jones <redacted>
Signed-off-by: Wei Wang <redacted>
Signed-off-by: Willem de Bruijn <willemb@google.com>

This description does not make sense to me.

irq X: nobody cared
only triggers after an interrupt is unhandled repeatedly.

So something causes a storm of useless tx interrupts here.

Let's find out what it was please. What you are doing is
just preventing linux from complaining.

The traffic that causes this warning is a netperf tcp_stream with at
least 128 flows between 2 hosts. And the warning gets triggered on the
receiving host, which has a lot of rx interrupts firing on all queues,
and a few tx interrupts.
And I think the scenario is: when the tx interrupt gets fired, it gets
coalesced with the rx interrupt. Basically, the rx and tx interrupts
get triggered very close to each other, and gets handled in one round
of do_IRQ(). And the rx irq handler gets called first, which calls
virtnet_poll(). However, virtnet_poll() calls virtnet_poll_cleantx()
to try to do the work on the corresponding tx queue as well. That's
why when tx interrupt handler gets called, it sees no work to do.
And the reason for the rx handler to handle the tx work is here:
https://lists.linuxfoundation.org/pipermail/virtualization/2017-April/034740.html

Indeed. It's not a storm necessarily. The warning occurs after one
hundred such events, since boot, which is a small number compared real
interrupt load.

Sorry, this is wrong. It is the other call to __report_bad_irq from
note_interrupt that applies here.

quoted

Occasionally seeing an interrupt with no work is expected after
7b0411ef4aa6 ("virtio-net: clean tx descriptors from rx napi"). As
long as this rate of events is very low compared to useful interrupts,
and total interrupt count is greatly reduced vs not having work
stealing, it is a net win.

Right, but if 99900 out of 100000 interrupts were wasted, then it is
surely an even greater win to disable interrupts while polling like
this.  Might be tricky to detect, disabling/enabling aggressively every
time even if there's nothing in the queue is sure to cause lots of cache
line bounces, and we don't want to enable callbacks if they were not
enabled e.g. by start_xmit ...  Some kind of counter?

Yes. It was known that the work stealing is more effective in some
workloads than others. But a 99% spurious rate I had not anticipated.

Most interesting is the number of interrupts suppressed as a result of
the feature. That is not captured by this statistic.

In any case, we'll take a step back to better understand behavior. And
especially why this high spurious rate exhibits in this workload with
many concurrent flows.

I've been thinking about it. Imagine work stealing working perfectly.
Each time we xmit a packet, it is stolen and freed.
Since xmit enables callbacks (just in case!) we also
get an interrupt, which is automatically spurious.

My conclusion is that we shouldn't just work around it but instead
(or additionally?)
reduce the number of interrupts by disabling callbacks e.g. when
a. we are currently stealing packets
or
b. we stole all packets

This should be enough to reduce the chances below 99% ;)

One annoying thing is that with split and event index, we do not disable
interrupts. Could be worth revisiting, for now maybe just disable the
event index feature? I am not sure it is actually worth it with
stealing.

-- 
MST