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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 28 Jun 2022 12:07:05 +0300
From:   Alexander Atanasov <alexander.atanasov@...tuozzo.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>
Cc:     David Hildenbrand <david@...hat.com>,
        Jason Wang <jasowang@...hat.com>, kernel@...nvz.org,
        virtualization@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] Create debugfs file with virtio balloon usage
 information

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