[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACGkMEvYC0vV9McY=G6V9K56yqtc5=9RVk7XbVX495GYyRcQcg@mail.gmail.com>
Date: Tue, 18 Jun 2024 08:52:38 +0800
From: Jason Wang <jasowang@...hat.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: "Michael S. Tsirkin" <mst@...hat.com>, Jason Xing <kerneljasonxing@...il.com>,
Heng Qi <hengqi@...ux.alibaba.com>, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, xuanzhuo@...ux.alibaba.com,
virtualization@...ts.linux.dev, ast@...nel.org, daniel@...earbox.net,
hawk@...nel.org, john.fastabend@...il.com, netdev@...r.kernel.org
Subject: Re: [patch net-next] virtio_net: add support for Byte Queue Limits
On Mon, Jun 17, 2024 at 5:30 PM Jiri Pirko <jiri@...nulli.us> wrote:
>
> Mon, Jun 17, 2024 at 03:44:55AM CEST, jasowang@...hat.com wrote:
> >On Mon, Jun 10, 2024 at 10:19 PM Michael S. Tsirkin <mst@...hat.com> wrote:
> >>
> >> On Fri, Jun 07, 2024 at 01:30:34PM +0200, Jiri Pirko wrote:
> >> > Fri, Jun 07, 2024 at 12:23:37PM CEST, mst@...hat.com wrote:
> >> > >On Fri, Jun 07, 2024 at 11:57:37AM +0200, Jiri Pirko wrote:
> >> > >> >True. Personally, I would like to just drop orphan mode. But I'm not
> >> > >> >sure others are happy with this.
> >> > >>
> >> > >> How about to do it other way around. I will take a stab at sending patch
> >> > >> removing it. If anyone is against and has solid data to prove orphan
> >> > >> mode is needed, let them provide those.
> >> > >
> >> > >Break it with no warning and see if anyone complains?
> >> >
> >> > This is now what I suggested at all.
> >> >
> >> > >No, this is not how we handle userspace compatibility, normally.
> >> >
> >> > Sure.
> >> >
> >> > Again:
> >> >
> >> > I would send orphan removal patch containing:
> >> > 1) no module options removal. Warn if someone sets it up
> >> > 2) module option to disable napi is ignored
> >> > 3) orphan mode is removed from code
> >> >
> >> > There is no breakage. Only, hypotetically performance downgrade in some
> >> > hypotetical usecase nobody knows of.
> >>
> >> Performance is why people use virtio. It's as much a breakage as any
> >> other bug. The main difference is, with other types of breakage, they
> >> are typically binary and we can not tolerate them at all. A tiny,
> >> negligeable performance regression might be tolarable if it brings
> >> other benefits. I very much doubt avoiding interrupts is
> >> negligeable though. And making code simpler isn't a big benefit,
> >> users do not care.
> >
> >It's not just making code simpler. As discussed in the past, it also
> >fixes real bugs.
> >
> >>
> >> > My point was, if someone presents
> >> > solid data to prove orphan is needed during the patch review, let's toss
> >> > out the patch.
> >> >
> >> > Makes sense?
> >>
> >> It's not hypothetical - if anything, it's hypothetical that performance
> >> does not regress. And we just got a report from users that see a
> >> regression without. So, not really.
> >
> >Probably, but do we need to define a bar here? Looking at git history,
> >we didn't ask a full benchmark for a lot of commits that may touch
>
> Moreover, there is no "benchmark" to run anyway, is it?
Yes, so my point is to have some agreement on
1) what kind of test needs to be run for a patch like this.
2) what numbers are ok or not
Thanks
>
>
> >performance.
> >
> >Thanks
> >
> >>
> >> >
> >> > >
> >> > >--
> >> > >MST
> >> > >
> >>
> >
>
Powered by blists - more mailing lists