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

[Stefan Hajnoczi <stefanha@redhat.com>]

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.

> 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.

> 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.

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.

Stefan

Attachment: signature.asc
Description: PGP signature