2022-07-05 10:21:09

by Alexander Atanasov

[permalink] [raw]
Subject: [PATCH v1 1/1] Create debugfs file with hyper-v balloon usage information

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.
While at it - fix a 10+ years old typo.

Signed-off-by: Alexander Atanasov <[email protected]>
---
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
}

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();
+ seq_printf(f, "%-22s: %lu\n", "total_pages_commited",
+ num_pages_committed);
+
+ seq_printf(f, "%-22s: %llu\n", "max_dynamic_page_count",
+ dm->max_dynamic_page_count);
+
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(hv_balloon_debug);
+
+static void hv_balloon_debugfs_init(struct hv_dynmem_device *b)
+{
+ debugfs_create_file("hv-balloon", 0444, NULL, b,
+ &hv_balloon_debug_fops);
+}
+
+static void hv_balloon_debugfs_exit(struct hv_dynmem_device *b)
+{
+ debugfs_remove(debugfs_lookup("hv-balloon", NULL));
+}
+
+#else
+
+static inline void hv_balloon_debugfs_init(struct hv_dynmem_device *b)
+{
+}
+
+static inline void hv_balloon_debugfs_exit(struct hv_dynmem_device *b)
+{
+}
+
+#endif /* CONFIG_DEBUG_FS */
+
static int balloon_probe(struct hv_device *dev,
const struct hv_vmbus_device_id *dev_id)
{
@@ -1854,6 +1975,8 @@ static int balloon_probe(struct hv_device *dev,
goto probe_error;
}

+ hv_balloon_debugfs_init(&dm_device);
+
return 0;

probe_error:
@@ -1879,6 +2002,8 @@ static int balloon_remove(struct hv_device *dev)
if (dm->num_pages_ballooned != 0)
pr_warn("Ballooned pages: %d\n", dm->num_pages_ballooned);

+ hv_balloon_debugfs_exit(dm);
+
cancel_work_sync(&dm->balloon_wrk.wrk);
cancel_work_sync(&dm->ha_wrk.wrk);

--
2.25.1


2022-07-05 17:28:37

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH v1 1/1] Create debugfs file with hyper-v balloon usage information

From: Alexander Atanasov <[email protected]> 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.

See also some minor comments below.

Michael

> While at it - fix a 10+ years old typo.
>
> Signed-off-by: Alexander Atanasov <[email protected]>
> ---
> 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.

> }
>
> 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.

> + seq_printf(f, "%-22s: %lu\n", "total_pages_commited",
> + num_pages_committed);
> +
> + seq_printf(f, "%-22s: %llu\n", "max_dynamic_page_count",
> + dm->max_dynamic_page_count);
> +
> + return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(hv_balloon_debug);
> +
> +static void hv_balloon_debugfs_init(struct hv_dynmem_device *b)
> +{
> + debugfs_create_file("hv-balloon", 0444, NULL, b,
> + &hv_balloon_debug_fops);
> +}
> +
> +static void hv_balloon_debugfs_exit(struct hv_dynmem_device *b)
> +{
> + debugfs_remove(debugfs_lookup("hv-balloon", NULL));
> +}
> +
> +#else
> +
> +static inline void hv_balloon_debugfs_init(struct hv_dynmem_device *b)
> +{
> +}
> +
> +static inline void hv_balloon_debugfs_exit(struct hv_dynmem_device *b)
> +{
> +}
> +
> +#endif /* CONFIG_DEBUG_FS */
> +
> static int balloon_probe(struct hv_device *dev,
> const struct hv_vmbus_device_id *dev_id)
> {
> @@ -1854,6 +1975,8 @@ static int balloon_probe(struct hv_device *dev,
> goto probe_error;
> }
>
> + hv_balloon_debugfs_init(&dm_device);
> +
> return 0;
>
> probe_error:
> @@ -1879,6 +2002,8 @@ static int balloon_remove(struct hv_device *dev)
> if (dm->num_pages_ballooned != 0)
> pr_warn("Ballooned pages: %d\n", dm->num_pages_ballooned);
>
> + hv_balloon_debugfs_exit(dm);
> +
> cancel_work_sync(&dm->balloon_wrk.wrk);
> cancel_work_sync(&dm->ha_wrk.wrk);
>
> --
> 2.25.1

2022-07-05 17:34:32

by Denis V. Lunev

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] Create debugfs file with hyper-v balloon usage information

On 05.07.2022 19:12, Michael Kelley (LINUX) wrote:
> From: Alexander Atanasov <[email protected]> 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.
>
> See also some minor comments below.
virtio efforts:
https://www.spinics.net/lists/kernel/msg4425155.html

