Re: Re: [PATCH v2 1/2] balloon: introduce 6 memory statistics

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jun 05, 2024 at 11:37:39PM -0700, zhenwei pi wrote:
> On 6/6/24 14:09, Michael S. Tsirkin wrote:
> > On Thu, Jun 06, 2024 at 11:35:43AM +0800, zhenwei pi wrote:
> >> Note that virtio balloon statistics are OS independent, the names are
> [SNIP]
> >> \item[VIRTIO_BALLOON_S_HTLB_PGFAIL (9)] The number of failed hugetlb page
> >> allocations in the guest.
> >> +
> >> +\item[VIRTIO_BALLOON_S_OOM_KILL (10)] The count of OOM killer invocations
> >> + increases when the kernel invokes the OOM killer. The OOM killer selects a
> task
> >> + to terminate in order to free up memory.
> >> +
> >> +\item[VIRTIO_BALLOON_S_ALLOC_STALL (11)] The count of stalls on memory
> allocation.
> >> +
> >> +\item[VIRTIO_BALLOON_S_ASYNC_SCAN (12)] The amount of memory scanned
> asynchronously
> >> + by the kernel background task (in bytes).
> >> +
> >> +\item[VIRTIO_BALLOON_S_DIRECT_SCAN (13)] The amount of memory scanned
> directly by
> >> + the running task (in bytes).
> >> +
> >> +\item[VIRTIO_BALLOON_S_ASYNC_RECLAIM (14)] The amount of memory reclaimed
> >> + asynchronously by the kernel background task (in bytes).
> >> +
> >> +\item[VIRTIO_BALLOON_S_DIRECT_RECLAIM (15)] The amount of memory reclaimed
> >> + directly by the running task (in bytes).
> >> \end{description}
> >
> > OOM kill description is unnecessarily verbose, the rest not very clear,
> > and not consistent with existing stats.
> >
> 
> Hi Michael,
> 
> You asked 'What exactly are "invocations"? The number of times a task
> was killed to free memory?' in the previous version, would you please
> give me an example,


Well consider what you write:

	\item[VIRTIO_BALLOON_S_OOM_KILL (10)] The count of OOM killer invocations
	increases when the kernel invokes the OOM killer.

this does not explain anything. You just repeat same thing twice.  I asked
what are invocations, your patch does not answer.  If the answer is "the
number of times a task was killed to free memory" then say so, that
would be an explanation.

> and any example of the rest?


Example of not clear:
 What is "the running task" you refer to? What are "stalls" what is stalled?
 The text seems to be agrammatical, using "the" the first time a concept is
 introduced.

Example of inconsistency:
	For example, "the kernel" is not a thing we ever defined.
	We do say "the guest" in balloon description (it is fundamentally a PV
	device and so special in that way).


But please try to think of a reader coming at this for the first time,
reading the whole chapter, not just your patch
and attempting to understand what each of the things is.
Do not just fix the examples I pointed out.
Getting a tech writer to read your text and give feedback is often a good idea.



> >
> >> \subsubsection{Free Page Hinting}\label{sec:Device Types / Memory Balloon
> Device / Device Operation / Free Page Hinting}
> >> --
> >> 2.43.0
> >>
> >
> 
> --
> zhenwei pi
>