linux-kernel - Re: [PATCH v16.1 6/9] virtio-balloon: Add support for providing free page reports to host

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]

Message-ID: <ada0ec83-8e7d-abb3-7053-0ec2bf2a9aa5@redhat.com>
Date:   Tue, 11 Feb 2020 13:19:31 +0100
From:   David Hildenbrand <david@...hat.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>
Cc:     Alexander Duyck <alexander.duyck@...il.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, willy@...radead.org,
        mhocko@...nel.org, linux-mm@...ck.org, akpm@...ux-foundation.org,
        mgorman@...hsingularity.net, vbabka@...e.cz,
        yang.zhang.wz@...il.com, nitesh@...hat.com, konrad.wilk@...cle.com,
        pagupta@...hat.com, riel@...riel.com, lcapitulino@...hat.com,
        dave.hansen@...el.com, wei.w.wang@...el.com, aarcange@...hat.com,
        pbonzini@...hat.com, dan.j.williams@...el.com,
        alexander.h.duyck@...ux.intel.com, osalvador@...e.de
Subject: Re: [PATCH v16.1 6/9] virtio-balloon: Add support for providing free
 page reports to host

>>
>> Did you see the discussion regarding unifying handling of
>> inflate/deflate/free_page_hinting_free_page_reporting, requested by
>> Michael? I think free page reporting is special and shall be left alone.
> 
> Not sure what do you mean by "left alone here". Could you clarify?

Don't try to unify handling like I proposed below, because it's
semantics are special.

> 
>> VIRTIO_BALLOON_F_REPORTING is nothing but a more advanced inflate, right
>> (sg, inflate based on size - not "virtio pages")?
> 
> 
> Not exactly - it's also initiated by guest as opposed to host, and
> not guided by the ballon size request set by the host.

True, but AFAIKS you could use existing INFLATE/DEFLATE in a similar
way. There is no way for the hypervisor to nack a request. The balloon
size is not glued to inflate/deflate requests. The guests manually
updates it.

> And uses a dedicated queue to avoid blocking other functionality ...

True, but the other queues also don't allow for an easy extension
AFAIKS, so that's another reason.

> 
> I really think this is more like an inflate immediately followed by deflate.

Depends on how you look at it. As inflate/deflate is not glued to the
balloon size (the guest updates the size manually), it's not obvious.

E.g., in QEMU, a deflate is just a performance improvement
("MADV_WILLNEED") - in that regard, it's more like an optional deflation.

[...]

> 
> I'd rather wait until we have a usecase and preferably a POC
> showing it helps before we add optional deflate ...
> For now I personally am fine with just making this go ahead as is,
> and imply SG and OPTIONAL_DEFLATE just for this VQ.

Also fine with me, you asked about if we can abstract any of this if I
am not wrong :) So this was my take.

> 
> Do you feel strongly we need to bring this up to a TC vote?

Not really. People have been asking about how to inflate/deflate huge
pages a long time ago (comes with different challenges - e.g., balloon
compaction). looked like this interface could have been reused for this
as well.

But yeah, I am not a fan of virtio-balloon and the whole inflate/deflate
thingy. So at least I don't see a need to extend the inflate/deflate
capability.

Free page reporting is a different story (and the semantics require no
inflate/deflate/balloon size) - it could have been moved to
virtio-whatever without any issues. So I am fine with this.

-- 
Thanks,

David / dhildenb