Balloon header includes:
- feature bit for Working Set Reporting
- number of Working Set bins member in balloon config
- types for communicating Working Set information
Signed-off-by: T.J. Alumbaugh <talumbau@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 */
/* 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;
+ uint8_t reserved[4];
+ __virtio64 idle_age_ms;
+ /* A bin each for anonymous and file-backed memory. */
+ __virtio64 memory_size_bytes[2];
+} QEMU_PACKED;
+
#endif /* _LINUX_VIRTIO_BALLOON_H */
--
2.41.0.rc0.172.g3f132b7071-goog
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 <talumbau@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
On Wed, May 31, 2023 at 10:12:26AM +0200, David Hildenbrand wrote: > 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). this belongs in driver description, indeed. > -> How does a request look like? > -> How does a response look like? > -> Error cases? These last 3 are best in the spec, not in driver. > (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. same. best in spec. > > Signed-off-by: T.J. Alumbaugh <talumbau@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
© 2016 - 2024 Red Hat, Inc.