On 06/13/2018 03:04 PM, Stefan Hajnoczi wrote:
> On Mon, Jun 11, 2018 at 06:11:53PM +0200, Halil Pasic wrote:
>> @@ -65,11 +65,12 @@ in which their processing is complete.
>>
>>
The Device Event Suppression data structure is write-only by the
>>
device. It includes information for reducing the number of
>> -device events - i.e. driver notifications to device.
>> +device events - i.e. sending an available buffer notification
>> +to the device.
>>
>>
The Driver Event Suppression data structure is read-only by the
>>
device. It includes information for reducing the number of
>> -driver events - i.e. device interrupts to driver.
>> +driver events - i.e. send a used buffer notification to the driver.
>
> This makes the "i.e." more natural:
>
> "sending fewer used buffer notifications to the driver"
>
> because the "send a used buffer notification to the driver" phrase
> doesn't directly relate to any preceding subphrase ("driver events",
> "number of driver events", or "reducing the number of driver events").
>
Will do. My sentence does not make sense.
>>
>>
\subsection{Driver and Device Ring Wrap Counters}
>>
\label{sec:Packed Virtqueues / Driver and Device Ring Wrap Counters}
>> @@ -309,22 +310,20 @@ in the ring.
>>
>>
\subsection{Driver and Device Event Suppression}
>>
\label{sec:Packed Virtqueues / Driver and Device Event Suppression}
>> -In many systems driver and device notifications involve
>> +In many systems used and available buffer notifications involve
>>
significant overhead. To mitigate this overhead,
>>
each virtqueue includes two identical structures used for
>>
controlling notifications between the device and the driver.
>>
>>
The Driver Event Suppression structure is read-only by the
>> -device and controls the events sent by the device
>> -to the driver (e.g. interrupts).
>> +device and controls the used buffer notifications (sent by the device
>> +to the driver).
>>
>>
The Device Event Suppression structure is read-only by
>> -the driver and controls the events sent by the driver
>> -to the device (e.g. IO).
>> +the driver and controls the available buffer notifications (sent by the
>> +driver to the device).
>>
>> -Each of these Event Suppression structures controls
>> -both Descriptor Ring events and structure events, and
>> -each includes the following fields:
>> +Each of these Event Suppression includes the following fields:
>
> Something is missing, "these" is plural but Event Suppression is
> singular:
>
> Each of these Event Suppression structures ...
>
Will put back structures. Connie spotted it too.
>>
Otherwise, if the VIRTIO_F_EVENT_IDX feature bit is negotiated:
>>
\begin{itemize}
>>
\item The driver MUST set \field{flags} to 0.
>> -\item The driver MAY use \field{used_event} to advise the device that interrupts are unnecessary until the device writes entry with an index specified by \field{used_event} into the used ring (equivalently, until \field{idx} in the
>> +\item The driver MAY use \field{used_event} to advise the device that
>> +notifications are unnecessary until the device writes entry with an index specified by \field{used_event} into the used ring (equivalently, until \field{idx} in the
>
> Pre-existing issue:
>
>
the device writes *an* entry
>
I will fix it if already touching the sentence. Thanks
for spotting it!
>> @@ -562,8 +570,8 @@ The driver offers buffers to one of the device's virtqueues as follows:
>>
\item The driver performs a suitable memory barrier to ensure that it updates
>>
the \field{idx} field before checking for notification suppression.
>>
>> -\item If notifications are not suppressed, the driver notifies the device
>> -
of the new available buffers.
>> +\item The driver sends an available buffers notification to the device if
>> +
such notifications are not suppressed.
>
> For consistency:
>
> s/available buffers notification/available buffer notification/
>
Sorry, I missed this one. Will fix.
Thanks for your review!
Regards,
Halil
