All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Davide Libenzi <davidel@xmailserver.org>,
	Avi Kivity <avi@redhat.com>,
	kvm@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] kvm: fix spurious interrupt with irqfd
Date: Tue, 19 Jan 2010 14:25:12 +0100	[thread overview]
Message-ID: <4B55B2B8.5090105@siemens.com> (raw)
In-Reply-To: <20100113171230.GB19798@redhat.com>

Michael S. Tsirkin wrote:
> kvm didn't clear irqfd counter on deassign, as a result we could get a
> spurious interrupt when irqfd is assigned back. this leads to poor
> performance and, in theory, guest crash.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  virt/kvm/eventfd.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index 62e4cd9..a9d3fc6 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -72,12 +72,13 @@ static void
>  irqfd_shutdown(struct work_struct *work)
>  {
>  	struct _irqfd *irqfd = container_of(work, struct _irqfd, shutdown);
> +	u64 cnt;
>  
>  	/*
>  	 * Synchronize with the wait-queue and unhook ourselves to prevent
>  	 * further events.
>  	 */
> -	remove_wait_queue(irqfd->wqh, &irqfd->wait);
> +	eventfd_ctx_remove_wait_queue(irqfd->eventfd, &irqfd->wait, &cnt);
>  
>  	/*
>  	 * We know no new events will be scheduled at this point, so block

For kvm-kmod, I'm fighting with compat support for
eventfd_ctx_remove_wait_queue. I basically have a solution for kernels
with CONFIG_KPROBES enabled (I need to look up unexported
__wake_up_locked[_key]), but there will also be target kernels that do
not have this. So there are three options for that case:

 - Warn the user and fall back to the old racy approach
 - (Somehow) disable KVM subsystems that use eventfd
 - Refuse to start KVM

As far as I understood, irqfd is interesting for device assignment and
now also for vhost, right? What about ioeventfd? I just wonder how broad
the impact of a broken or non-existent eventfd subsystem for kvm-kmod
is. Any thoughts welcome.

Jan

PS: If anyone forgot why Avi handed over this job, you should now
remember why. :)

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

  reply	other threads:[~2010-01-19 13:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1263402727.git.mst@redhat.com>
2010-01-13 17:12 ` [PATCH 1/2] kvm: fix spurious interrupt with irqfd Michael S. Tsirkin
2010-01-19 13:25   ` Jan Kiszka [this message]
2010-01-19 13:48     ` Michael S. Tsirkin
2010-01-19 14:03       ` Jan Kiszka
2010-01-19 14:04         ` Michael S. Tsirkin
2010-01-19 14:23           ` Jan Kiszka
2010-01-19 14:29             ` Michael S. Tsirkin
2010-01-19 14:03     ` Avi Kivity
2010-01-13 17:12 ` [PATCH 2/2] kvm: do not store wqh in irqfd 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=4B55B2B8.5090105@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=avi@redhat.com \
    --cc=davidel@xmailserver.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    /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.