[virtio-dev] Re: [virtio] Re: [virtio-comment] Re: [virtio] Re: [PATCH v10 04/10] admin: introduce virtio admin virtqueues

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 7425 bytes --]

On Wed, Mar 08, 2023 at 01:57:43PM +0100, Jiri Pirko wrote:
> Wed, Mar 08, 2023 at 01:44:18PM CET, stefanha@redhat.com wrote:
> >On Wed, Mar 08, 2023 at 11:17:35AM +0100, Jiri Pirko wrote:
> >> Tue, Mar 07, 2023 at 08:03:47PM CET, stefanha@redhat.com wrote:
> >> >On Tue, Mar 07, 2023 at 04:07:54PM +0100, Jiri Pirko wrote:
> >> >> Tue, Mar 07, 2023 at 03:39:11PM CET, stefanha@redhat.com wrote:
> >> >> >On Tue, Mar 07, 2023 at 09:03:18AM +0100, Jiri Pirko wrote:
> >> >> >> Mon, Mar 06, 2023 at 07:37:31PM CET, mst@redhat.com wrote:
> >> >> >> >On Mon, Mar 06, 2023 at 06:03:40AM -0500, Stefan Hajnoczi wrote:
> >> >> >> >> On Sun, Mar 05, 2023 at 07:18:24PM -0500, Michael S. Tsirkin wrote:
> >> >> >> >> > On Sun, Mar 05, 2023 at 07:03:02PM -0500, Stefan Hajnoczi wrote:
> >> >> >> >> > > On Sun, Mar 05, 2023 at 04:38:59AM -0500, Michael S. Tsirkin wrote:
> >> >> >> >> > > > On Fri, Mar 03, 2023 at 03:21:33PM -0500, Stefan Hajnoczi wrote:
> >> >> >> >> > > > > What happens if a command takes 1 second to complete, is the device
> >> >> >> >> > > > > allowed to process the next command from the virtqueue during this time,
> >> >> >> >> > > > > possibly completing it before the first command?
> >> >> >> >> > > > > 
> >> >> >> >> > > > > This requires additional clarification in the spec because "they are
> >> >> >> >> > > > > processed by the device in the order in which they are queued" does not
> >> >> >> >> > > > > explain whether commands block the virtqueue (in order completion) or
> >> >> >> >> > > > > not (out of order completion).
> >> >> >> >> > > > 
> >> >> >> >> > > > Oh I begin to see. Hmm how does e.g. virtio scsi handle this?
> >> >> >> >> > > 
> >> >> >> >> > > virtio-scsi, virtio-blk, and NVMe requests may complete out of order.
> >> >> >> >> > > Several may be processed by the device at the same time.
> >> >> >> >> > 
> >> >> >> >> > Let's say I submit a write followed by read - is read
> >> >> >> >> > guaranteed to return an up to date info?
> >> >> >> >> 
> >> >> >> >> In general, no. The driver must wait for the write completion before
> >> >> >> >> submitting the read if it wants consistency.
> >> >> >> >> 
> >> >> >> >> Stefan
> >> >> >> >
> >> >> >> >I see.  I think it's a good design to follow then.
> >> >> >> 
> >> >> >> Hmm, is it suitable to have this approach for configuration interface?
> >> >> >> Storage device is a different beast, having parallel reads and writes
> >> >> >> makes complete sense for performance.
> >> >> >> 
> >> >> >> ->read a req
> >> >> >> ->read b req
> >> >> >> ->read c req
> >> >> >> <-read a rep
> >> >> >> <-read b rep
> >> >> >> <-read c rep
> >> >> >> 
> >> >> >> There is no dependency, even between writes.
> >> >> >> 
> >> >> >> But in case of configuration, does not make any sense to me.
> >> >> >> Why is it needed? To pass the burden of consistency of
> >> >> >> configuration to driver sounds odd at least.
> >> >> >> 
> >> >> >> I sense there is no concete idea about what the "admin virtqueue" should
> >> >> >> serve for exactly.
> >> >> >
> >> >> >It's useful for long-running commands because they prevent other
> >> >> >commands from executing.
> >> >> >
> >> >> >An example I've given is that deleting a group member might require
> >> >> >waiting for the group member's I/O activity to finish. If that I/O
> >> >> >activity cannot be cancelled instantaneously, then it could take an
> >> >> >unbounded amount of time to delete the group member. The device would be
> >> >> >unable to process futher admin commands.
> >> >> 
> >> >> I see. Then I believe that the device should handle the dependencies.
> >> >> Example 1:
> >> >> -> REQ cmd to create group member A
> >> >> -> REQ cmd to create group member B
> >> >> <- REP cmd to create group member A
> >> >> <- REP cmd to create group member B
> >> >> 
> >> >> The device according to internal implementation can either serialize the
> >> >> 2 group member creations or do it in parallel, if it supports it.
> >> >> 
> >> >> Example 2:
> >> >> -> REQ cmd to create group member A
> >> >> -> REQ cmd config group member A
> >> >> <- REP cmd to create group member A
> >> >> <- REP cmd config group member A
> >> >> 
> >> >> Here the serialization is necessary and the device is the one to take
> >> >> care of it.
> >> >> 
> >> >> Makes sense?
> >> >
> >> >Yes, I understand. The spec would need to define ordering rules for
> >> >specific commands and the device must implement them. It allows the
> >> >driver to pipeline commands while also allowing out-of-order completion
> >> >(parallelism) in some cases. The disadvantage of this approach is
> >> >complexity in the spec and implementations.
> >> >
> >> >An alternative is unconditional out-of-order completion, where there are
> >> >no per-command ordering rules. The driver must wait for a command to
> >> >complete if it relies on the results of that command for its next
> >> >command. I like this approach because it's less complex in the spec and
> >> >for device implementers, while the burden on the driver implementer is
> >> >still reasonable.
> >> 
> >> But isn't this duplicating the burden of maintaining dependencies to
> >> both driver and device? I mean, device should not depend on driver doing
> >> the right thing, that means it has to check the dependencies for every
> >> incoming command anyway. The only difference would be to wait instead of
> >> returning "-EBUSY" in case the dependency is not satisfied yet.
> >
> >The device does not need to reject commands that have dependencies with
> >-EBUSY. The result of commands with dependencies is either A -> B or B
> >-> A.
> >
> >For example:
> >1. Create Group Member A
> >2. Delete Group Member A
> >
> >Command 2 might succeed or it might fail with -ENOENT because Group
> >Member A doesn't exist yet.
> 
> Yeah, correct.
> 
> >
> >> Device knows exactly what are the dependencies. And I believe, those are
> >> device implementation specific. For example, some implementation could
> >> support parallel VF config cmd execution, some implementation might
> >> need to serialize that. Driver has no clue.
> >
> >Yes, that's up to the device. Out-of-order completion is a superset of
> >in-order completion. So the device is allowed to run commands in series
> >when it wants. A driver designed for out-of-order completion will work
> >fine either way.
> 
> Agreed.
> 
> 
> >
> >> Could you please elaborate a bit more what you mean by "complexity in
> >> the spec"?
> >
> >When adding commands to the spec, the dependency relationships with
> >other commands need to be thought about and documented.
> >
> >Device implementers need to get those relationships right. That means
> >they need to remember that command B waits for command A.
> 
> That's my point, this is device implementation specific. Spec should not
> define anything like this, that might be limiting in some of the cases.
> 
> >
> >Driver implementers have to understand that command B waits for command
> >A but not command C according to the spec.
> >
> >That seems complex to me.
> 
> Sure, I agree this does not belong to the spec, it can't, really.
> For the record, I never suggested it.
> 
> I think we are in agreement.

Okay. In that case I think I misunderstood what you meant. Sorry!

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]