On Wed, Jan 10, 2018 at 6:24 AM, Waldek Kozaczuk <jwkozac...@gmail.com>
wrote:
> Your observation was correct. It worked. At least I have not seen same
> errors in any of my tests after the change. Thanks !!!
>
Great.
The approach of adding another function "syscall_wrapper" makes system
calls a little bit slower, but I think this is negligible and we can keep
this approach. It's much easier.
> Please see my other responses in-lined.
>
> In my last email I also forgot to mention that when I was testing boltdb
> app I came across the issue in mmap(). Boltlb mmaps the file
> with MADV_RANDOM advice which is not supported by OSv implementation of
> mmap. I changed following code:
>
> unsigned libc_madvise_to_advise(int advice)
> {
> if (advice == MADV_DONTNEED) {
> return mmu::advise_dontneed;
> } else if (advice == MADV_NOHUGEPAGE) {
> return mmu::advise_nohugepage;
> }
> return 0;
> }
>
> to
>
> unsigned libc_madvise_to_advise(int advice)
> {
> if (advice == MADV_DONTNEED) {
> return mmu::advise_dontneed;
> } else if (advice == MADV_NOHUGEPAGE || advice == MADV_RANDOM) {
> return mmu::advise_nohugepage;
> }
> return 0;
> }
>
> It made my example work but I am not sure how much of a hack it is and how
> difficult it would be to support MADV_RANDOM properly.
>
To support MADV_RANDOM properly, we'll need to use it in file mappings to
avoid read-ahead. I don't think doing it is the most urgent thing we should
do.
I suggest you can add MADV_RANDOM as a separate "else if" case (or change
this to switch), and WARN_ONCE that "madvise(MADV_RANDOM) stubbed".
You can have it return mmu::advise::nohugepage but I'm not even sure it's
needed (I think the file mappings don't use huge pages anyway?).
Then we can have support for this option, but still not forget it doesn't
actually do anything.
You can send some of these patches separately, you don't have to delay it
into a huge patch series when everything works.
> I was also wondering that existing implementation of syscall stack makes
> every app thread allocate it upon creation eagerly whether syscall are
> going to be made or not. For example java apps (which I believe use libc
> functions) would be penalized by memory usage. I was wondering how
> difficult it would be to make allocation of syscall stack lazily only when
> first syscall is made.
>
The problem is that we won't have a stack to do this allocation work on.
One thing we could do have for all threads a tiny syscall stack (doesn't
even need to be a whole page, can be smaller!), and the first thing a
syscall does is
to check if this thread has this tiny stack, and if it does it allocates a
different one. The stack needs to be just big enough for malloc() to run.
We could
even avoid calling malloc() directly and just send a message to a different
thread to do this for us, but I think this will be an overkill.
Another wild idea that can save even more: Could we use one of the existing
per-thread exception stack as the temporary syscall stack (again replacing
it on the first call)?
Since we control the syscall code, we'll make sure we don't generate any
actual exception in the syscall_entry code until we replace the stack by
a new one.
> More specifically instead of allocating the stack in thread::setup_tcb()
> it would be done by C ++ function called from assembly in entry.S.
> Obviously it is an optimization which we can implement later.
>
--
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.
