linux-kernel - Re: [PATCH v1 3/3] virtio-balloon: Switch back to OOM handler for VIRTIO_BALLOON_F_DEFLATE_ON

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, 6 Feb 2020 10:05:43 +0100
From:   David Hildenbrand <david@...hat.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        virtualization@...ts.linux-foundation.org,
        Tyler Sanderson <tysand@...gle.com>,
        Wei Wang <wei.w.wang@...el.com>,
        Alexander Duyck <alexander.h.duyck@...ux.intel.com>,
        David Rientjes <rientjes@...gle.com>,
        Nadav Amit <namit@...are.com>, Michal Hocko <mhocko@...nel.org>
Subject: Re: [PATCH v1 3/3] virtio-balloon: Switch back to OOM handler for
 VIRTIO_BALLOON_F_DEFLATE_ON_OOM

>> commit bf50e69f63d21091e525185c3ae761412be0ba72
>> Author: Dave Hansen <dave@...ux.vnet.ibm.com>
>> Date:   Thu Apr 7 10:43:25 2011 -0700
>>
>>     virtio balloon: kill tell-host-first logic
>>
>>     The virtio balloon driver has a VIRTIO_BALLOON_F_MUST_TELL_HOST
>>     feature bit.  Whenever the bit is set, the guest kernel must
>>     always tell the host before we free pages back to the allocator.
>>     Without this feature, we might free a page (and have another
>>     user touch it) while the hypervisor is unprepared for it.
>>
>>     But, if the bit is _not_ set, we are under no obligation to
>>     reverse the order; we're under no obligation to do _anything_.
>>     As of now, qemu-kvm defines the bit, but doesn't set it.
> 
> Well this is not what the spec says in the end.

I didn't check the spec, maybe I should do that :)

> To continue that commit message:
> 
>     This patch makes the "tell host first" logic the only case.  This
>     should make everybody happy, and reduce the amount of untested or
>     untestable code in the kernel.

Yeah, but this comment explains that the current deflate is only in
place, because it makes the code simpler (to support both cases). Of
course, doing the deflate might result in performance improvements.
(e.g., MADV_WILLNEED)

> 
> you can try proposing the change to the virtio TC, see what do others
> think.

We can just drop the comment from this patch for now. The tell_host host
not be an issue AFAIKS.

-- 
Thanks,

David / dhildenb