All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Liu Ping Fan <pingfank@linux.vnet.ibm.com>,
	Stefan Hajnoczi <stefanha@gmail.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	qemu-devel@nongnu.org, Anthony Liguori <anthony@codemonkey.ws>,
	Jan Kiszka <jan.kiszka@siemens.com>
Subject: Re: [Qemu-devel] [patch v4 05/16] memory: introduce ref, unref interface for MemoryRegionOps
Date: Tue, 23 Oct 2012 18:09:17 +0200	[thread overview]
Message-ID: <5086C12D.8000009@redhat.com> (raw)
In-Reply-To: <5086B708.6000209@redhat.com>

On 10/23/2012 05:26 PM, Paolo Bonzini wrote:
>>> Yes, that's the point of doing things asynchronously---you do not need
>>> to do everything within stop_machine, you can start canceling AIO as
>>> soon as the OS sends the hot-unplug request.  Then you only proceed with
>>> stop_machine and freeing device memory when the first part.
>> 
>> You cannot always cancel I/O (for example threaded I/O already in progress).
> 
> Yep, but we try to do this anyway today and nothing changes really.  The
> difference is between hotplug never completing and blocking
> synchronously, vs. hotplug never completing and not invoking the
> asynchronous callback.  I.e. really no difference at all.

Not cancelling is not the same as not completing; the request will
complete on its own eventually.  The question is whether the programming
model is synchronous or callback based.

> 
>>> In other words, isolate can complete asynchronously.
>> 
>> Can it?  I don't think so.
>> 
>> Here's how I see it:
>> 
>>  1. non-malicious guest stops driving device
>>  2. isolate()
>>  3. a malicious guest cannot drive the device at this point
>>  4. some kind of barrier to let the device, or drive activity from a
>> malicious guest, wind down
>>  5. destroy()
>> 
>> If you need to report the completion of step 2, it cannot be done
>> asynchronously.
> 
> In hardware everything is asynchronous anyway.  It will *look*
> synchronous, because if CPU#0 is stuck in a synchronous isolate(), and
> CPU#1 polls for the outcome, CPU#1 will lock on the BQL held by CPU#0.

That is fine.  isolate() is expensive but it is cpu bound, it does not
involve any I/O (unlike the barrier afterwards, which has to wait on any
I/O which we were not able to cancel).

> But our interfaces had better support asynchronicity, and indeed they
> do: after you write to the "eject" register, the "up" will show the
> device as present until after destroy is done.  This can be changed to
> show the device as present only until after step 4 is done.

Let's say we want to eject the hotplug hardware itself (just as an
example).  With refcounts, the callback that updates "up" will hold on
to to it via refcounts.  With stop_machine(), you need to cancel that
callback, or wait for it somehow, or it can arrive after the
stop_machine() and bite you.

> 
>> We may also want notification after step 4 (or 5); if the device holds
>> some host resource someone may want to know that it is ready for reuse.
> 
> I think guest notification should be after (4), while management
> notification should be after (5).

Yes. After (2) we can return from the eject mmio.

