Re: [virtio] [PATCH 2/5] feedback: drop use of "we"

[Rusty Russell <rusty@au1.ibm.com>]

"Michael S. Tsirkin" <mst@redhat.com> writes:
> explicitly say driver/device or use passive form
> as appropriate.

Indeed.  Much of this was already done in commit:

3.2.1: Language tightening.
Git: 97776cc2fbe8da7495f90e5b19046b5546f7bf27
SVN: r201

I've contrasted below, but they're basically the same.

> -The head of the buffer we mapped is the first d in the algorithm
> +The head of the mapped buffer is the first d in the algorithm
>  above (the descriptor chain head).  A naive implementation would do the following (with the
>  appropriate conversion to-and-from little-endian assumed):

This ended up being:

The descriptor chain head is the first d in the algorithm
above, ie. the index of the descriptor table entry referring to the first
part of the buffer.  A naive implementation would do the following (with the


> @@ -652,9 +652,10 @@ appropriate conversion to-and-from little-endian assumed):
>  	avail->ring[avail->idx % qsz] = head;
>  \end{lstlisting}
>  
> -However, in general we can add many descriptor chains before we update
> +However, in general it is posible to add many descriptor chains
> +before updating

However, in general the driver can add many descriptor chains before it updates

>  the “idx” field (at which point they become visible to the
> -device), so we keep a counter of how many we've added:
> +device) by keeping a counter of how many have been added:

device), so it is common to keep a counter of how many the driver has added:

>  \begin{lstlisting}
>  	avail->ring[(avail->idx + added++) % qsz] = head;
> @@ -663,12 +664,12 @@ device), so we keep a counter of how many we've added:
>  \subsubsection{Updating The Index Field}\label{sec:General Initialization And Device Operation / Device Operation / Supplying Buffers to The Device / Updating The Index Field}
>  
>  Once the index field of the virtqueue is updated, the device will
> -be able to access the descriptor chains we've created and the
> +be able to access the descriptor chains that driver created and the
>  memory they refer to. This is why a memory barrier is generally
>  used before the index update, to ensure it sees the most up-to-date

Same.

>  copy.
>  
> -The index field always increments, and we let it wrap naturally at
> +The index field always increments, and wraps naturally at
>  65536:

The index field always increments, and the driver can let it wrap naturally at

>  \begin{lstlisting}
> @@ -682,17 +683,18 @@ it can be expensive.  So the device can suppress such notifications if it
>  doesn't need them.  The driver has to be careful to expose the new index
>  value before checking if notifications are suppressed: it's OK to notify
>  gratuitously, but not to omit a required notification. So again,
> -we use a memory barrier here before reading the flags or the
> +driver uses a memory barrier here before reading the flags or the
>  avail_event field.

the driver SHOULD use a memory barrier here before reading the flags or the

>  If the VIRTIO_F_RING_EVENT_IDX feature is not negotiated, and if the
>  VRING_USED_F_NOTIFY flag is not set, the driver SHOULD notify the
>  device.
>  
> -If the VIRTIO_F_RING_EVENT_IDX feature is negotiated, we read the
> +If the VIRTIO_F_RING_EVENT_IDX feature is negotiated, driver
> +reads the
>  avail_event field in the available ring structure. If the

Same.

>  available index crossed_the avail_event field value since the
> -last notification, we go ahead and write to the PCI configuration
> +last notification, driver will go ahead and write to the PCI configuration

available index crossed_the avail_event field value since the
last notification, the driver SHOULD notify the device.  The avail_event field 

Thanks,
Rusty.