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

[Alexander Atanasov <alexander.atanasov@virtuozzo.com>]

Hello,

On 14/07/2022 16:24, David Hildenbrand wrote:
> On 14.07.22 15:20, Alexander Atanasov wrote:
>> Hello,
>>
>> On 14/07/2022 14:35, David Hildenbrand wrote:
>>> On 05.07.22 10:36, 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.
>>>>
>>>> Signed-off-by: Alexander Atanasov <alexander.atanasov@virtuozzo.com>
>>>> ---
>>>>    drivers/virtio/virtio_balloon.c     | 77 +++++++++++++++++++++++++++++
>>>>    include/uapi/linux/virtio_balloon.h |  1 +
>>>>    2 files changed, 78 insertions(+)
>>>>
>>>> V2:
>>>>    - fixed coding style
>>>>    - removed pretty print
>>>> V3:
>>>>    - removed dublicate of features
>>>>    - comment about balooned_pages more clear
>>>>    - convert host pages to balloon pages
>>>> V4:
>>>>    - added a define for BALLOON_PAGE_SIZE to make things clear
>>>>
>>>>
>>>>
>>>> +	seq_printf(f, "%-22s: %u\n", "ballooned_pages", num_pages);
>>>> +
>>>> +	/* Total Memory for the guest from host */
>>>> +	seq_printf(f, "%-22s: %lu\n", "total_pages",
>>>> +			guest_to_balloon_pages(i.totalram));
>>> totalram is calculated from totalram_pages().
>>>
>>> When we inflate/deflate, we adjust totalram as well via
>>> adjust_managed_page_count().
>> That is true only when not using DEFLATE_ON_OOM.
>>
>> Otherwise inflated memory is accounted as used and total ram stays the same.
>>> Consequently, this doesn't calculate what you actually want?
>>> Total memory would be totalram+inflated, current would be totalram.
>> My calculations are correct for the case deflate_on_oom is enabled.
>>
> Which is the corner cases. You'd have to special case on DEFLATE_ON_OOM
> availability.

Besides checking the features?

>
>>> But, TBH, only export num_pages. User space can just lookup the other
>>> information (totalram) via /proc/meminfo.
>> I have missed that the memory accounting is made differently depending
>> on a flag.
>>
>> Since the calculations are different i'd prefer to have the values
>> calculate and printed there.
> What about an indication instead, whether or not inflated pages are
> accounted into total or not? That would be slightly cleaner IMHO.

If you care if they are included or not you can check the features and
see if the flag is enabled or not. Going vice versa is not so easy to calculate
the final values. Please, see the v5.

-- 
Regards,
Alexander Atanasov