From: "Michael S. Tsirkin" <mst@redhat.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org
Subject: Re: [PATCH] vfio-pci: Add KVM INTx acceleration
Date: Tue, 16 Oct 2012 19:34:15 +0200 [thread overview]
Message-ID: <20121016173415.GA15698@redhat.com> (raw)
In-Reply-To: <1350406178.2112.249.camel@bling.home>
On Tue, Oct 16, 2012 at 10:49:38AM -0600, Alex Williamson wrote:
> On Tue, 2012-10-16 at 17:23 +0200, Michael S. Tsirkin wrote:
> > On Tue, Oct 16, 2012 at 09:13:15AM -0600, Alex Williamson wrote:
> > > > There's no chance we ship e.g. q35 by mistake without this API: since
> > > > there is no way this specific assert can be missed in even basic
> > > > testing:
> > > >
> > > > So I see it differently:
> > > >
> > > > As coded here:
> > > > chipset authors get lazy and do not implement API.
> > > > bad performance for all users.
> > > >
> > > > With assert:
> > > > chipset authors implement necessary API.
> > > > good performance for all users.
> > >
> > > I prefer a carrot, not a whip. Thanks,
> > >
> > > Alex
> > >
> >
> > It's not just that.
> > Problem is performance testing/fixing is hard.
>
> Getting an error_report from the driver saying it's using a slow path
> and why makes that significantly easier.
>
> > Catching and fixing asserts is easy.
>
> Easy for who? The user trying to test a feature? Probably not. Me,
> who may not have access to the chipset documentation or understand the
> platform? Maybe, maybe not.
>
> > So working around buggy qemu code really backfires
> > as it reverses the motivation for writing well performing
> > code. History proves me right: for each API change where
> > we implemented a fallback old code stayed around for years.
>
> Does that necessarily mean it was wrong? How many of those API changes
> added new features that may have been abandoned if the developer was
> required to make sweeping changes to get their code accepted? If not
> abandoned, how much delayed? How many land mines might we have in the
> code for changes that were done incorrectly or missed? I don't
> understand why adding robustness to the API is such a contentious point,
> but it's your prerogative, just as it's mine to avoid using that API
> arbitrarily. Thanks,
>
> Alex
Yea. All I say is, I intend to fix things so you don't need
to probe for this API. I think it does not make sense to
add a temporary API as a stopgap for this since it will solve no
actual problem. If e.g. Jason finds it hard to add this to
q35, we could add a stopgap solution for vfio.
--
MST
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org
Subject: Re: [Qemu-devel] [PATCH] vfio-pci: Add KVM INTx acceleration
Date: Tue, 16 Oct 2012 19:34:15 +0200 [thread overview]
Message-ID: <20121016173415.GA15698@redhat.com> (raw)
In-Reply-To: <1350406178.2112.249.camel@bling.home>
On Tue, Oct 16, 2012 at 10:49:38AM -0600, Alex Williamson wrote:
> On Tue, 2012-10-16 at 17:23 +0200, Michael S. Tsirkin wrote:
> > On Tue, Oct 16, 2012 at 09:13:15AM -0600, Alex Williamson wrote:
> > > > There's no chance we ship e.g. q35 by mistake without this API: since
> > > > there is no way this specific assert can be missed in even basic
> > > > testing:
> > > >
> > > > So I see it differently:
> > > >
> > > > As coded here:
> > > > chipset authors get lazy and do not implement API.
> > > > bad performance for all users.
> > > >
> > > > With assert:
> > > > chipset authors implement necessary API.
> > > > good performance for all users.
> > >
> > > I prefer a carrot, not a whip. Thanks,
> > >
> > > Alex
> > >
> >
> > It's not just that.
> > Problem is performance testing/fixing is hard.
>
> Getting an error_report from the driver saying it's using a slow path
> and why makes that significantly easier.
>
> > Catching and fixing asserts is easy.
>
> Easy for who? The user trying to test a feature? Probably not. Me,
> who may not have access to the chipset documentation or understand the
> platform? Maybe, maybe not.
>
> > So working around buggy qemu code really backfires
> > as it reverses the motivation for writing well performing
> > code. History proves me right: for each API change where
> > we implemented a fallback old code stayed around for years.
>
> Does that necessarily mean it was wrong? How many of those API changes
> added new features that may have been abandoned if the developer was
> required to make sweeping changes to get their code accepted? If not
> abandoned, how much delayed? How many land mines might we have in the
> code for changes that were done incorrectly or missed? I don't
> understand why adding robustness to the API is such a contentious point,
> but it's your prerogative, just as it's mine to avoid using that API
> arbitrarily. Thanks,
>
> Alex
Yea. All I say is, I intend to fix things so you don't need
to probe for this API. I think it does not make sense to
add a temporary API as a stopgap for this since it will solve no
actual problem. If e.g. Jason finds it hard to add this to
q35, we could add a stopgap solution for vfio.
--
MST
next prev parent reply other threads:[~2012-10-16 17:32 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-15 20:28 [PATCH] vfio-pci: Add KVM INTx acceleration Alex Williamson
2012-10-15 20:28 ` [Qemu-devel] " Alex Williamson
2012-10-16 6:39 ` Michael S. Tsirkin
2012-10-16 6:39 ` [Qemu-devel] " Michael S. Tsirkin
2012-10-16 13:51 ` Alex Williamson
2012-10-16 13:51 ` [Qemu-devel] " Alex Williamson
2012-10-16 14:14 ` Michael S. Tsirkin
2012-10-16 14:14 ` [Qemu-devel] " Michael S. Tsirkin
2012-10-16 14:48 ` Alex Williamson
2012-10-16 14:48 ` [Qemu-devel] " Alex Williamson
2012-10-16 15:08 ` Michael S. Tsirkin
2012-10-16 15:08 ` [Qemu-devel] " Michael S. Tsirkin
2012-10-16 15:13 ` Alex Williamson
2012-10-16 15:13 ` [Qemu-devel] " Alex Williamson
2012-10-16 15:23 ` Michael S. Tsirkin
2012-10-16 15:23 ` [Qemu-devel] " Michael S. Tsirkin
2012-10-16 16:49 ` Alex Williamson
2012-10-16 16:49 ` [Qemu-devel] " Alex Williamson
2012-10-16 17:34 ` Michael S. Tsirkin [this message]
2012-10-16 17:34 ` 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=20121016173415.GA15698@redhat.com \
--to=mst@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.