On Mon, Sep 19, 2016 at 1:11 PM, Gleb Natapov <g...@scylladb.com> wrote:

> > So if I understand correctly, both your and Gleb's sentiment is there is
> no
> > need not to make the scheduler more forgiving for page table changes, but
> > rather require that sched::threads objects live in a mapping which could
> > never change - which more-or-less means in the OSv case, that
> sched::thread
> > objects should be allocated only on the heap.
> >
> As most other things this is trade-off. Allowing sched::threads to live
> in a stack is not worth removing the optimization for, but may be something
> else will provide so much benefit that allowing non app threads accessing
> mmaped area will be worth supporting. Optimization may still avoid
> sending IPI to idle cpus.
>

So was that the main benefit of that lazy TLB patch - idle CPUs, not
"system threads" in general?


>
> > So a patch which will enforce sched::thread to be allocated only on the
> > heap is the right way to go here? (I sent such a patch already, but I'll
> > try to send a slightly less ugly one. In any case it will be a long
> patch).
> >
> I think so. BTW can this be fixed by forcing tlb flush when sending new
> thread to another cpu somehow?
>

This is more-or-less what Timmons' patch did: before the scheduler on the
target CPU wants to inspect the incoming threads, it checks if it needs to
flush
the TLB.

What bothered me with that patch was what I wrote in my previous email -
that
the check is done using that lazy_tlb_flush flag - but the existing code
zeros this
flag as soon as it decides to send an IPI, so at that point the target
scheduler will
no longer know that it needs to flush the TLB. So I think for this to be
correct we
need to leave the flag set until we actually flushed the TLB (so the target
CPU, not
the source CPU, will zero this flag). That's also easy to change, I think.

So now I need to decide whether to "fix" the support for on-stack thread
objects,
or just enforce sched::thread to always be dynamically allocated.

On-stack threads (or more generally, sched::thread as a member, rather than
a
pointer - even if not on the stack) are a strange thing for other reasons
as well -
e.g., their detach() support is incorrect and there is no way to NOT wait
for them.
So perhaps it makes sense to get rid of them for other reasons as well.


>
> > By the way, it is obvious that a sched::thread cannot live in a paged-out
> > mapping, as the scheduler cannot sleep to wait for it. But the issue here
> > is more subtle: stack memory is guaranteed to be always paged in (for
> > reasons explained here https://github.com/cloudius-
> systems/osv/issues/143)
> > - and nontheless, it is still not fine to put a sched::thread there
> because
> > of the TLB flushing issue.
>
> --
>                         Gleb.
>

-- 
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