Re: [PATCH v1 5/5] accel/tcg: better handle memory constrained systems

[Alex Bennée]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Fri, Jul 17, 2020 at 11:51:39AM +0100, Alex Bennée wrote:
>> It turns out there are some 64 bit systems that have relatively low
>> amounts of physical memory available to them (typically CI system).
>> Even with swapping available a 1GB translation buffer that fills up
>> can put the machine under increased memory pressure. Detect these low
>> memory situations and reduce tb_size appropriately.
>> 
>> Fixes: 600e17b261
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: BALATON Zoltan <balaton@eik.bme.hu>
>> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>> ---
>>  accel/tcg/translate-all.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
>> index 2afa46bd2b1..2ff0ba6d19b 100644
>> --- a/accel/tcg/translate-all.c
>> +++ b/accel/tcg/translate-all.c
>> @@ -976,7 +976,12 @@ static inline size_t size_code_gen_buffer(size_t 
>> tb_size)
>>  {
>>      /* Size the buffer.  */
>>      if (tb_size == 0) {
>> -        tb_size = DEFAULT_CODE_GEN_BUFFER_SIZE;
>> +        size_t phys_mem = qemu_get_host_physmem();
>> +        if (phys_mem > 0 && phys_mem < (2 * DEFAULT_CODE_GEN_BUFFER_SIZE)) {
>> +            tb_size = phys_mem / 4;
>> +        } else {
>> +            tb_size = DEFAULT_CODE_GEN_BUFFER_SIZE;
>> +        }
>
> I'm not convinced this is going to work when running QEMU inside a
> container environment with RAM cap, because the physmem level is
> completely unrelated to the RAM the container is permitted to actually
> use in practice. ie host has 32 GB of RAM, but the container QEMU is
> in only has 1 GB permitted.

What will happen when the mmap happens? Will a capped container limit
the attempted mmap? I would hope the container case at least gave
different feedback than a "silent" OOM.

> I don't have much of a better suggestion, as I don't think we want
> to get into reading the cgroups memory limits. It does feel like the
> assumption we can blindly use a 1GB cache though is invalid even
> with this patch applied.
>
> Regards,
> Daniel


-- 
Alex Bennée