On 26.05.23 00:20, T.J. Alumbaugh wrote:
Hi,
please try writing a comprehensive patch description: the goal should be
that one can understand what's happening in the single patch without all
of the following patches at hand. [ that's how I am reading them, and
ahve to ask many stupid questions :P ]
Balloon header includes:
- feature bit for Working Set Reporting
- number of Working Set bins member in balloon config
- types for communicating Working Set information
Can you briefly summarize how all the bits here interact?
I assume, once VIRTIO_BALLOON_F_WS_REPORTING has been negotiated
(1) There is a new virtqueue for sending WS-related requests from the
device (host) to the driver (guest).
-> How does a request look like?
-> How does a response look like?
-> Error cases?
(2) There is a new config space option.
-> Who's supposed to read this, who's supposed to write it?
-> Can it be changed dynamically?
-> What's the meaning / implication of that value.
Signed-off-by: T.J. Alumbaugh <talum...@google.com>
---
.../standard-headers/linux/virtio_balloon.h | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/include/standard-headers/linux/virtio_balloon.h
b/include/standard-headers/linux/virtio_balloon.h
index f343bfefd8..df61eaceee 100644
--- a/include/standard-headers/linux/virtio_balloon.h
+++ b/include/standard-headers/linux/virtio_balloon.h
@@ -37,6 +37,7 @@
#define VIRTIO_BALLOON_F_FREE_PAGE_HINT 3 /* VQ to report free pages */
#define VIRTIO_BALLOON_F_PAGE_POISON 4 /* Guest is using page poisoning */
#define VIRTIO_BALLOON_F_REPORTING 5 /* Page reporting virtqueue */
+#define VIRTIO_BALLOON_F_WS_REPORTING 6 /* Working set report virtqueues */
... are there multiple virtqueues? How many?
/* Size of a PFN in the balloon interface. */
#define VIRTIO_BALLOON_PFN_SHIFT 12
@@ -59,6 +60,9 @@ struct virtio_balloon_config {
};
/* Stores PAGE_POISON if page poisoning is in use */
uint32_t poison_val;
+ /* Stores the number of histogram bins if WS reporting in use */
+ uint8_t working_set_num_bins;
+ uint8_t padding[3];
};
#define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */
@@ -116,4 +120,20 @@ struct virtio_balloon_stat {
__virtio64 val;
} QEMU_PACKED;
+enum virtio_balloon_working_set_op {
+ VIRTIO_BALLOON_WS_REQUEST = 1, /* a Working Set request from the host */
+ VIRTIO_BALLOON_WS_CONFIG = 2, /* a Working Set config from the host */
+};
+
+struct virtio_balloon_working_set {
+ /* A tag for additional metadata */
+ __virtio16 tag;
+ /* The NUMA node for this report. */
+ __virtio16 node_id;
How will we handle the case when the guest decides to use a different
NUMA layout (e.g., numa disabled, fake numa, ...).
Is the guest supposed to detect that and *not* indicate a NUMA ID then?
Also, I wonder
+ uint8_t reserved[4];
+ __virtio64 idle_age_ms;
+ /* A bin each for anonymous and file-backed memory. */
Why not have them separately, and properly named?
I'm not sure if it's a good idea to distinguish them based on anon vs.
file-backed.
What would you do with shmem? It can be swapped like anon memory, ... if
swap is enabled.
What's the main motivation for splitting this up? Is the "file-backed"
part supposed to give some idea about the pagecache size? But what about
mlock or page pinning?
Now I should take a step back and read the cover letter :)
--
Thanks,
David / dhildenb
