All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Wang <jasowang@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Halil Pasic <pasic@linux.ibm.com>,
	Cornelia Huck <cohuck@redhat.com>,
	stefanha@redhat.com, virtio-comment@lists.oasis-open.org,
	eperezma@redhat.com, sgarzare@redhat.com
Subject: Re: [virtio-comment] [PATCH RFC] virtio: introduce VIRTIO_F_DEVICE_STOP
Date: Tue, 29 Dec 2020 04:04:27 -0500 (EST)	[thread overview]
Message-ID: <661905925.40710498.1609232667286.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <20201228072749-mutt-send-email-mst@kernel.org>



----- Original Message -----
> On Mon, Dec 28, 2020 at 03:01:57PM +0800, Jason Wang wrote:
> > 
> > On 2020/12/28 下午2:21, Halil Pasic wrote:
> > > On Sun, 27 Dec 2020 05:00:05 -0500
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > On Fri, Dec 25, 2020 at 08:38:35AM +0100, Halil Pasic wrote:
> > > > > When driver is trying to set DEVICE_STOPPED, the device MUST not
> > > > > process new avail requests and MUST complete all requests that is
> > > > > currently processing before setting DEVICE_STOPPED.
> > > > ...
> > > > 
> > > > > Since we have a synchronous API setting DEVICE_STOPPED would also
> > > > > have to
> > > > > block until all in-flight requests are completed.
> > > > Judging from the surrounding discussion,
> > > > when you say complete you seem to mean "use", and
> > > My intention was merely to paraphrase Jason's proposal which says:
> > > 
> > > +When driver is trying to set DEVICE_STOPPED, the device MUST not
> > > +process new avail requests and MUST complete all requests that is
> > > +currently processing before setting DEVICE_STOPPED.
> > > 
> > > > I'm not sure how you define in flight, but
> > > > it seems there could be many of these (up to a full queue?)
> > > In the follow-on discussion 'in-flight' emerged as an alternative to
> > > 'all requests that is currently processing' form the proposed text.
> > > 
> > > A large part of the discussion is, IMHO, about finding precise
> > > definitions, for what the quoted paragraph is trying to express.
> > > 
> > > > so waiting for all available buffers to be
> > > > used might indeed require an asynchronous interface,
> > > > which gets complex very quickly.
> > 
> > 
> > Some part of the virtio has enforced an asynchronous interface during
> > reset:
> > 
> > For MMIO the spec said:
> > 
> > """
> > 
> > To stop using the queue the driver MUST write zero (0x0) to this QueueReady
> > and MUST read the value back to ensure synchronization.
> > 
> > """
> > 
> > For PCI it said:
> > 
> > """
> > 
> > After writing 0 to device_status, the driver MUST wait for a read of
> > device_status to return 0 before reinitializing the device.
> > 
> > """
> 
> Absolutely. It's ok for simple things.
> However doing anything major like waiting for IO
> in a tight loop is a problem.

It's mainly used for Qemu. And driver can choose to cancel the stop by
trying to clear the bit. Does this work?


> 
> 
> 
> > 
> > > > 
> > > > However wouldn't it be possible for device to just cancel
> > > > processing available buffers even if it started processing
> > > > them? Some buffers could be in indeterminate state
> > > > (e.g. we might not have a way to know how much data did
> > > > device have time to write into a buffer).
> > > > 
> > > Maybe Jason can answer this one.
> > 
> > 
> > For networking device, it should be possible (packet could be dropped or
> > duplicated). But I'm not sure it works for other device. E.g for the block
> > devices that needs to communicate with a remote backend. Waat's more
> > important, my understadning is that the intermediate state is something
> > that
> > we need to avoid (hard to be migrated anyhow).
> 
> The difficulty is on the device side though, so why not say: if it wants to
> flush IO that's up to it.

Do you mean to introduce a device specific way to flush IO? If yes, it
actually introduces a new intermediate state implicitly:

