Re: [PATCH] KVM: s390: remove delayed reallocation of page tables for KVM

[Christian Borntraeger <borntraeger@de.ibm.com>]

Am 23.04.2015 um 14:01 schrieb Alexander Graf:
> 
> 
>> Am 23.04.2015 um 13:43 schrieb Christian Borntraeger <borntraeger@de.ibm.com>:
>>
>>> Am 23.04.2015 um 13:37 schrieb Alexander Graf:
>>>
>>>
>>>> Am 23.04.2015 um 13:08 schrieb Christian Borntraeger <borntraeger@de.ibm.com>:
>>>>
>>>> From: Martin Schwidefsky <schwidefsky@de.ibm.com>
>>>>
>>>> Replacing a 2K page table with a 4K page table while a VMA is active
>>>> for the affected memory region is fundamentally broken. Rip out the
>>>> page table reallocation code and replace it with a simple system
>>>> control 'vm.allocate_pgste'. If the system control is set the page
>>>> tables for all processes are allocated as full 4K pages, even for
>>>> processes that do not need it.
>>>>
>>>> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
>>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>
>>> Couldn't you make this a hidden kconfig option that gets automatically selected when kvm is enabled? Or is there a non-kvm case that needs it too?
>>
>> For things like RHEV the default could certainly be "enabled", but for normal
>> distros like SLES/RHEL, the idea was to NOT enable that by default, as the non-KVM
>> case is more common and might suffer from the additional memory consumption of
>> the page tables. (big databases come to mind)
>>
>> We could think about having rpms like kvm to provide a sysctl file that sets it if we
>> want to minimize the impact. Other ideas?
> 
> Oh, I'm sorry, I misread the ifdef. I don't think it makes sense to have a config option for the default value then, just rely only on sysctl.conf for changed defaults.
> 
> As far as mechanisms to change it go, every distribution has their own ways of dealing with this. RH has a "profile" thing, we don't really have anything central, but individual sysctl.d files for example that a kvm package could provide.
> Either way, the default choosing shouldn't happen in .config ;).

So you vote for getting rid of the Kconfig?

Also, please add some helpful error message in qemu to guide users to the sysctl.

Yes, we will provide a qemu patch (cc stable) after this hits the kernel.

> As far as alternative approaches go, I don't have a great idea otoh. We could have an elf flag indicating that this process needs 4k page tables to limit the impact to a single process.

This approach was actually Martins first fix. The problem is that the decision takes place on execve,
but we need an answer at fork time. So we always started with 4k page tables and freed the 2nd halv on
execve. Now this did not work for processes that only fork (without execve).

> In fact, could we maybe still limit the scope to non-global? A personality may work as well. Or ulimit?

I think we will go for now with the sysctl and see if we can come up with some automatic way as additional
patch later on.

Christian