[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] [PATCH v2 3/3] content: Document balloon feature free page reporting
On 15.05.20 19:33, Alexander Duyck wrote: > From: Alexander Duyck <alexander.h.duyck@linux.intel.com> > > Free page reporting is a feature that allows the guest to proactively > report unused pages to the host. By making use of this feature is is > possible to reduce the overall memory footprint of the guest in cases where > some significant portion of the memory is idle. Add documentation for the > free page reporting feature describing the functionality and requirements. > > Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com> > --- > content.tex | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 83 insertions(+), 1 deletion(-) > > diff --git a/content.tex b/content.tex > index 3d30fd5bb6fa..3cb38105f794 100644 > --- a/content.tex > +++ b/content.tex > @@ -5007,12 +5007,15 @@ \subsection{Virtqueues}\label{sec:Device Types / Memory Balloon Device / Virtque > \item[1] deflateq > \item[2] statsq > \item[3] free_page_vq > +\item[4] reporting_vq > \end{description} > > statsq only exists if VIRTIO_BALLOON_F_STATS_VQ is set. > > free_page_vq only exists if VIRTIO_BALLOON_F_FREE_PAGE_HINT is set. > > + reporting_vq only exists if VIRTIO_BALLOON_F_PAGE_REPORTING is 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 > @@ -5029,6 +5032,8 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu > \item[ VIRTIO_BALLOON_F_PAGE_POISON(4) ] The device has to be notified if > the driver is expecting balloon pages to contain a certain value when > returned. Configuration field poison_val is valid. > +\item[ VIRTIO_BALLOON_F_PAGE_REPORTING(5) ] The device has support for free > + page reporting. A virtqueue for reporting free guest memory is present. > > \end{description} > > @@ -5039,6 +5044,10 @@ \subsection{Feature bits}\label{sec:Device Types / Memory Balloon Device / Featu > The driver SHOULD clear the VIRTIO_BALLOON_F_PAGE_POISON flag if it is not > expecting any specific value to be stored in the page. > > +If the driver is expecting the pages to retain some initialized value, > +it MUST NOT accept VIRTIO_BALLOON_F_PAGE_REPORTING unless it also > +negotiates VIRTIO_BALLOON_F_PAGE_POISON. > + Is "accept" really the right word here? Below you use "negotiate", which makes more sense. > \devicenormative{\subsubsection}{Feature bits}{Device Types / Memory Balloon Device / Feature bits} > If the device offers the VIRTIO_BALLOON_F_MUST_TELL_HOST feature > bit, and if the driver did not accept this feature bit, the > @@ -5101,10 +5110,16 @@ \subsection{Device Initialization}\label{sec:Device Types / Memory Balloon Devic > \item If the VIRTIO_BALLOON_F_PAGE_POISON feature bit is negotiated, the > driver updates the \field{poison_val} configuration field. > > +\item If the VIRTIO_BALLOON_F_PAGE_REPORTING feature bit is negotiated the > + reporting_vq is identified. > + > \item DRIVER_OK is set: device operation begins. > > \item If the VIRTIO_BALLOON_F_STATS_VQ feature bit is negotiated, then > notify the device about the stats virtqueue buffer. > + > +\item If the VIRTIO_BALLOON_F_PAGE_REPORTING feature bit is negotiated then > + begin reporting free pages to device. > \end{enumerate} > > \subsection{Device Operation}\label{sec:Device Types / Memory Balloon Device / Device Operation} > @@ -5478,7 +5493,9 @@ \subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Dev > > Page Poison provides a way to notify the host of the contents that are > currently in the balloon pages, and those that are expected to be in the > -pages when they are pulled from the balloon. > +pages when they are pulled from the balloon. It is used for in-place > +reporting of pages without needing to pull them from the memory allocator > +of the guest. Let's see how that looks like after you modify patch #2. > > If VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the guest > will place the expected poison value into the \field{poison_val} > @@ -5504,6 +5521,71 @@ \subsubsection{Page Poison}\label{sec:Device Types / Memory Balloon Device / Dev > page hinting, the device MAY ignore the content of \field{poison_val} > for those operations. > > +\subsubsection{Free Page Reporting}\label{sec:Device Types / Memory Balloon Device / Device Operation / Free Page Reporting} > + > +Free Page Reporting provides a mechanism similar to balloon inflation, > +however it does not provide a deflation queue. The expectation is that the > +device will have a means by which it can detect the guest page access and > +fault in such pages with some initial value, likely a zero page. > + > +The driver will respond to to memory conditions and begin reporting free "to to memory conditions" I don't understand what you are trying to say. The driver will simply report some free pages (e.g., of a guest-specific minimum size) when it feels like the right time has come. This (and below) is too implementation specific. You could just implement a driver that hints a single page every time it is getting freed. Nothing wrong about that. There is just the option do to a bulk report whenever the driver feels like doing it. > +pages when some number of pages are available. > + > +\begin{enumerate} > + > +\item The driver determines it has enough pages available to begin > + reporting pages. > + > +\item The driver gathers pages into a scatter-gather list and adds them to > + the reporting_vq. > + > +\item The device acknowledges the reporting request by using the > + reporting_vq descriptor. > + > +\item Once the device has acknowledged the report, the pages can be > + returned to the location from which they were pulled. > + > +\item The driver can then continue to gather and report pages until it > + has determined it has reported a sufficient quantity of pages. > + > +\end{enumerate} > + > +\drivernormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting} > + > +Normative statements in this section apply if the > +VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated. > + > +If the driver is expecting the free page to contain some initial value it > +MUST NOT negotiate this feature without negotiating the > +VIRTIO_BALLOON_F_PAGE_POISON feature as well and supply this value via > +\field{poison_val}. > + > +If the driver is expecting the pages to retain some initialized value, > +it MUST NOT accept VIRTIO_BALLOON_F_PAGE_REPORTING unless it also > +negotiates the VIRTIO_BALLOON_F_PAGE_POISON feature and supplies this > +value via \field{poison_val}. I'd probably do something like The driver MUST NOT negotiate VIRTIO_BALLOON_F_PAGE_REPORTING without VIRTIO_BALLOON_F_PAGE_POISON in case it relies on reported pages to not change their content. The driver MUST negotiate VIRTIO_BALLOON_F_PAGE_REPORTING and VIRTIO_BALLOON_F_PAGE_POISON in case it relies on reported pages to be filled with \field{poison_val} when reusing them. But I'll have to double check your QEMU patches :) > + > +The driver MUST NOT use the reported pages until the device has > +acknowledged the reporting request. > + > +The driver MAY report free pages any time after DRIVER_OK is set. Now, this is the right thing to document. Maybe also something like "The driver MAY decide to not report some free pages, especially of smaller granularity." > + > +It is RECOMMENDED that the driver avoid unnecessary reads or writes to the s/avoid/avoids/ > +page contents as this could reduce the performance for free page reporting. > + You're the second user of RECOMMENDED. I'd do: "The driver SHOULD avoid unnecessary reads ..." > +\devicenormative{\paragraph}{Free Page Reporting}{Device Types / Memory Balloon Device / Device Operation / Free Page Reporting} > + > +Normative statements in this section apply if the > +VIRTIO_BALLOON_F_PAGE_REPORTING feature has been negotiated. > + > +The device MAY modify the contents of any page supplied in a report > +request even before acknowledging that request by using the > +reporting_vq descriptor. > + > +If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the device > +SHALL NOT modify the the page if this will result in the page containing a > +value other than \field{poison_val}. You're the first user of "SHALL NOT" -> "MUST NOT" ? s/the the page/the content of a reported page/ "If the VIRTIO_BALLOON_F_PAGE_POISON feature has been negotiated, the device MUST NOT modify the content of a reported page to something other than than \field{poison_val}." ? > + > \section{SCSI Host Device}\label{sec:Device Types / SCSI Host Device} > -- Thanks, David / dhildenb
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]