[virtio] [PATCH] VIRTIO_F_RING_PACKED_USED: packing used descriptors

[virtio] [PATCH] VIRTIO_F_RING_PACKED_USED: packing used descriptors

[Michael S. Tsirkin]

Sometimes the device needs to only write out a single used descriptor
after processing a batch of multiple available descriptors.
This can happen when using
descriptor chaining or with in-order
use of descriptors.  In this case, the device writes out a single used
descriptor with the buffer id of the last descriptor in the batch.

When this happens, used descriptors stop being
contiguous in memory. As a result each used descriptor
needs a separate memory write. For devices on the pci express bus,
this incurs overhead of a separate write trasaction
(e.g. pci express headers etc).

This patch proposes a new feature: VIRTIO_F_RING_PACKED_USED.
The idea is that both device and driver count all of the
descriptors in a batch as if all of them are being written out
when used.  The resulting batch counter controls
wrap around to beginning of the ring, which then serves
as a synchronization point.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 content.tex     |  3 +++
 packed-ring.tex | 27 +++++++++++++++++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/content.tex b/content.tex
index 47a4c6a..5b3121e 100644
--- a/content.tex
+++ b/content.tex
@@ -6247,6 +6247,9 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
   \item[VIRTIO_F_ORDER_PLATFORM(36)] This feature indicates
   that memory accesses by the driver and the device are ordered
   in a way described by the platform.
+  \item[VIRTIO_F_RING_PACKED_USED(37)] This feature indicates
+  support for packed write out of used descriptors. Requires
+  VIRTIO_F_RING_PACKED.
 
   If this feature bit is negotiated, the ordering in effect for any
   memory accesses by the driver that need to be ordered in a specific way
diff --git a/packed-ring.tex b/packed-ring.tex
index ea92543..f8a2676 100644
--- a/packed-ring.tex
+++ b/packed-ring.tex
@@ -131,17 +131,36 @@ \subsection{Polling of available and used descriptors}
 poll (or test) a single location in memory: the next device descriptor after
 the one they processed previously, in circular order.
 
+\subsection{Skipping Used Descriptors}
+\label{sec:Packed Virtqueues / Skipping Used Descriptors}
 Sometimes the device needs to only write out a single used descriptor
 after processing a batch of multiple available descriptors.  As
 described in more detail below, this can happen when using
 descriptor chaining or with in-order
-use of descriptors.  In this case, the device writes out a used
-descriptor with the buffer id of the last descriptor in the group.
-After processing the used descriptor, both device and driver then
+use of descriptors.  In this case, the device writes out a single used
+descriptor with the buffer id of the last descriptor in the batch.
+
+If VIRTIO_F_RING_PACKED_USED has not been negotiated,
+after processing the used descriptor, both device and driver then
 skip forward in the ring the number of the remaining descriptors
