[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH v1 1/2] content/virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
On Tue, Nov 13, 2018 at 06:27:46PM +0800, Wei Wang wrote:
> The VIRTIO_BALLOON_F_FREE_PAGE_HINT feature supports driver reporting
> free page hints to the device.
>
> Live migration can use the virtio-balloon device to get the guest free
> page hints, and avoid sending those free pages. It is not concerned that
> the memory pages are used (e.g. guest reclaimed some of them due to memory
> pressure) after they are given to the device as hints of free pages,
> because they will be tracked by the hypervisor and transferred in the
> subsequent round if they are used and written.
>
> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---
> content.tex | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 116 insertions(+), 6 deletions(-)
>
> diff --git a/content.tex b/content.tex
> index c346183..673c891 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -4367,9 +4367,9 @@ The traditional virtio memory balloon device is a primitive device for
> managing guest memory: the device asks for a certain amount of
> memory, and the driver supplies it (or withdraws it, if the device
> has more than it asks for). This allows the guest to adapt to
> -changes in allowance of underlying physical memory. If the
> -feature is negotiated, the device can also be used to communicate
> -guest memory statistics to the host.
> +changes in allowance of underlying physical memory. The device can
> +also be used to communicate guest memory statistics and guest free
> +memory pages to the host when the related features are negotiated.
Do we have to mention "pages"? How about "free memory" everywhere?
>
> \subsection{Device ID}\label{sec:Device Types / Memory Balloon Device / Device ID}
> 5
> @@ -4378,11 +4378,14 @@ guest memory statistics to the host.
> \begin{description}
> \item[0] inflateq
> \item[1] deflateq
> -\item[2] statsq.
> +\item[2] statsq
> +\item[3] freepageq
> \end{description}
>
> Virtqueue 2 only exists if VIRTIO_BALLON_F_STATS_VQ set.
>
> + Virtqueue 3 only exists if VIRTIO_BALLON_F_FREE_PAGE_HINT set.
> +
> \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Feature bits}
> \begin{description}
> \item[VIRTIO_BALLOON_F_MUST_TELL_HOST (0)] Host has to be told before
> @@ -4392,6 +4395,8 @@ guest memory statistics to the host.
> memory statistics is present.
> \item[VIRTIO_BALLOON_F_DEFLATE_ON_OOM (2) ] Deflate balloon on
> guest out of memory condition.
> +\item[VIRTIO_BALLOON_F_FREE_PAGE_HINT(3) ] Guest reports free pages
> + to host.
>
> \end{description}
>
> @@ -4415,16 +4420,32 @@ allow guest to use memory before notifying host if
> VIRTIO_BALLOON_F_MUST_TELL_HOST is not negotiated.
>
> \subsection{Device configuration layout}\label{sec:Device Types / Memory Balloon Device / Device configuration layout}
> - Both fields of this configuration
> - are always available.
> +
> +\begin{lstlisting}
> +#define VIRTIO_BALLOON_CMD_ID_MIN 0x80000000
Let's define MAX too for consistency?
> +#define VIRTIO_BALLOON_CMD_ID_STOP 0x0
> +#define VIRTIO_BALLOON_CMD_ID_DONE 0x1
> +\end{lstlisting}
>
> \begin{lstlisting}
> struct virtio_balloon_config {
> le32 num_pages;
> le32 actual;
> + le32 free_page_report_cmd_id;
> };
> \end{lstlisting}
>
> +\field{num_pages} and \field{actual} are always available.
> +
> +\field{free_page_report_cmd_id} is available if
> +VIRTIO_BALLON_F_FREE_PAGE_HINT negotiated.
> +
> +\devicenormative{\subsubsection}{Device configuration layout}{Device Types / Traditional Memory Balloon Device / Device configuration layout}
> +
> +The device MUST NOT set \field{free_page_report_cmd_id} to a value
> +between \field{VIRTIO_BALLOON_CMD_ID_DONE} and
> +\field{VIRTIO_BALLOON_CMD_ID_MIN} exclusive.
Exclusive is confusing since you can include one edge or
both.
Let's write it up positively. So the only legal values are
VIRTIO_BALLOON_CMD_ID_MIN to VIRTIO_BALLOON_CMD_ID_MAX
inclusive or VIRTIO_BALLOON_CMD_ID_STOP?
> +
> \subparagraph{Legacy Interface: Device configuration layout}\label{sec:Device Types / Memory Balloon Device / Device
> configuration layout / Legacy Interface: Device configuration layout}
> When using the legacy interface, transitional devices and drivers
> @@ -4448,8 +4469,20 @@ The device initialization process is outlined below:
> \item DRIVER_OK is set: device operation begins.
> \item Notify the device about the stats virtqueue buffer.
> \end{enumerate}
> +
> +\item If the VIRTIO_BALLOON_F_FREE_PAGE_HINT feature bit is
> + negotiated:
> + \begin{enumerate}
> + \item Identify the free page virtqueue.
> + \item Set \field{free_page_report_cmd_id} to
> + \field{VIRTIO_BALLOON_CMD_ID_MIN}.
Does driver does it or the device?
And does it have to be MIN? Why?
Should not driver *read* the ID instead?
> + \item Register a notifier with an external component (e.g. a live
> + migration agent) who will request for the free page reporting
> + from the driver.
Spec only operates in terms of driver and device.
I don't know what does above mean in these terms
but please restate it accordingly.
No notifiers live migration agents and external (to what?) components please.
> + \end{enumerate}
> \end{enumerate}
>
> +
don't add more empty lines pls.
> \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation}
>
> The device is driven either by the receipt of a configuration
> @@ -4718,6 +4751,83 @@ first buffer.
> allocations in the guest.
> \end{description}
>
> +\subsubsection{Free Page Reporting}\label{sec:Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> +
> +The free page reporting is driven by the device. The device MAY
> +request the driver to start or stop the free page reporting.
> +
> +A request to start the free page reporting proceeds as follows:
> +
> +\begin{enumerate}
> +\item The device updates \field{free_page_report_cmd_id}: if it has
> + reached the maximum value, 0xffffffff, resets it to
> + \field{VIRTIO_BALLOON_CMD_ID_MIN}. Otherwise, increments it by 1.
> +\item The device sends a configuration notification to the driver.
that's configuration change notification
> +\item The driver reads \field{free_page_report_cmd_id}: if it is not
> + \field{VIRTIO_BALLOON_CMD_ID_STOP},
> + \field{VIRTIO_BALLOON_CMD_ID_DONE}, and different from the value it
> + read last time, start free page reporting as follows:
> + \begin{enumerate}
> + \item Write the received \field{free_page_report_cmd_id} to a
> + buffer and add it to the free page virtqueue to indicate the start
> + of the reporting.
> + \item Collect free pages and write each page address into an entry
> + of the virtqueue.
Spec does not define terms such as entry for virtqueue or writing
into virtqueue.
What you mean is add each page as an input buffer to the
virtqueue I think?
Also are there any alignment assumptions? What does "page" mean here
generally? A size aligned physically contigious region of free memory?
> + \item Write \field{VIRTIO_BALLOON_CMD_ID_STOP} into a buffer and add
> + it to the free page virtqueue, which signifies that all the free
> + pages are reported.
better "that free page reporting is complete"?
> + \end{enumerate}
> + A driver to device notification is sent in the above steps after
> + adding a buffer or address to the virtqueue if the notification
> + is not suppressed.
> +\item The device pops entries from the virtqueue:
We don't define "pops" for devices either.
The terminology we use is something like
"processes availble buffers".
Please look at other examples.
> it starts to consume
> + the addresses
Need to define what does "consume" mean here.
> after it receives the buffer that stores a value which
> + equals to \field{free_page_report_cmd_id}, and stops the address
> + consumption after receiving a buffer which stores
> + \field{VIRTIO_BALLOON_CMD_ID_STOP}.
> +\end{enumerate}
> +
> +A request to actively stop the free page reporting proceeds as
> +follows:
> +
> +\begin{enumerate}
> +\item The device sets \field{free_page_report_cmd_id} to
> + \field{VIRTIO_BALLOON_CMD_ID_STOP}, followed by a configuration
> + notification to the driver if not suppressed.
> +\item The driver reads \field{free_page_report_cmd_id} and identifies
> + that it is \field{VIRTIO_BALLOON_CMD_ID_STOP}, then it stops
> + collecting free pages.
> +\item The driver writes \field{VIRTIO_BALLOON_CMD_ID_STOP} into a
> + buffer and adds it to the virtqueue.
> +\item The device stops poping entires from the virtqueue
> + after receiving the buffer that stores
> + \field{VIRTIO_BALLOON_CMD_ID_STOP}.
> +\end{enumerate}
> +
> +
> +The device can end the free page reporting by setting
> +\field{free_page_report_cmd_id} to \field{VIRTIO_BALLOON_CMD_ID_DONE},
> +followed by a configuration notification to the driver.
> +
> +\drivernormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting}
> +Normative statements in this section apply if and only if the
> +VIRTIO_BALLOON_F_FREE_PAGE_HINT feature has been negotiated.
So just say
if VIRTIO_BALLOON_F_FREE_PAGE_HINT has been negotiated:
and then proceed.
> +
> +The driver SHOULD add the addresses of free pages as input buffers
> +to the virtqueue.
> +
> +The driver SHOULD add the buffer that stores the value of
> +\field{free_page_report_cmd_id} or \field{VIRTIO_BALLOON_CMD_ID_STOP}
> +as an output buffer to the virtqueue.
> +
> +The driver SHOULD use different buffers to send the the value of
> +\field{free_page_report_cmd_id} and
> +\field{VIRTIO_BALLOON_CMD_ID_STOP} to avoid one being overwritten by
> +another when the device has a delay in reading the command.
> +
> +The driver SHOULD release all the collected free pages after receiving
> +\field{VIRTIO_BALLOON_CMD_ID_DONE}.
> +
> \section{SCSI Host Device}\label{sec:Device Types / SCSI Host Device}
>
> The virtio SCSI host device groups together one or more virtual
> --
> 2.7.4
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]