In general balloon information is always reported to the host, that is why
balloon is invented. Though the information inside the guest is not
always available and it is hard to track/guess.

We have had a lot of trouble to convince our customers that the
memory was taken by host being inside virtual machine. Thus debugfs
interface is good for such rare checks especially keeping in mind
that this would be the same for most frequently used by us
balloons.

Den

2022-07-06 18:08:49

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] Create debugfs file with hyper-v balloon usage information

On Tue, Jul 05, 2022 at 09:44:09AM +0000, Alexander Atanasov wrote:
[...]
> +/*
> + * DEBUGFS Interface
> + */
> +#ifdef CONFIG_DEBUG_FS
> +
> +/**
> + * virtio_balloon_debug_show - shows statistics of balloon operations.

C&P error here. :-)

> + * @f: pointer to the &struct seq_file.
> + * @offset: ignored.
> + *
> + * Provides the statistics that can be accessed in virtio-balloon in the debugfs.
> + *

Ditto.

Wei.

2022-07-06 18:24:37

by Alexander Atanasov

[permalink] [raw]
Subject: [PATCH v2 1/1] Create debugfs file with hyper-v balloon usage information

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.
While at it - fix a 10+ years old typo.

Signed-off-by: Alexander Atanasov <[email protected]>
---
drivers/hv/hv_balloon.c | 126 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 125 insertions(+), 1 deletion(-)

V1->V2:
- Fix C&P errors - you got me :)

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..91dfde06c6fb 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,13 @@ 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 +1086,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
}

break;
@@ -1807,6 +1818,115 @@ static int balloon_connect_vsp(struct hv_device *dev)
return ret;
}

+/*
+ * DEBUGFS Interface
+ */
+#ifdef CONFIG_DEBUG_FS
+
+/**
+ * hv_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 hv-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();
+ seq_printf(f, "%-22s: %lu\n", "total_pages_commited",
+ num_pages_committed);
+
+ seq_printf(f, "%-22s: %llu\n", "max_dynamic_page_count",
+ dm->max_dynamic_page_count);
+
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(hv_balloon_debug);
+
+static void hv_balloon_debugfs_init(struct hv_dynmem_device *b)
+{
+ debugfs_create_file("hv-balloon", 0444, NULL, b,
+ &hv_balloon_debug_fops);
+}
+
+static void hv_balloon_debugfs_exit(struct hv_dynmem_device *b)
+{
+ debugfs_remove(debugfs_lookup("hv-balloon", NULL));
+}
+
+#else
+
+static inline void hv_balloon_debugfs_init(struct hv_dynmem_device *b)
+{
+}
+
+static inline void hv_balloon_debugfs_exit(struct hv_dynmem_device *b)
+{
+}
+
+#endif /* CONFIG_DEBUG_FS */
+
static int balloon_probe(struct hv_device *dev,
const struct hv_vmbus_device_id *dev_id)
{
@@ -1854,6 +1974,8 @@ static int balloon_probe(struct hv_device *dev,
goto probe_error;
}

+ hv_balloon_debugfs_init(&dm_device);
+
return 0;

probe_error:
@@ -1879,6 +2001,8 @@ static int balloon_remove(struct hv_device *dev)
if (dm->num_pages_ballooned != 0)
pr_warn("Ballooned pages: %d\n", dm->num_pages_ballooned);

+ hv_balloon_debugfs_exit(dm);
+
cancel_work_sync(&dm->balloon_wrk.wrk);
cancel_work_sync(&dm->ha_wrk.wrk);

--
2.25.1

2022-07-08 15:55:07

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH v2 1/1] Create debugfs file with hyper-v balloon usage information

From: Alexander Atanasov <[email protected]> Sent: Wednesday, July 6, 2022 11:15 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.
> While at it - fix a 10+ years old typo.
>
> Signed-off-by: Alexander Atanasov <[email protected]>
> ---
> drivers/hv/hv_balloon.c | 126 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 125 insertions(+), 1 deletion(-)
>
> V1->V2:
> - Fix C&P errors - you got me :)

Did you see my code comments on v1 of your patch (in addition to my
broader comment about debugfs vs. tracing) ? I didn't see a response from
you, and my code suggestions are not incorporated into this v2, so I wondered
if you disagreed with my suggestions, or just missed them.

Michael