-- 
error compiling committee.c: too many arguments to function

  reply	other threads:[~2012-10-23 16:10 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-22  9:23 [Qemu-devel] [patch v4 00/16] push mmio dispatch out of big lock Liu Ping Fan
2012-10-22  9:23 ` [Qemu-devel] [patch v4 01/16] atomic: introduce atomic operations Liu Ping Fan
2012-10-22  9:23 ` [Qemu-devel] [patch v4 02/16] qom: apply atomic on object's refcount Liu Ping Fan
2012-10-22  9:23 ` [Qemu-devel] [patch v4 03/16] hotplug: introduce qdev_unplug_complete() to remove device from views Liu Ping Fan
2012-10-22  9:23 ` [Qemu-devel] [patch v4 04/16] pci: remove pci device from mem view when unplug Liu Ping Fan
2012-10-22  9:23 ` [Qemu-devel] [patch v4 05/16] memory: introduce ref, unref interface for MemoryRegionOps Liu Ping Fan
2012-10-22  9:38   ` Avi Kivity
2012-10-23 11:51     ` Paolo Bonzini
2012-10-23 11:55       ` Avi Kivity
2012-10-23 11:57         ` Paolo Bonzini
2012-10-23 12:02           ` Avi Kivity
2012-10-23 12:06             ` Paolo Bonzini
2012-10-23 12:15               ` Avi Kivity
2012-10-23 12:32                 ` Paolo Bonzini
2012-10-23 14:49                   ` Avi Kivity
2012-10-23 15:26                     ` Paolo Bonzini
2012-10-23 16:09                       ` Avi Kivity [this message]
2012-10-24  7:29                         ` Paolo Bonzini
2012-10-25 16:28                           ` Avi Kivity
2012-10-26 15:05                             ` Paolo Bonzini
2012-10-23 12:04         ` Jan Kiszka
2012-10-23 12:12           ` Paolo Bonzini
2012-10-23 12:16             ` Jan Kiszka
2012-10-23 12:28               ` Avi Kivity
2012-10-23 12:40                 ` Jan Kiszka
2012-10-23 14:37                   ` Avi Kivity
2012-10-22  9:23 ` [Qemu-devel] [patch v4 06/16] memory: document ref, unref interface Liu Ping Fan
2012-10-22  9:23 ` [Qemu-devel] [patch v4 07/16] memory: make mmio dispatch able to be out of biglock Liu Ping Fan
2012-10-23 12:12   ` Jan Kiszka
2012-10-23 12:36     ` Avi Kivity
2012-10-24  6:31       ` liu ping fan
2012-10-24  6:56         ` liu ping fan
2012-10-25  8:57           ` Avi Kivity
2012-10-22  9:23 ` [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info Liu Ping Fan
2012-10-22  9:30   ` Jan Kiszka
2012-10-22 17:13     ` Peter Maydell
2012-10-23  5:58       ` liu ping fan
2012-10-23 11:48       ` Paolo Bonzini
2012-10-23 11:50         ` Peter Maydell
2012-10-23 11:51           ` Jan Kiszka
2012-10-23 12:00           ` Paolo Bonzini
2012-10-23 12:27             ` Peter Maydell
2012-11-18 10:02             ` Brad Smith
2012-11-18 16:14               ` Paolo Bonzini
2012-11-18 16:15                 ` Paolo Bonzini
2012-10-22  9:23 ` [Qemu-devel] [patch v4 09/16] memory: introduce mmio request pending to anti nested DMA Liu Ping Fan
2012-10-22 10:28   ` Avi Kivity
2012-10-23 12:38   ` Gleb Natapov
2012-10-24  6:31     ` liu ping fan
2012-10-22  9:23 ` [Qemu-devel] [patch v4 10/16] memory: introduce lock ops for MemoryRegionOps Liu Ping Fan
2012-10-22 10:30   ` Avi Kivity
2012-10-23  5:53     ` liu ping fan
2012-10-23  8:53       ` Jan Kiszka
2012-10-22  9:23 ` [Qemu-devel] [patch v4 11/16] vcpu: push mmio dispatcher out of big lock Liu Ping Fan
2012-10-22 10:31   ` Avi Kivity
2012-10-22 10:36     ` Jan Kiszka
2012-10-22  9:23 ` [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000 Liu Ping Fan
2012-10-22 10:37   ` Avi Kivity
2012-10-23  9:04   ` Jan Kiszka
2012-10-24  6:31     ` liu ping fan
2012-10-24  7:17       ` Jan Kiszka
2012-10-25  9:01         ` Avi Kivity
2012-10-25  9:31           ` Jan Kiszka
2012-10-25 16:21             ` Avi Kivity
2012-10-25 16:39               ` Jan Kiszka
2012-10-25 17:02                 ` Avi Kivity
2012-10-25 18:48                   ` Jan Kiszka
2012-10-29  5:24                     ` liu ping fan
2012-10-24  7:29     ` liu ping fan
2012-10-25 13:34       ` Jan Kiszka
2012-10-25 16:23         ` Avi Kivity
2012-10-25 16:41           ` Jan Kiszka
2012-10-25 17:03             ` Avi Kivity
2012-10-29  5:24         ` liu ping fan
2012-10-31  7:03           ` Jan Kiszka
2012-10-22  9:23 ` [Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state Liu Ping Fan
2012-10-22 10:40   ` Avi Kivity
2012-10-23  5:52     ` liu ping fan
2012-10-23  9:06       ` Avi Kivity
2012-10-23  9:07       ` Jan Kiszka
2012-10-23  9:32         ` liu ping fan
2012-10-23  9:37           ` Avi Kivity
2012-10-24  6:36             ` liu ping fan
2012-10-25  8:55               ` Avi Kivity
2012-10-25  9:00             ` Peter Maydell
2012-10-25  9:04               ` Avi Kivity
2012-10-26  3:05                 ` liu ping fan
2012-10-26  3:08                   ` liu ping fan
2012-10-26 10:25                     ` Jan Kiszka
2012-10-29  5:24                       ` liu ping fan
2012-10-29  7:50                         ` Peter Maydell
2012-10-22  9:23 ` [Qemu-devel] [patch v4 14/16] qdev: introduce stopping state Liu Ping Fan
2012-10-22  9:23 ` [Qemu-devel] [patch v4 15/16] e1000: introduce unmap() to fix unplug issue Liu Ping Fan
2012-10-22  9:23 ` [Qemu-devel] [patch v4 16/16] e1000: implement MemoryRegionOps's ref&lock interface Liu Ping Fan
2012-10-25 14:04 ` [Qemu-devel] [patch v4 00/16] push mmio dispatch out of big lock Peter Maydell
2012-10-25 16:44   ` Jan Kiszka
2012-10-25 17:07   ` Avi Kivity
2012-10-25 17:13     ` Peter Maydell
2012-10-25 18:13       ` Marcelo Tosatti
2012-10-25 19:00         ` Jan Kiszka
2012-10-25 19:06           ` Peter Maydell
2012-10-29 15:24       ` Avi Kivity

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=5086C12D.8000009@redhat.com \
    --to=avi@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=jan.kiszka@siemens.com \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=pingfank@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.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.