qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Anthony Liguori <aliguori@us.ibm.com>,
	qemu-devel@nongnu.org, Blue Swirl <blauwirbel@gmail.com>,
	khoa@us.ibm.com, Paolo Bonzini <pbonzini@redhat.com>,
	Asias He <asias@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v4 03/11] dataplane: add host memory mapping code
Date: Thu, 29 Nov 2012 14:54:26 +0200	[thread overview]
Message-ID: <20121129125426.GC9372@redhat.com> (raw)
In-Reply-To: <20121129124519.GA13589@stefanha-thinkpad.redhat.com>

On Thu, Nov 29, 2012 at 01:45:19PM +0100, Stefan Hajnoczi wrote:
> On Thu, Nov 29, 2012 at 02:33:11PM +0200, Michael S. Tsirkin wrote:
> > On Thu, Nov 22, 2012 at 04:16:44PM +0100, Stefan Hajnoczi wrote:
> > > The data plane thread needs to map guest physical addresses to host
> > > pointers.  Normally this is done with cpu_physical_memory_map() but the
> > > function assumes the global mutex is held.  The data plane thread does
> > > not touch the global mutex and therefore needs a thread-safe memory
> > > mapping mechanism.
> > > 
> > > Hostmem registers a MemoryListener similar to how vhost collects and
> > > pushes memory region information into the kernel.  There is a
> > > fine-grained lock on the regions list which is held during lookup and
> > > when installing a new regions list.
> > > 
> > > When the physical memory map changes the MemoryListener callbacks are
> > > invoked.  They build up a new list of memory regions which is finally
> > > installed when the list has been completed.
> > > 
> > > Note that this approach is not safe across memory hotplug because mapped
> > > pointers may still be in used across memory unplug.  However, this is
> > > currently a problem for QEMU in general and needs to be addressed in the
> > > future.
> > 
> > Sounds like a serious problem.
> > I'm not sure I understand - do you say this currently a problem for QEMU
> > virtio? Coul you give an example please?
> 
> This is a limitation of the memory API but cannot be triggered by users
> today since we don't support memory hot unplug.  I'm just explaining
> that virtio-blk-data-plane has the same issue as hw/virtio-blk.c or any
> other device emulation code here.
> 
> Some more detail:
> 
> The issue is that hw/virtio-blk.c submits an asynchronous I/O request on
> the host with the guest buffer.  Then virtio-blk emulation returns back
> to the caller and continues QEMU execution.
> 
> It is unsafe to unplug memory while the I/O request is pending since
> there's no mechanism (e.g. refcount) to wait until the guest memory is
> no longer in use.
> 
> This is a known issue.  There's no way to trigger a problem today but we
> need to eventually enhance QEMU's memory API to handle this case.
> 
> Stefan

For this problem we would simply need to flush outstanding aio
before freeing memory for unplug, no refcount necessary.

This patch however introduces the issue in the frontend
and it looks like there won't be any way to solve
it without changing the API.

-- 
MST

  reply	other threads:[~2012-11-29 12:51 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-22 15:16 [Qemu-devel] [PATCH v4 00/11] virtio: virtio-blk data plane Stefan Hajnoczi
2012-11-22 15:16 ` [Qemu-devel] [PATCH v4 01/11] raw-posix: add raw_get_aio_fd() for virtio-blk-data-plane Stefan Hajnoczi
2012-11-22 15:16 ` [Qemu-devel] [PATCH v4 02/11] configure: add CONFIG_VIRTIO_BLK_DATA_PLANE Stefan Hajnoczi
2012-11-22 15:16 ` [Qemu-devel] [PATCH v4 03/11] dataplane: add host memory mapping code Stefan Hajnoczi
2012-11-29 12:33   ` Michael S. Tsirkin
2012-11-29 12:45     ` Stefan Hajnoczi
2012-11-29 12:54       ` Michael S. Tsirkin [this message]
2012-11-29 12:57         ` Michael S. Tsirkin
2012-12-05  8:13           ` Stefan Hajnoczi
2012-11-29 13:54   ` Michael S. Tsirkin
2012-11-29 14:26     ` Stefan Hajnoczi
2012-11-29 14:36       ` Michael S. Tsirkin
2012-11-29 15:26         ` Paolo Bonzini
2012-12-05  8:31         ` Stefan Hajnoczi
2012-12-05 11:22           ` Michael S. Tsirkin
2012-11-22 15:16 ` [Qemu-devel] [PATCH v4 04/11] dataplane: add virtqueue vring code Stefan Hajnoczi
2012-11-29 12:50   ` Michael S. Tsirkin
2012-11-29 15:17     ` Paolo Bonzini
2012-12-05 12:57     ` Stefan Hajnoczi
2012-11-29 13:48   ` Michael S. Tsirkin
2012-11-22 15:16 ` [Qemu-devel] [PATCH v4 05/11] dataplane: add event loop Stefan Hajnoczi
2012-11-22 15:16 ` [Qemu-devel] [PATCH v4 06/11] dataplane: add Linux AIO request queue Stefan Hajnoczi
2012-11-22 15:16 ` [Qemu-devel] [PATCH v4 07/11] iov: add iov_discard() to remove data Stefan Hajnoczi
2012-11-22 15:16 ` [Qemu-devel] [PATCH v4 08/11] test-iov: add iov_discard() testcase Stefan Hajnoczi
2012-11-22 15:16 ` [Qemu-devel] [PATCH v4 09/11] iov: add qemu_iovec_concat_iov() Stefan Hajnoczi
2012-11-22 15:16 ` [Qemu-devel] [PATCH v4 10/11] dataplane: add virtio-blk data plane code Stefan Hajnoczi
2012-11-29 13:41   ` Michael S. Tsirkin
2012-11-29 14:02   ` Michael S. Tsirkin
2012-11-29 15:21     ` Paolo Bonzini
2012-11-29 15:27       ` Michael S. Tsirkin
2012-11-29 15:47         ` Paolo Bonzini
2012-11-30 13:57           ` Stefan Hajnoczi
2012-11-22 15:16 ` [Qemu-devel] [PATCH v4 11/11] virtio-blk: add x-data-plane=on|off performance feature Stefan Hajnoczi
2012-11-29 13:12   ` Michael S. Tsirkin
2012-11-29 14:45     ` Stefan Hajnoczi
2012-11-29 14:55       ` Michael S. Tsirkin
2012-12-04 11:20         ` Michael S. Tsirkin
2012-12-04 14:19           ` Stefan Hajnoczi
2012-11-29  9:18 ` [Qemu-devel] [PATCH v4 00/11] virtio: virtio-blk data plane Stefan Hajnoczi
2012-11-29 12:03   ` Paolo Bonzini
2012-11-29 14:09   ` Michael S. Tsirkin
2012-11-29 14:48     ` Stefan Hajnoczi
2012-11-29 15:19       ` 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=20121129125426.GC9372@redhat.com \
    --to=mst@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=asias@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=khoa@us.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).