Re: [PATCH v1 1/1] Create debugfs file with hyper-v balloon usage information
From: Alexander Atanasov
Date: Fri Jul 08 2022 - 12:25:44 EST
Hi,
On 05/07/2022 20:12, Michael Kelley (LINUX) wrote:
From: Alexander Atanasov <alexander.atanasov@xxxxxxxxxxxxx> Sent: Tuesday, July 5, 2022 2:44 AM
Allow the guest to know how much it is ballooned by the host.
It is useful when debugging out of memory conditions.
When host gets back memory from the guest it is accounted
as used memory in the guest but the guest have no way to know
how much it is actually ballooned.
Expose current state, flags and max possible memory to the guest.
Thanks for the contribution! I can see it being useful. But I'd note
that the existing code has a trace function that outputs pretty much all
the same information when it is reported to the Hyper-V host in
post_status() every 1 second. However, the debugfs interface might be
a bit easier to use for ongoing sampling. Related, I see that the VMware
balloon driver use the debugfs interface, but no tracing. The virtio
balloon driver has neither. I'm not against adding this debugfs interface,
but I wanted to make sure there's real value over the existing tracing.
Yes it is reported to the host but this is for the guest. The value is
that the user space can track more accurate the memory pressure.
For example userspace cache size calculation based on total and used
ram can came out very wrong without balloon into account. If you have a
nested virtualization/containers things can get confusing too.
VMWare have it already. Virtio patches are waiting for response.
See also some minor comments below.
Sorry, i missed the email.
Michael
While at it - fix a 10+ years old typo.
Signed-off-by: Alexander Atanasov <alexander.atanasov@xxxxxxxxxxxxx>
---
drivers/hv/hv_balloon.c | 127 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 126 insertions(+), 1 deletion(-)
Note - no attempt to handle guest vs host page size difference
is made - see ballooning_enabled.
Basicly if balloon page size != guest page size balloon is off.
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 91e8a72eee14..b7b87d168d46 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -11,6 +11,7 @@
#include <linux/kernel.h>
#include <linux/jiffies.h>
#include <linux/mman.h>
+#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/init.h>
#include <linux/module.h>
@@ -248,7 +249,7 @@ struct dm_capabilities_resp_msg {
* num_committed: Committed memory in pages.
* page_file_size: The accumulated size of all page files
* in the system in pages.
- * zero_free: The nunber of zero and free pages.
+ * zero_free: The number of zero and free pages.
* page_file_writes: The writes to the page file in pages.
* io_diff: An indicator of file cache efficiency or page file activity,
* calculated as File Cache Page Fault Count - Page Read Count.
@@ -567,6 +568,14 @@ struct hv_dynmem_device {
__u32 version;
struct page_reporting_dev_info pr_dev_info;
+
+#ifdef CONFIG_DEBUG_FS
+ /*
+ * Maximum number of pages that can be hot_add-ed
+ */
+ __u64 max_dynamic_page_count;
+#endif
+
};
static struct hv_dynmem_device dm_device;
@@ -1078,6 +1087,9 @@ static void process_info(struct hv_dynmem_device *dm,
struct dm_info_msg *msg)
pr_info("Max. dynamic memory size: %llu MB\n",
(*max_page_count) >> (20 - HV_HYP_PAGE_SHIFT));
+#ifdef CONFIG_DEBUG_FS
+ dm->max_dynamic_page_count = *max_page_count;
+#endif
Arguably, you could drop the #ifdef's in the above two places, to reduce the code
clutter. The extra memory and CPU overhead of always saving max_dynamic_page_count
is negligible. What I don't know for sure is if the compiler or other static checking
tools will complain about a field being set but not used.
Ok, i will drop them. If the tools complain i will have to fix it.
There is also a min dynamic memory size in the Hyper-V setup , which is
interesting but the driver doesn't know about it.
}
break;
@@ -1807,6 +1819,115 @@ static int balloon_connect_vsp(struct hv_device *dev)
return ret;
}
+/*
+ * DEBUGFS Interface
+ */
+#ifdef CONFIG_DEBUG_FS
+
+/**
+ * virtio_balloon_debug_show - shows statistics of balloon operations.
+ * @f: pointer to the &struct seq_file.
+ * @offset: ignored.
+ *
+ * Provides the statistics that can be accessed in virtio-balloon in the debugfs.
+ *
+ * Return: zero on success or an error code.
+ */
+static int hv_balloon_debug_show(struct seq_file *f, void *offset)
+{
+ struct hv_dynmem_device *dm = f->private;
+ unsigned long num_pages_committed;
+ char *sname;
+
+ seq_printf(f, "%-22s: %u.%u\n", "host_version",
+ DYNMEM_MAJOR_VERSION(dm->version),
+ DYNMEM_MINOR_VERSION(dm->version));
+
+ seq_printf(f, "%-22s:", "capabilities");
+ if (ballooning_enabled())
+ seq_puts(f, " enabled");
+
+ if (hot_add_enabled())
+ seq_puts(f, " hot_add");
+
+ seq_puts(f, "\n");
+
+ seq_printf(f, "%-22s: %u", "state", dm->state);
+ switch (dm->state) {
+ case DM_INITIALIZING:
+ sname = "Initializing";
+ break;
+ case DM_INITIALIZED:
+ sname = "Initialized";
+ break;
+ case DM_BALLOON_UP:
+ sname = "Balloon Up";
+ break;
+ case DM_BALLOON_DOWN:
+ sname = "Balloon Down";
+ break;
+ case DM_HOT_ADD:
+ sname = "Hot Add";
+ break;
+ case DM_INIT_ERROR:
+ sname = "Error";
+ break;
+ default:
+ sname = "Unknown";
+ }
+ seq_printf(f, " (%s)\n", sname);
+
+ /* HV Page Size */
+ seq_printf(f, "%-22s: %ld\n", "page_size", HV_HYP_PAGE_SIZE);
+
+ /* Pages added with hot_add */
+ seq_printf(f, "%-22s: %u\n", "pages_added", dm->num_pages_added);
+
+ /* pages that are "onlined"/used from pages_added */
+ seq_printf(f, "%-22s: %u\n", "pages_onlined", dm->num_pages_onlined);
+
+ /* pages we have given back to host */
+ seq_printf(f, "%-22s: %u\n", "pages_ballooned", dm->num_pages_ballooned);
+
+ num_pages_committed = vm_memory_committed();
+ num_pages_committed += dm->num_pages_ballooned +
+ (dm->num_pages_added > dm->num_pages_onlined ?
+ dm->num_pages_added - dm->num_pages_onlined : 0) +
+ compute_balloon_floor();
Duplicating this calculation that also appears in post_status() is not ideal. Maybe
post_status() should store the value in a field in the struct hv_dynmem_device, and
this function would just output the field. Again, there's the potential for a code
checker to complain about a field being written but not read. Alternatively,
the calculation could go into a helper function that is called here and in
post_status(). I'm not sure if it is more useful to report the last value that
was reported by the Hyper-V host, or the currently calculated value. There is a
trace point that records the values reported to the host, so maybe the latter
is more useful here.
I will extract it in a function. The calculation is cheap and
considering that the debugfs file will be rarely read it is better with
function.
It came out this way because i initially tried to add calculations of
page sizes in the case where balloon and host have different page sizes.
But later on i figured out that in that case the balloon is not working.
This can of course be improved.
--
Regards,
Alexander Atanasov