>
> 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..91dfde06c6fb 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,13 @@ 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 +1086,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
> }
>
> break;
> @@ -1807,6 +1818,115 @@ static int balloon_connect_vsp(struct hv_device *dev)
> return ret;
> }
>
> +/*
> + * DEBUGFS Interface
> + */
> +#ifdef CONFIG_DEBUG_FS
> +
> +/**
> + * hv_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 hv-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();
> + seq_printf(f, "%-22s: %lu\n", "total_pages_commited",
> + num_pages_committed);
> +
> + seq_printf(f, "%-22s: %llu\n", "max_dynamic_page_count",
> + dm->max_dynamic_page_count);
> +
> + return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(hv_balloon_debug);
> +
> +static void hv_balloon_debugfs_init(struct hv_dynmem_device *b)
> +{
> + debugfs_create_file("hv-balloon", 0444, NULL, b,
> + &hv_balloon_debug_fops);
> +}
> +
> +static void hv_balloon_debugfs_exit(struct hv_dynmem_device *b)
> +{
> + debugfs_remove(debugfs_lookup("hv-balloon", NULL));
> +}
> +
> +#else
> +
> +static inline void hv_balloon_debugfs_init(struct hv_dynmem_device *b)
> +{
> +}
> +
> +static inline void hv_balloon_debugfs_exit(struct hv_dynmem_device *b)
> +{
> +}
> +
> +#endif /* CONFIG_DEBUG_FS */
> +
> static int balloon_probe(struct hv_device *dev,
> const struct hv_vmbus_device_id *dev_id)
> {
> @@ -1854,6 +1974,8 @@ static int balloon_probe(struct hv_device *dev,
> goto probe_error;
> }
>
> + hv_balloon_debugfs_init(&dm_device);
> +
> return 0;
>
> probe_error:
> @@ -1879,6 +2001,8 @@ static int balloon_remove(struct hv_device *dev)
> if (dm->num_pages_ballooned != 0)
> pr_warn("Ballooned pages: %d\n", dm->num_pages_ballooned);
>
> + hv_balloon_debugfs_exit(dm);
> +
> cancel_work_sync(&dm->balloon_wrk.wrk);
> cancel_work_sync(&dm->ha_wrk.wrk);
>
> --
> 2.25.1

2022-07-08 17:09:18

by Alexander Atanasov

[permalink] [raw]
Subject: Re: [PATCH v1 1/1] Create debugfs file with hyper-v balloon usage information

Hi,

On 05/07/2022 20:12, Michael Kelley (LINUX) wrote:
> From: Alexander Atanasov <[email protected]> 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 <[email protected]>
>> ---
>> 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

2022-07-11 18:59:10

by Alexander Atanasov

[permalink] [raw]
Subject: [PATCH v3 1/1] Create debugfs file with hyper-v balloon usage information

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.
While at it - fix a 10+ years old typo.

Signed-off-by: Alexander Atanasov <[email protected]>
---
drivers/hv/hv_balloon.c | 135 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 129 insertions(+), 6 deletions(-)

V1->V2:
- Fix C&P errors
V2->V3:
- Move computation to a separate funcion
- Remove ifdefs to reduce code clutter

This patch addresses your suggestions. Once again, sorry i have missed
your email reply last time.


diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 91e8a72eee14..ba52d3a3e3e3 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,11 @@ struct hv_dynmem_device {
__u32 version;

struct page_reporting_dev_info pr_dev_info;
+
+ /*
+ * Maximum number of pages that can be hot_add-ed
+ */
+ __u64 max_dynamic_page_count;
};

static struct hv_dynmem_device dm_device;
@@ -1078,6 +1084,7 @@ 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));
+ dm->max_dynamic_page_count = *max_page_count;
}

break;
@@ -1116,6 +1123,19 @@ static unsigned long compute_balloon_floor(void)
return min_pages;
}

+/*
+ * Compute total committed memory pages
+ */
+
+static unsigned long get_pages_committed(struct hv_dynmem_device *dm)
+{
+ return vm_memory_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();
+}
+
/*
* Post our status as it relates memory pressure to the
* host. Host expects the guests to post this status
@@ -1157,11 +1177,7 @@ static void post_status(struct hv_dynmem_device *dm)
* asking us to balloon them out.
*/
num_pages_avail = si_mem_available();
- num_pages_committed = vm_memory_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();
+ num_pages_committed = get_pages_committed(dm);

trace_balloon_status(num_pages_avail, num_pages_committed,
vm_memory_committed(), dm->num_pages_ballooned,
@@ -1807,6 +1823,109 @@ static int balloon_connect_vsp(struct hv_device *dev)
return ret;
}

