On Mon, Sep 19, 2016 at 01:30:15PM +0300, Nadav Har'El wrote: > 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? > Both. If we'll allow to run multiple apps one day in separate namespaces we can flush only cpus that run threads of the same application (this is what Linux does as you probably guess).
> > > > > > 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. > It is. Will cause superfluous flushes though. > 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. > I think the later is very reasonable restriction. > 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. > > -- 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.
