[Devel] [PATCH vz7] Create debugfs file with virtio balloon usage information

[Konstantin Khorenko]

Subject: [PATCH vz7] virtio_balloon: Create debugfs file with the usage information

On 02.09.2022 13:19, Alexander Atanasov wrote:
> Allow the guest to know how much it is ballooned by the host.
> Depending on options the ballooned memory is accounted in two ways.
> If deflate on oom is enabled - ballooned memory is accounted as used.
> If deflate on oom is not enabled - ballooned memory is subtracted
> from total ram.

Allow the guest to know how much memory is ballooned by the Host.

Depending on options the ballooned memory is accounted in two ways:
  1. If deflate on OOM is enabled - ballooned memory is accounted as used.
  2. If deflate on OOM is not enabled - ballooned memory is subtracted
     from total RAM.

> https://jira.sw.ru/browse/PSBM-140407
> Signed-off-by: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
> ---
>   drivers/virtio/virtio_balloon.c | 52 +++++++++++++++++++++++++++++++++
>   1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 142719d05dc5..e8cefeea4f22 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -23,6 +23,7 @@
>   #include <linux/virtio_balloon.h>
>   #include <linux/swap.h>
>   #include <linux/workqueue.h>
> +#include <linux/debugfs.h>
>   #include <linux/delay.h>
>   #include <linux/slab.h>
>   #include <linux/module.h>
> @@ -447,6 +448,54 @@ static int init_vqs(struct virtio_balloon *vb)
>   	return 0;
>   }
>   
> +/*
> + * DEBUGFS Interface
> + */
> +#ifdef CONFIG_DEBUG_FS
> +
> +static int virtio_balloon_debug_show(struct seq_file *f, void *offset)
> +{
> +	struct virtio_balloon *vb = f->private;
> +	u64 inflated_kb = vb->num_pages << (VIRTIO_BALLOON_PFN_SHIFT - 10);

/*
  * Balloon device works in 4K page units.  So each page is pointed to by
  * multiple balloon pages.  All memory counters in this driver are in balloon
  * page units.
  */
#define VIRTIO_BALLOON_PAGES_PER_PAGE (unsigned)(PAGE_SIZE >> VIRTIO_BALLOON_PFN_SHIFT)

/* Size of a PFN in the balloon interface. */
#define VIRTIO_BALLOON_PFN_SHIFT 12

Taking into account defines above, i think the proper code should be:
   /* "+2" here because we want the value in kilobytes */
   u64 inflated_kb = vb->num_pages << (PAGE_SHIFT - VIRTIO_BALLOON_PFN_SHIFT + 2);

As it's said VIRTIO_BALLOON_PAGE cannot be larger than PAGE_SIZE,
so if we imagine VIRTIO_BALLOON_PFN_SHIFT is defined to, say, 10, the original code
"<< (VIRTIO_BALLOON_PFN_SHIFT - 10)" will definitely work wrong.

===========================

Another question: why we report "num_pages", but not "actual"?

struct virtio_balloon_config {
         /* Number of pages host wants Guest to give up. */
         __u32 num_pages;
         /* Number of pages we've actually got in balloon. */
         __u32 actual;
};

If we have a VM with, say, 4GB RAM and balloon is configured to grab, say, 4GB, we need to know the 
memory _actually_ grabbed by balloon i think.

Or may be to report both "num_pages" and "actual"?


> +	u64 inflated_total = 0;
> +	u64 inflated_free = 0;
> +
> +	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> +		inflated_free = inflated_kb;
> +	else
> +		inflated_total = inflated_kb;
> +
> +	seq_printf(f, "InflatedTotal: %lld kB\n", inflated_total);
> +	seq_printf(f, "InflatedFree: %lld kB\n", inflated_free);

Are those namings are stoned already?
May be to rename them to "DeflatedTotal" and "InflatedUsed" ?

s/kB/KB/

> +
> +	return 0;
> +}
> +
> +DEFINE_SHOW_ATTRIBUTE(virtio_balloon_debug);
> +
> +static void  virtio_balloon_debugfs_init(struct virtio_balloon *b)
> +{
> +	debugfs_create_file("virtio-balloon", 0444, NULL, b,
> +				&virtio_balloon_debug_fops);
> +}
> +
> +static void  virtio_balloon_debugfs_exit(struct virtio_balloon *b)
> +{
> +	debugfs_remove(debugfs_lookup("virtio-balloon", NULL));
> +}
> +
> +#else
> +
> +static inline void virtio_balloon_debugfs_init(struct virtio_balloon *b)
> +{
> +}
> +
> +static inline void virtio_balloon_debugfs_exit(struct virtio_balloon *b)
> +{
> +}
> +
> +#endif /* CONFIG_DEBUG_FS */
> +
>   #ifdef CONFIG_BALLOON_COMPACTION
>   /*
>    * virtballoon_migratepage - perform the balloon page migration on behalf of
> @@ -568,6 +617,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
>   
>   	if (towards_target(vb))
>   		virtballoon_changed(vdev);
> +	virtio_balloon_debugfs_init(vb);
> +
>   	return 0;
>   
>   out_oom_notify:
> @@ -595,6 +646,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
>   {
>   	struct virtio_balloon *vb = vdev->priv;
>   
> +	virtio_balloon_debugfs_exit(vb);
>   	unregister_oom_notifier(&vb->nb);
>   
>   	spin_lock_irq(&vb->stop_update_lock);