+/*
+ * DEBUGFS Interface
+ */
+#ifdef CONFIG_DEBUG_FS
+
+/**
+ * hv_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 hv-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;
+ 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);
+
+ seq_printf(f, "%-22s: %lu\n", "total_pages_commited",
+ get_pages_committed(dm));
+
+ seq_printf(f, "%-22s: %llu\n", "max_dynamic_page_count",
+ dm->max_dynamic_page_count);
+
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(hv_balloon_debug);
+
+static void hv_balloon_debugfs_init(struct hv_dynmem_device *b)
+{
+ debugfs_create_file("hv-balloon", 0444, NULL, b,
+ &hv_balloon_debug_fops);
+}
+
+static void hv_balloon_debugfs_exit(struct hv_dynmem_device *b)
+{
+ debugfs_remove(debugfs_lookup("hv-balloon", NULL));
+}
+
+#else
+
+static inline void hv_balloon_debugfs_init(struct hv_dynmem_device *b)
+{
+}
+
+static inline void hv_balloon_debugfs_exit(struct hv_dynmem_device *b)
+{
+}
+
+#endif /* CONFIG_DEBUG_FS */
+
static int balloon_probe(struct hv_device *dev,
const struct hv_vmbus_device_id *dev_id)
{
@@ -1854,6 +1973,8 @@ static int balloon_probe(struct hv_device *dev,
goto probe_error;
}

+ hv_balloon_debugfs_init(&dm_device);
+
return 0;

probe_error:
@@ -1879,6 +2000,8 @@ static int balloon_remove(struct hv_device *dev)
if (dm->num_pages_ballooned != 0)
pr_warn("Ballooned pages: %d\n", dm->num_pages_ballooned);

+ hv_balloon_debugfs_exit(dm);
+
cancel_work_sync(&dm->balloon_wrk.wrk);
cancel_work_sync(&dm->ha_wrk.wrk);

--
2.25.1

2022-07-12 17:02:52

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH v3 1/1] Create debugfs file with hyper-v balloon usage information

From: Alexander Atanasov <[email protected]> Sent: Monday, July 11, 2022 11:18 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.
> While at it - fix a 10+ years old typo.
>
> Signed-off-by: Alexander Atanasov <[email protected]>
> ---
> drivers/hv/hv_balloon.c | 135 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 129 insertions(+), 6 deletions(-)
>
> V1->V2:
> - Fix C&P errors
> V2->V3:
> - Move computation to a separate funcion
> - Remove ifdefs to reduce code clutter
>
> This patch addresses your suggestions. Once again, sorry i have missed
> your email reply last time.
>

Reviewed-by: Michael Kelley <[email protected]>

2022-07-13 16:48:01

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] Create debugfs file with hyper-v balloon usage information

On Tue, Jul 12, 2022 at 04:24:12PM +0000, Michael Kelley (LINUX) wrote:
> From: Alexander Atanasov <[email protected]> Sent: Monday, July 11, 2022 11:18 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.
> > While at it - fix a 10+ years old typo.
> >
> > Signed-off-by: Alexander Atanasov <[email protected]>
> > ---
[...]
>
> Reviewed-by: Michael Kelley <[email protected]>

I added "Drivers: hv:" prefix to the subject line and applied it to
hyperv-next. Thanks.

2022-09-03 04:59:48

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH v3 1/1] Create debugfs file with hyper-v balloon usage information

From: Wei Liu <[email protected]> Sent: Wednesday, July 13, 2022 8:19 AM
>
> On Tue, Jul 12, 2022 at 04:24:12PM +0000, Michael Kelley (LINUX) wrote:
> > From: Alexander Atanasov <[email protected]> Sent: Monday, July
> 11, 2022 11:18 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.
> > > While at it - fix a 10+ years old typo.
> > >
> > > Signed-off-by: Alexander Atanasov <[email protected]>
> > > ---
> [...]
> >
> > Reviewed-by: Michael Kelley <[email protected]>
>
> I added "Drivers: hv:" prefix to the subject line and applied it to
> hyperv-next. Thanks.

Alexander -- I finally caught up on the long discussion of balloon
driver reporting that occurred over much of August. I think your
original plan had been for each of the balloon driver to report
useful information in debugfs. But as a result of the discussion,
it looks like virtio-balloon will be putting the information in
/proc/meminfo. If that's the case, it seems like we should
drop these changes to the Hyper-V balloon driver, and have
the Hyper-V balloon driver take the same approach as
virtio-balloon.

These Hyper-V balloon driver changes have already gone
into 6.0-rc1. If we're going to drop them, we should do
the revert before 6.0 is done.

Thoughts?

Michael

2022-09-05 09:23:44

by Denis V. Lunev

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] Create debugfs file with hyper-v balloon usage information

