linux-kernel - Re: [PATCH v1 2/2] Drivers: virtio: balloon: Report inflated memory

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:   Mon, 15 Aug 2022 16:05:36 +0000
From:   Nadav Amit <namit@...are.com>
To:     Alexander Atanasov <alexander.atanasov@...tuozzo.com>
CC:     "Michael S. Tsirkin" <mst@...hat.com>,
        David Hildenbrand <david@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        "kernel@...nvz.org" <kernel@...nvz.org>,
        Wei Liu <wei.liu@...nel.org>,
        Linux Virtualization <virtualization@...ts.linux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Pv-drivers <Pv-drivers@...are.com>
Subject: Re: [PATCH v1 2/2] Drivers: virtio: balloon: Report inflated memory

On Aug 15, 2022, at 5:52 AM, Alexander Atanasov <alexander.atanasov@...tuozzo.com> wrote:

> ⚠ External Email
> 
> Hi,
> 
> On 9.08.22 20:44, Nadav Amit wrote:
>> On Aug 9, 2022, at 2:53 AM, Alexander Atanasov <alexander.atanasov@...tuozzo.com> wrote:
>> 
>>> Update the value in page_alloc on balloon fill/leak.
>> 
>> Some general comments if this patch goes forward.
>> 
>> Please cc pv-drivers@...are.com in the future.
> 
> Ok.
> 
>>> Cc: David Hildenbrand <david@...hat.com>
>>> Cc: Wei Liu <wei.liu@...nel.org>
>>> Cc: Nadav Amit <namit@...are.com>
>>> 
>>> Signed-off-by: Alexander Atanasov <alexander.atanasov@...tuozzo.com>
>>> ---
>>> drivers/virtio/virtio_balloon.c | 13 +++++++++++++
>>> 1 file changed, 13 insertions(+)
>>> 
>>> Firts user, other balloons i will do if it is accepted to avoid too much emails.
>>> 
>>> 
>>> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
>>> index b9737da6c4dd..e2693ffbd48b 100644
>>> --- a/drivers/virtio/virtio_balloon.c
>>> +++ b/drivers/virtio/virtio_balloon.c
>>> @@ -208,6 +208,16 @@ static void set_page_pfns(struct virtio_balloon *vb,
>>>                                       page_to_balloon_pfn(page) + i);
>>> }
>>> 
>>> +static void update_meminfo(struct virtio_balloon *vb)
>> 
>> Putting aside the less-than-optimal function name, I would like to ask that
> 
> Right, i will think of a better one.
> 
>> any new generic balloon logic would go into balloon_compaction.[hc] as much
> 
> If it is going to be a place for generic logic may be it should be
> renamed to balloon_common.[ch] ?
> 
>> as possible. I made the effort to reuse this infrastructure (which is now
>> used by both VMware and virtio), and would prefer to share as much code as
>> possible.
>> 
>> For instance, I would appreciate if the update upon inflate would go into
>> balloon_page_list_enqueue() and balloon_page_enqueue(). VMware's 2MB pages
>> logic is not shared, so it would require a change that is specific for
>> VMware code.
> 
> I looked at the code and i do not see how i can reuse it since
> BALLOON_COMPACTION can be disabled and as you say even for VMWare it
> would require updates on other places. Looks like if i do so i would
> have to handle update from each driver for both cases. I think it is
> better to clearly mark the spots when drivers do their internal
> recalculations and report to the core. I see only VMWare is using
> balloon_page_list_enqueue , virtio balloon is using only migration and
> i don't see how to hook it there - i haven't checked the rest of the
> balloons but i guess it would be similiar . I agree it is a good to have
> a common place for such logic but it might be better of for a separate
> work in the future.

Fine. I would live with whatever you do and if needed change it later.

Thanks for considering.

Regards,
Nadav