linux-kernel - Re: [RFC][Patch v8 0/7] KVM: Guest Free Page Hinting

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, 18 Feb 2019 21:04:57 +0100
From:   David Hildenbrand <david@...hat.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>
Cc:     Nitesh Narayan Lal <nitesh@...hat.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, pbonzini@...hat.com,
        lcapitulino@...hat.com, pagupta@...hat.com, wei.w.wang@...el.com,
        yang.zhang.wz@...il.com, riel@...riel.com, dodgen@...gle.com,
        konrad.wilk@...cle.com, dhildenb@...hat.com, aarcange@...hat.com,
        Alexander Duyck <alexander.duyck@...il.com>
Subject: Re: [RFC][Patch v8 0/7] KVM: Guest Free Page Hinting

>>>>> So I'm fine with a simple implementation but the interface needs to
>>>>> allow the hypervisor to process hints in parallel while guest is
>>>>> running.  We can then fix any issues on hypervisor without breaking
>>>>> guests.
>>>>
>>>> Yes, I am fine with defining an interface that theoretically let's us
>>>> change the implementation in the guest later.
>>>> I consider this even a
>>>> prerequisite. IMHO the interface shouldn't be different, it will be
>>>> exactly the same.
>>>>
>>>> It is just "who" calls the batch freeing and waits for it. And as I
>>>> outlined here, doing it without additional threads at least avoids us
>>>> for now having to think about dynamic data structures and that we can
>>>> sometimes not report "because the thread is still busy reporting or
>>>> wasn't scheduled yet".
>>>
>>> Sorry I wasn't clear. I think we need ability to change the
>>> implementation in the *host* later. IOW don't rely on
>>> host being synchronous.
>>>
>>>
>> I actually misread it :) . In any way, there has to be a mechanism to
>> synchronize.
>>
>> If we are going via a bare hypercall (like s390x, like what Alexander
>> proposes), it is going to be a synchronous interface either way. Just a
>> bare hypercall, there will not really be any blocking on the guest side.
> 
> It bothers me that we are now tied to interface being synchronous. We
> won't be able to fix it if there's an issue as that would break guests.

I assume with "fix it" you mean "fix kfree taking longer on every X call"?

Yes, as I initially wrote, this mimics s390x. That might be good (we
know it has been working for years) and bad (we are inheriting the same
problem class, if it exists). And being synchronous is part of the
approach for now.

I tend to focus on the first part (we don't know anything besides it is
working) while you focus on the second part (there could be a potential
problem). Having a real problem at hand would be great, then we would
know what exactly we actually have to fix. But read below.

> 
>> Via virtio, I guess it is waiting for a response to a requests, right?
> 
> For the buffer to be used, yes. And it could mean putting some pages
> aside until hypervisor is done with them. Then you don't need timers or
> tricks like this, you can get an interrupt and start using the memory.

I am very open to such an approach as long as we can make it work and it
is not too complicated. (-> simple)

This would mean for example

1. Collect entries to be reported per VCPU in a buffer. Say magic number
256/512.

2. Once the buffer is full, do crazy "take pages out of the balloon
action" and report them to the hypervisor via virtio. Let the VCPU
continue. This will require some memory to store the request. Small
hickup for the VCPU to kick of the reporting to the hypervisor.

3. On interrupt/response, go over the response and put the pages back to
the buddy.

(assuming that reporting a bulk of frees is better than reporting every
single free obviously)

This could allow nice things like "when OOM gets trigger, see if pages
are currently being reported and wait until they have been put back to
the buddy, return "new pages available", so in a real "low on memory"
scenario, no OOM killer would get involved. This could address the issue
Wei had with reporting when low on memory.

Is that something you have in mind? I assume we would have to allocate
memory when crafting the new requests. This is the only reason I tend to
prefer a synchronous interface for now. But if allocation is not a
problem, great.

-- 

Thanks,

David / dhildenb