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

[Konstantin Khorenko]

On 05.09.2022 16:01, Konstantin Khorenko wrote:
> 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.

And please, address this comment as well.


> ===========================
> 
> 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);
> _______________________________________________
> Devel mailing list
> Devel at openvz.org
> https://lists.openvz.org/mailman/listinfo/devel
> .