Re: [PATCH 1/1] Create debugfs file with virtio balloon usage information

[Alexander Atanasov]

Hello,

On 27/06/2022 23:42, Michael S. Tsirkin wrote:
> On Mon, Jun 27, 2022 at 07:19:09PM +0000, Alexander Atanasov wrote:
> > 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.
> >
> > No pretty printing and fixed as per coding style.
> > ....
> > +static int virtio_balloon_debug_show(struct seq_file *f, void *offset)
> > +{
> > +	struct virtio_balloon *b = f->private;
> > +	u32 num_pages;
> > +	struct sysinfo i;
> > +
> > +	si_meminfo(&i);
> > +
> > +	seq_printf(f, "%-22s: %llx\n", "capabilities", b->vdev->features);
> why do we need this in debugfs? Isn't this available in sysfs already?

Yes, it doesn't make sense to have it without pretty printing. I will 
remove it.

> > +	seq_printf(f, "%-22s: %d\n", "page_size", 4096);
> I suspect this function doesn't work properly when page size is not 4K.

It is the page size used by the balloon and it is always 4K and not the 
page size used by the guest which can be different.

/*
 * 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.
 */

And the code agrees with the comment. To be consistent the file must use 
the same units.


> > +
> > +	virtio_cread_le(b->vdev, struct virtio_balloon_config, actual,
> > +			&num_pages);
> > +	/* Memory returned to host or amount we can inflate if possible */
> > +	seq_printf(f, "%-22s: %u\n", "ballooned_pages", num_pages);
> I don't really get the comment here.

I will try to reword it to be more clear .

    /*
     * Pages allocated by host from the guest memory.
     * Host inflates the balloon to get more memory.
     * Guest needs to deflate the balloon to get more memory.
     */

> > +	/* Total Memory for the guest from host */
> > +	seq_printf(f, "%-22s: %lu\n", "total_pages", i.totalram);
> > +
> > +	/* Current memory for the guest */
> > +	seq_printf(f, "%-22s: %lu\n", "current_pages", i.totalram - num_pages);
> Are you sure these are in units of 4Kbyte pages?

The guest can have a different page size, so a conversion to balloon 
page size is required - fix in the following patch .

--
Regards,
Alexander Atanasov