On Thursday, September 6, 2018 at 11:01:32 AM UTC-4, Waldek Kozaczuk wrote:
>
>
>
> On Thu, Sep 6, 2018 at 9:57 AM Nadav Har'El <n...@scylladb.com> wrote:
>
>>
>> On Thu, Sep 6, 2018 at 3:11 PM, Waldek Kozaczuk <jwkozac...@gmail.com> 
>> wrote:
>>
>>>
>>> On Thu, Sep 6, 2018 at 4:36 AM Nadav Har'El <n...@scylladb.com> wrote:
>>>
>>>>
>>>> On Thu, Sep 6, 2018 at 5:32 AM, Waldemar Kozaczuk <jwkozac...@gmail.com
>>>> > wrote:
>>>>
>>>>> This patch changes syscall stack implementation to use
>>>>> mmap/munmap/mprotect combination instead of malloc/free
>>>>> for large syscall stack. This makes it possible to add guard page
>>>>> to detect stack overflow scenarios. Lastly new implementation 
>>>>> also detects if tiny stack is not overflows when
>>>>> executing logic to setup large stack.
>>>>> Tiny stack had to be increased to 4096 bytes to accomodate
>>>>> mmap call.
>>>>>
>>>>
>>>> This is unfortunate, as this will add memory use to each and every 
>>>> thread (see also https://github.com/cloudius-systems/osv/issues/972)
>>>> I wonder if we could use less than 4096 (less enough to ensure more 
>>>> than one of them can fit a page) or do something to mmap code to make it 
>>>> use less stack.
>>>>
>>> I experimented with it and it looks like 1280 bytes (256 more) is 
>>> enough. Leaving it at 1024 triggers nice assert showing tiny stack 
>>> overflow. 
>>>
>>
>> Sadly it turns out (see memory::pool::max_object_size which is 
>> page_size/4) that our memory allocator, if allocating anything between 1025 
>> and 4096 bytes, allocates whole pages.
>> So we don't actually win anything by using 1280 instead of 4096 :-( and 
>> we may suffer later if small compiler changes or whatever will suddenly 
>> cause us
>> to need a bit more than 1280. So maybe we should just go back to 4096?
>>
> Does it actually mean if app allocates between 1025 and 4096 we would be 
wasting almost 75% or RAM for each of those?

> I will send new patch. 
>
>>
>> If you're wondering we don't use the small allocator (with several 
>> objects per page) right up to  page_size/2 -
>> Each memory-pool page has a header and then an array of these chunks. We 
>> *can't* fit a header plus two page_size/2 chunks...
>> I  think it should have been possible to have a page_size/2-sized pool, 
>> just don't use it all the way up to page_size/2, and only use it for 
>> allocations up to (page_size-sizeof(page_header))/2, which I think is 2032 
>> bytes.
>>
> I think we could also address it meanwhile (until we implement 972) by 
> using some kind of shared memory pool dedicated to tiny stacks. So for 
> example every time we initialize new app thread we would allocate memory 
> enough for 2 or more threads. 
>
>> Allocating between 2032 and 2048 bytes could have fallen back to 
>> allocating a full page (as will happen for more than 2048 bytes).
>> I don't know why this wasn't done. I'm CC'ing Pawel, who wrote this code, 
>> maybe he remembers. 
>>
>> Yeah it would be nice to address it or at least have an issue open. This 
> could help us make OSv more memory efficient in general (not sure how these 
> kind of small mallocs are common - probably varies by application). 
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to