-in the group until processing (reading for the driver and writing
+in the batch until processing (reading for the driver and writing
 for the device) the next used descriptor.
 
+If VIRTIO_F_RING_PACKED_USED has been negotiated,
+both the device and the driver are expected to maintain,
+internally, a 16-bit used batch counter initialized to 0.
+Each time a batch of descriptors is used, the batch counter
+is incremented by the number of descriptors in the batch.
+If after processing the used descriptor, the batch counter
+does not exceed the ring size, the next used descriptor
+is written immediately adjacent to the current one, in ring
+order.
+When after processing the used descriptor, the batch counter
+exceeds the ring size, the batch counter is decremented by the
+ring size; both device and driver then skip to the
+beginning (offset 0) of the ring until processing (reading for
+the driver and writing for the device) the next used descriptor.
+
 \subsection{Write Flag}
 \label{sec:Packed Virtqueues / Write Flag}
 
-- 
MST


---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that 
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php 

[virtio] Re: [virtio-dev] [PATCH] VIRTIO_F_RING_PACKED_USED: packing used descriptors

[Stefan Hajnoczi]

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

On Thu, Oct 29, 2020 at 03:50:57AM -0400, Michael S. Tsirkin wrote:
> +When after processing the used descriptor, the batch counter
> +exceeds the ring size, the batch counter is decremented by the
> +ring size; both device and driver then skip to the
> +beginning (offset 0) of the ring until processing (reading for
> +the driver and writing for the device) the next used descriptor.

This may be clearer:

  both device and driver then use ring index 0 for the next used
  descriptor

Or:

  The device writes the next used descriptor at ring index 0. The driver
  reads the next used descriptor from ring index 0.

I'm not sure I understand this scheme. Is this pseudo-code correct?

  batch_counter += num_used_descs
  if batch_counter > ring_size:
      batch_counter -= ring_size
      used_idx = 0

Can you share an explanation of how this scheme ensures that the
driver's next avail descriptor index never overtakes the device's next
used descriptor index?

Previously it was obvious that the two never cross because the used
descriptor index jumped by the number of avail descriptors that were
processed by the device.

Stefan

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

Re: [virtio] Re: [virtio-dev] [PATCH] VIRTIO_F_RING_PACKED_USED: packing used descriptors

[Michael S. Tsirkin]

On Fri, Oct 30, 2020 at 11:37:32AM +0000, Stefan Hajnoczi wrote:
> On Thu, Oct 29, 2020 at 03:50:57AM -0400, Michael S. Tsirkin wrote:
> > +When after processing the used descriptor, the batch counter
> > +exceeds the ring size, the batch counter is decremented by the
> > +ring size; both device and driver then skip to the
> > +beginning (offset 0) of the ring until processing (reading for
> > +the driver and writing for the device) the next used descriptor.
> 
> This may be clearer:
> 
>   both device and driver then use ring index 0 for the next used
>   descriptor
> 
> Or:
> 
>   The device writes the next used descriptor at ring index 0. The driver
>   reads the next used descriptor from ring index 0.
> 
> I'm not sure I understand this scheme. Is this pseudo-code correct?
> 
>   batch_counter += num_used_descs
>   if batch_counter > ring_size:
>       batch_counter -= ring_size
>       used_idx = 0

yes


> Can you share an explanation of how this scheme ensures that the
> driver's next avail descriptor index never overtakes the device's next
> used descriptor index?

Simply because to overtake it, it needs to wrap around to 0.
That happens when batch counter wraps over to 0, if we make
device wrap at that point all is well.


> Previously it was obvious that the two never cross because the used
> descriptor index jumped by the number of avail descriptors that were
> processed by the device.
> 
> Stefan

it still does that, just at the time of wrap around, not
after each descriptor.


---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that 
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php 

Re: [virtio] Re: [virtio-dev] [PATCH] VIRTIO_F_RING_PACKED_USED: packing used descriptors

[Stefan Hajnoczi]

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

On Fri, Oct 30, 2020 at 11:01:10AM -0400, Michael S. Tsirkin wrote:
> On Fri, Oct 30, 2020 at 11:37:32AM +0000, Stefan Hajnoczi wrote:
> > On Thu, Oct 29, 2020 at 03:50:57AM -0400, Michael S. Tsirkin wrote:
> > > +When after processing the used descriptor, the batch counter
> > > +exceeds the ring size, the batch counter is decremented by the
> > > +ring size; both device and driver then skip to the
> > > +beginning (offset 0) of the ring until processing (reading for
> > > +the driver and writing for the device) the next used descriptor.
> > 
> > This may be clearer:
> > 
> >   both device and driver then use ring index 0 for the next used
> >   descriptor
> > 
> > Or:
> > 
> >   The device writes the next used descriptor at ring index 0. The driver
> >   reads the next used descriptor from ring index 0.
> > 
> > I'm not sure I understand this scheme. Is this pseudo-code correct?
> > 
> >   batch_counter += num_used_descs
> >   if batch_counter > ring_size:
> >       batch_counter -= ring_size
> >       used_idx = 0
> 
> yes
> 
> 
> > Can you share an explanation of how this scheme ensures that the
> > driver's next avail descriptor index never overtakes the device's next
> > used descriptor index?
> 
> Simply because to overtake it, it needs to wrap around to 0.
> That happens when batch counter wraps over to 0, if we make
> device wrap at that point all is well.

What confuses me is that batch_counter is not reset to 0. It is set to
batch_counter -= ring_size.

I guess I need to get my crayons and draw pictures :-).

Stefan

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

Re: [virtio] Re: [virtio-dev] [PATCH] VIRTIO_F_RING_PACKED_USED: packing used descriptors

[Michael S. Tsirkin]

On Tue, Nov 03, 2020 at 05:18:34PM +0000, Stefan Hajnoczi wrote:
> On Fri, Oct 30, 2020 at 11:01:10AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Oct 30, 2020 at 11:37:32AM +0000, Stefan Hajnoczi wrote:
> > > On Thu, Oct 29, 2020 at 03:50:57AM -0400, Michael S. Tsirkin wrote:
> > > > +When after processing the used descriptor, the batch counter
> > > > +exceeds the ring size, the batch counter is decremented by the
> > > > +ring size; both device and driver then skip to the
> > > > +beginning (offset 0) of the ring until processing (reading for
> > > > +the driver and writing for the device) the next used descriptor.
> > > 
> > > This may be clearer:
> > > 
> > >   both device and driver then use ring index 0 for the next used
> > >   descriptor
> > > 
> > > Or:
> > > 
> > >   The device writes the next used descriptor at ring index 0. The driver
> > >   reads the next used descriptor from ring index 0.
> > > 
> > > I'm not sure I understand this scheme. Is this pseudo-code correct?
> > > 
> > >   batch_counter += num_used_descs
> > >   if batch_counter > ring_size:
> > >       batch_counter -= ring_size
> > >       used_idx = 0
> > 
> > yes
> > 
> > 
> > > Can you share an explanation of how this scheme ensures that the
> > > driver's next avail descriptor index never overtakes the device's next
> > > used descriptor index?
> > 
> > Simply because to overtake it, it needs to wrap around to 0.
> > That happens when batch counter wraps over to 0, if we make
> > device wrap at that point all is well.
> 
> What confuses me is that batch_counter is not reset to 0. It is set to
> batch_counter -= ring_size.
> 
> I guess I need to get my crayons and draw pictures :-).
> 
> Stefan



The idea is that when all available buffers have been used then
batch counter of the device matches the next avail index of the driver.


---------------------------------------------------------------------
To unsubscribe from this mail list, you must leave the OASIS TC that 
generates this mail.  Follow this link to all your TCs in OASIS at:
https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php