On Mon, Jun 17, 2024 at 5:30 PM Jiri Pirko <jiri@xxxxxxxxxxx> wrote: > > Mon, Jun 17, 2024 at 03:44:55AM CEST, jasowang@xxxxxxxxxx wrote: > >On Mon, Jun 10, 2024 at 10:19 PM Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: > >> > >> On Fri, Jun 07, 2024 at 01:30:34PM +0200, Jiri Pirko wrote: > >> > Fri, Jun 07, 2024 at 12:23:37PM CEST, mst@xxxxxxxxxx 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 > >> > > > >> > > >
