public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Jan Kiszka <jan.kiszka@siemens.com>,
	Jan Kiszka <jan.kiszka@web.de>, Avi Kivity <avi@redhat.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	kvm <kvm@vger.kernel.org>, Tom Lyon <pugs@ieee.org>,
	Alex Williamson <alex.williamson@redhat.com>
Subject: Re: [PATCH v3 2/4] genirq: Inform handler about line sharing state
Date: Sat, 18 Dec 2010 20:11:00 +0200	[thread overview]
Message-ID: <20101218181100.GA2182@redhat.com> (raw)
In-Reply-To: <alpine.LFD.2.00.1012171725300.12146@localhost6.localdomain6>

On Fri, Dec 17, 2010 at 05:32:46PM +0100, Thomas Gleixner wrote:
> On Fri, 17 Dec 2010, Jan Kiszka wrote:
> > Am 17.12.2010 16:25, Thomas Gleixner wrote:
> > > Your aproach with disable_irq_nosync() is completely flawed, simply
> > > because you try to pretend that your interrupt handler is done, while
> > > it is not done at all. The threaded interrupt handler is done when
> > > user space completes. Everything else is just hacking around the
> > > problem and creates all that nasty transitional problems.
> > 
> > disable_irq_nosync is the pattern currently used in KVM, it's nothing
> > new in fact.
> 
> That does not make it less flawed :)
> 
> > The approach looks interesting but requires separate code for
> > non-PCI-2.3 devices, i.e. when we have no means to mask at device level.
> 
> Why? You can have the same code, you just can't request COND_ONESHOT
> handlers for it, so it needs unshared ONESHOT or it won't work at all,
> no matter what approach you chose. No device level mask, no sharing,
> it's that simple.
> 
> > Further drawbacks - unless I missed something on first glance:
> > 
> > - prevents any future optimizations that would work without IRQ thread
> >   ping-pong (ie. once we allow guest IRQ injection from hardirq context
> >   for selected but typical setups)
> > - two additional, though light-weight, context switches on each
> >   interrupt completion
> 
> The drawback of these two points is way less than the horror which you
> need to introduce to do the whole async handler disable, userspace
> enable dance. Robust and simple solutions really a preferred over
> complex and fragile horror which has a questionable runtime benefit.

I'd like to note that the overhead of involving the scheduler in
interrupt injection for an assigned device should be easily measurable:
just make the MSI handlers threaded and see what the result is.

In the case of emulated devices, when we had an extra thread
involved in MSI handling, the vcpu thread and the
interrupt injection thread were competing for cpu with strange
fluctuations in performance as the result
(i.e. sometimes we would get good speed as threading would introduce
a kind of interrupt coalescing, sometimes we would get huge latency).

> > - continuous polling if user space decides to leave the interrupt
> >   unhandled (e.g. because the virtual IRQ line is masked)
>  
> That should be a solvable problem.
> 
> Thanks,
> 
> 	tglx

  reply	other threads:[~2010-12-18 18:11 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-13 22:59 [PATCH v3 0/4] KVM & genirq: Enable adaptive IRQ sharing for passed-through devices Jan Kiszka
2010-12-13 22:59 ` [PATCH v3 1/4] genirq: Introduce driver-readable IRQ status word Jan Kiszka
2010-12-14 20:47   ` Thomas Gleixner
2010-12-14 23:10     ` Jan Kiszka
2010-12-13 22:59 ` [PATCH v3 2/4] genirq: Inform handler about line sharing state Jan Kiszka
2010-12-14 20:54   ` Thomas Gleixner
2010-12-14 23:00     ` Jan Kiszka
2010-12-15 13:04       ` Thomas Gleixner
2010-12-15 14:18         ` Jan Kiszka
2010-12-15 14:49           ` Thomas Gleixner
2010-12-15 15:41           ` Thomas Gleixner
2010-12-15 15:49             ` Jan Kiszka
2010-12-15 16:02               ` Thomas Gleixner
2010-12-14 21:46   ` Thomas Gleixner
2010-12-14 23:01     ` Jan Kiszka
2010-12-15  8:05       ` Thomas Gleixner
2010-12-15  9:37         ` Jan Kiszka
2010-12-15  9:48           ` Thomas Gleixner
2010-12-16 13:13   ` Thomas Gleixner
2010-12-16 20:26     ` Jan Kiszka
2010-12-16 21:28       ` change of email address: pugs@cisco.com -> pugs@ieee.org Tom Lyon
2010-12-17  8:18       ` [PATCH v3 2/4] genirq: Inform handler about line sharing state Jan Kiszka
2010-12-17 10:23         ` Thomas Gleixner
2010-12-17 10:31           ` Jan Kiszka
2010-12-17 10:41             ` Thomas Gleixner
2010-12-17 10:48               ` Jan Kiszka
2010-12-17 15:25                 ` Thomas Gleixner
2010-12-17 16:06                   ` Jan Kiszka
2010-12-17 16:32                     ` Thomas Gleixner
2010-12-18 18:11                       ` Michael S. Tsirkin [this message]
2010-12-13 22:59 ` [PATCH v3 3/4] genirq: Add support for IRQF_COND_ONESHOT Jan Kiszka
2010-12-13 22:59 ` [PATCH v3 4/4] KVM: Allow host IRQ sharing for passed-through PCI 2.3 devices Jan Kiszka
2010-12-14  8:53 ` [PATCH v3 0/4] KVM & genirq: Enable adaptive IRQ sharing for passed-through devices Avi Kivity
2010-12-14 22:01 ` Thomas Gleixner

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=20101218181100.GA2182@redhat.com \
    --to=mst@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=avi@redhat.com \
    --cc=jan.kiszka@siemens.com \
    --cc=jan.kiszka@web.de \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=pugs@ieee.org \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox