linux-kernel - Re: [PATCH v4 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:   Thu, 14 Jul 2022 16:35:46 +0300
From:   Alexander Atanasov <alexander.atanasov@...tuozzo.com>
To:     David Hildenbrand <david@...hat.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>
Cc:     kernel@...nvz.org, virtualization@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 1/1] Create debugfs file with virtio balloon usage
 information

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@...tuozzo.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