On 03.09.2022 06:37, Michael Kelley (LINUX) wrote:
> From: Wei Liu <[email protected]> Sent: Wednesday, July 13, 2022 8:19 AM
>> On Tue, Jul 12, 2022 at 04:24:12PM +0000, Michael Kelley (LINUX) wrote:
>>> From: Alexander Atanasov <[email protected]> Sent: Monday, July
>> 11, 2022 11:18 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.
>>>> While at it - fix a 10+ years old typo.
>>>>
>>>> Signed-off-by: Alexander Atanasov <[email protected]>
>>>> ---
>> [...]
>>> Reviewed-by: Michael Kelley <[email protected]>
>> I added "Drivers: hv:" prefix to the subject line and applied it to
>> hyperv-next. Thanks.
> Alexander -- I finally caught up on the long discussion of balloon
> driver reporting that occurred over much of August. I think your
> original plan had been for each of the balloon driver to report
> useful information in debugfs. But as a result of the discussion,
> it looks like virtio-balloon will be putting the information in
> /proc/meminfo. If that's the case, it seems like we should
> drop these changes to the Hyper-V balloon driver, and have
> the Hyper-V balloon driver take the same approach as
> virtio-balloon.
>
> These Hyper-V balloon driver changes have already gone
> into 6.0-rc1. If we're going to drop them, we should do
> the revert before 6.0 is done.
>
> Thoughts?
>
> Michael
VMware balloon still has debugfs binging. I'd better keep something outside
of meminfo as here we could keep much more useful information.

Den

2022-09-05 16:31:23

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] Create debugfs file with hyper-v balloon usage information

On Sat, Sep 03, 2022 at 04:37:12AM +0000, Michael Kelley (LINUX) wrote:
> From: Wei Liu <[email protected]> Sent: Wednesday, July 13, 2022 8:19 AM
> >
> > On Tue, Jul 12, 2022 at 04:24:12PM +0000, Michael Kelley (LINUX) wrote:
> > > From: Alexander Atanasov <[email protected]> Sent: Monday, July
> > 11, 2022 11:18 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.
> > > > While at it - fix a 10+ years old typo.
> > > >
> > > > Signed-off-by: Alexander Atanasov <[email protected]>
> > > > ---
> > [...]
> > >
> > > Reviewed-by: Michael Kelley <[email protected]>
> >
> > I added "Drivers: hv:" prefix to the subject line and applied it to
> > hyperv-next. Thanks.
>
> Alexander -- I finally caught up on the long discussion of balloon
> driver reporting that occurred over much of August. I think your
> original plan had been for each of the balloon driver to report
> useful information in debugfs. But as a result of the discussion,
> it looks like virtio-balloon will be putting the information in
> /proc/meminfo. If that's the case, it seems like we should
> drop these changes to the Hyper-V balloon driver, and have
> the Hyper-V balloon driver take the same approach as
> virtio-balloon.

The debugfs interface contains far more information than what can be put
into meminfo.

That being said, I can send a PR to revert the changes if we need time
to wait for the other discussion to come to an conclusion.

>
> These Hyper-V balloon driver changes have already gone
> into 6.0-rc1. If we're going to drop them, we should do
> the revert before 6.0 is done.
>

Agreed.

Michael, let me know what you think.

Thanks,
Wei.

> Thoughts?
>
> Michael

2022-09-07 08:25:10

by Alexander Atanasov

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] Create debugfs file with hyper-v balloon usage information

Hello,

On 3.09.22 7:37, Michael Kelley (LINUX) wrote:
>
> Alexander -- I finally caught up on the long discussion of balloon
> driver reporting that occurred over much of August. I think your
> original plan had been for each of the balloon driver to report
> useful information in debugfs. But as a result of the discussion,
> it looks like virtio-balloon will be putting the information in
> /proc/meminfo. If that's the case, it seems like we should
> drop these changes to the Hyper-V balloon driver, and have
> the Hyper-V balloon driver take the same approach as
> virtio-balloon.
>
> These Hyper-V balloon driver changes have already gone
> into 6.0-rc1. If we're going to drop them, we should do
> the revert before 6.0 is done.
>
> Thoughts?


There is a lot of information that would not go into /proc/meminfo.
vmware have only one file with all, virtio have separate file for
features to expose. HV doesn't have other files so i think file is still
useful. Only the memory in kB will go into the /proc/meminfo -
pages/page sizes would not. But i am not sure how to handle the
situation since i guess it will take time for the MM guys to decide -
there is currently only one Ack on the series.



--
Regards,
Alexander Atanasov