1) device is stopped
2) device is stopped and IO is flushed

This looks more complicated than a single new state:

1) device is stopped and IO is flushed

Thanks

> 
> > 
> > > 
> > > > Making it clearer what does "complete" mean here might help.
> > > > 
> > > I think what we are trying to accomplish here, is avoiding, having
> > > non-standardised state in device (that can not be dropped) when
> > > migrating.
> > > 
> > > I'm still struggling with wrapping my mind around the problem. AFAIU
> > > migration and migratability is not really a feature of the virtio
> > > standard, but can be a feature of it's implementation
> > > (e.g. QEMU & KVM), where the standard does help a great deal by having
> > > certain aspects of the operation and interaction nailed down.
> > 
> > 
> > It looks like a must (at least from the level of semantics) for designing
> > software API for vDPA. Using a standard spec may help to avoid subtle
> > misunderstanding of different vendors.
> > 
> > Thanks
> > 
> > 
> > > 
> > > Regards,
> > > Halil
> > > 
> 
> 


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


  reply	other threads:[~2020-12-29  9:04 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-18  4:23 [virtio-comment] [PATCH RFC] virtio: introduce VIRTIO_F_DEVICE_STOP Jason Wang
2020-12-18 10:15 ` [virtio-comment] " Stefano Garzarella
2020-12-21  3:08   ` Jason Wang
2020-12-21 11:06     ` Stefano Garzarella
2020-12-22  2:38       ` Jason Wang
2020-12-21 21:33 ` [virtio-comment] " Halil Pasic
2020-12-22  2:36   ` Jason Wang
2020-12-22  6:50     ` Halil Pasic
2020-12-22  7:30       ` Jason Wang
2020-12-22 12:14         ` Cornelia Huck
2020-12-22 12:51           ` Jason Wang
2020-12-22 15:54             ` Cornelia Huck
2020-12-23  2:48               ` Jason Wang
2020-12-25  7:38                 ` Halil Pasic
2020-12-27 10:00                   ` Michael S. Tsirkin
2020-12-28  6:21                     ` Halil Pasic
2020-12-28  7:01                       ` Jason Wang
2020-12-28 12:30                         ` Michael S. Tsirkin
2020-12-29  9:04                           ` Jason Wang [this message]
2021-01-12 10:54                             ` Michael S. Tsirkin
2021-01-13  3:35                               ` Jason Wang
2020-12-29 13:35                         ` Halil Pasic
2020-12-30  8:15                           ` Jason Wang
2021-01-11 18:16                             ` Cornelia Huck
2021-01-12  3:27                               ` Jason Wang
2021-01-12 12:13                                 ` Cornelia Huck
2021-01-13  2:52                                   ` Jason Wang
2021-01-14 12:00                                     ` Cornelia Huck
2020-12-28  6:47                   ` Jason Wang
2020-12-29 13:20                     ` Halil Pasic
2020-12-30  8:03                       ` Jason Wang
2020-12-24  4:52             ` Halil Pasic
2020-12-24  5:51               ` Jason Wang
2020-12-25  3:18                 ` Halil Pasic
2020-12-25  6:45                   ` Jason Wang
2020-12-27 11:12                     ` Michael S. Tsirkin
2020-12-28  7:05                       ` Jason Wang
2020-12-28 12:27                         ` Michael S. Tsirkin
2020-12-29  8:57                           ` Jason Wang
2021-05-03  9:02 ` [virtio-comment] " Eugenio Perez Martin
2021-05-06  2:51   ` Jason Wang
2021-05-05 13:16 ` Michael S. Tsirkin
2021-05-06  7:26   ` Jason Wang

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=661905925.40710498.1609232667286.JavaMail.zimbra@redhat.com \
    --to=jasowang@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=eperezma@redhat.com \
    --cc=mst@redhat.com \
    --cc=pasic@linux.ibm.com \
    --cc=sgarzare@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=virtio-comment@lists.oasis-open.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.