From: "Michael S. Tsirkin" <mst@redhat•com>
To: Jason Wang <jasowang@redhat•com>
Cc: Wei Wang <weiwan@google•com>,
Willem de Bruijn <willemdebruijn.kernel@gmail•com>,
David Miller <davem@davemloft•net>,
Network Development <netdev@vger•kernel.org>,
Jakub Kicinski <kuba@kernel•org>,
virtualization@lists•linux-foundation.org
Subject: Re: [PATCH net] virtio-net: suppress bad irq warning for tx napi
Date: Sun, 21 Feb 2021 06:33:01 -0500 [thread overview]
Message-ID: <20210221062810-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <9b077d6c-aeca-8266-4579-fae02c8b31de@redhat.com>
On Thu, Feb 18, 2021 at 01:39:19PM +0800, Jason Wang wrote:
>
> On 2021/2/10 下午5:14, Michael S. Tsirkin wrote:
> > On Tue, Feb 09, 2021 at 10:00:22AM -0800, Wei Wang wrote:
> > > On Tue, Feb 9, 2021 at 6:58 AM Willem de Bruijn
> > > <willemdebruijn.kernel@gmail•com> wrote:
> > > > > > > > I have no preference. Just curious, especially if it complicates the patch.
> > > > > > > >
> > > > > > > My understanding is that. It's probably ok for net. But we probably need
> > > > > > > to document the assumptions to make sure it was not abused in other drivers.
> > > > > > >
> > > > > > > Introduce new parameters for find_vqs() can help to eliminate the subtle
> > > > > > > stuffs but I agree it looks like a overkill.
> > > > > > >
> > > > > > > (Btw, I forget the numbers but wonder how much difference if we simple
> > > > > > > remove the free_old_xmits() from the rx NAPI path?)
> > > > > > The committed patchset did not record those numbers, but I found them
> > > > > > in an earlier iteration:
> > > > > >
> > > > > > [PATCH net-next 0/3] virtio-net tx napi
> > > > > > https://lists.openwall.net/netdev/2017/04/02/55
> > > > > >
> > > > > > It did seem to significantly reduce compute cycles ("Gcyc") at the
> > > > > > time. For instance:
> > > > > >
> > > > > > TCP_RR Latency (us):
> > > > > > 1x:
> > > > > > p50 24 24 21
> > > > > > p99 27 27 27
> > > > > > Gcycles 299 432 308
> > > > > >
> > > > > > I'm concerned that removing it now may cause a regression report in a
> > > > > > few months. That is higher risk than the spurious interrupt warning
> > > > > > that was only reported after years of use.
> > > > >
> > > > > Right.
> > > > >
> > > > > So if Michael is fine with this approach, I'm ok with it. But we
> > > > > probably need to a TODO to invent the interrupt handlers that can be
> > > > > used for more than one virtqueues. When MSI-X is enabled, the interrupt
> > > > > handler (vring_interrup()) assumes the interrupt is used by a single
> > > > > virtqueue.
> > > > Thanks.
> > > >
> > > > The approach to schedule tx-napi from virtnet_poll_cleantx instead of
> > > > cleaning directly in this rx-napi function was not effective at
> > > > suppressing the warning, I understand.
> > > Correct. I tried the approach to schedule tx napi instead of directly
> > > do free_old_xmit_skbs() in virtnet_poll_cleantx(). But the warning
> > > still happens.
> > Two questions here: is the device using packed or split vqs?
> > And is event index enabled?
> >
> > I think one issue is that at the moment with split and event index we
> > don't actually disable events at all.
>
>
> Do we really have a way to disable that? (We don't have a flag like packed
> virtqueue)
>
> Or you mean the trick [1] when I post tx interrupt RFC?
>
> Thanks
>
> [1] https://lkml.org/lkml/2015/2/9/113
Something like this. Or basically any other value will do,
e.g. move the index back to a value just signalled ...
>
> >
> > static void virtqueue_disable_cb_split(struct virtqueue *_vq)
> > {
> > struct vring_virtqueue *vq = to_vvq(_vq);
> >
> > if (!(vq->split.avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT)) {
> > vq->split.avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
> > if (!vq->event)
> > vq->split.vring.avail->flags =
> > cpu_to_virtio16(_vq->vdev,
> > vq->split.avail_flags_shadow);
> > }
> > }
> >
> > Can you try your napi patch + disable event index?
> >
> >
next prev parent reply other threads:[~2021-02-21 11:34 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-29 0:21 [PATCH net] virtio-net: suppress bad irq warning for tx napi Wei Wang
2021-02-02 2:24 ` Jakub Kicinski
2021-02-02 3:06 ` Jason Wang
2021-02-02 14:37 ` Willem de Bruijn
2021-02-03 5:33 ` Jason Wang
2021-02-03 18:28 ` Willem de Bruijn
2021-02-04 3:06 ` Jason Wang
2021-02-04 20:50 ` Willem de Bruijn
2021-02-08 3:29 ` Jason Wang
2021-02-08 19:08 ` Willem de Bruijn
2021-02-09 3:27 ` Jason Wang
2021-02-09 14:58 ` Willem de Bruijn
2021-02-09 18:00 ` Wei Wang
2021-02-10 9:14 ` Michael S. Tsirkin
2021-02-11 0:13 ` Wei Wang
2021-02-18 5:39 ` Jason Wang
2021-02-21 11:33 ` Michael S. Tsirkin [this message]
2021-02-02 23:11 ` Michael S. Tsirkin
2021-02-02 23:47 ` Wei Wang
2021-02-02 23:53 ` Willem de Bruijn
2021-02-03 0:06 ` Willem de Bruijn
2021-02-03 10:38 ` Michael S. Tsirkin
2021-02-03 18:24 ` Willem de Bruijn
2021-02-03 23:09 ` Michael S. Tsirkin
2021-02-03 23:52 ` Wei Wang
2021-02-04 20:47 ` Willem de Bruijn
2021-02-05 22:28 ` Wei Wang
2021-04-13 5:15 ` Michael S. Tsirkin
2021-09-29 20:21 ` Wei Wang
2021-09-29 21:53 ` Michael S. Tsirkin
2021-09-29 23:08 ` Wei Wang
2021-09-30 5:40 ` Michael S. Tsirkin
2021-09-30 18:10 ` Dave Taht
2021-04-12 22:08 ` Michael S. Tsirkin
2021-04-12 22:33 ` Michael S. Tsirkin
2021-04-12 23:14 ` David Miller
2021-04-13 4:24 ` Michael S. Tsirkin
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=20210221062810-mutt-send-email-mst@kernel.org \
--to=mst@redhat$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=jasowang@redhat$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=virtualization@lists$(echo .)linux-foundation.org \
--cc=weiwan@google$(echo .)com \
--cc=willemdebruijn.kernel@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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