Messages in this thread | | | Subject | Re: [PATCH v2 11/11] mm,sched: conditionally skip lazy TLB mm refcounting | From | Rik van Riel <> | Date | Mon, 30 Jul 2018 15:15:46 -0400 |
| |
On Mon, 2018-07-30 at 18:26 +0200, Peter Zijlstra wrote: > On Mon, Jul 30, 2018 at 10:30:11AM -0400, Rik van Riel wrote: > > > > What happened to the rework I did there? That not only avoided > > > fiddling > > > with active_mm, but also avoids grab/drop cycles for the other > > > architectures when doing task->kthread->kthread->task things. > > > > I don't think I saw that. I only saw your email from > > July 20th with this fragment of code, which does not > > appear to avoid the grab/drop cycles, and still fiddles > > with active_mm: > > Yeah, that's it. Note how it doesn't do a grab+drop for kernel- > >kernel, > where the current could would have. > > And also note that it only fiddles with active_mm if it does the > grab+drop thing (the below should have s/ifdef/ifndef/ to make more > sense maybe).
I'll kick off a test with your variant. I don't think we will see any performance difference on x86 (due to not using a refcount at all any more), but unless Ingo is in a hurry I guess there's no issue rewriting this part of the patch series :)
Do the other patches look ok to you and Andy?
> So for ARCH_NO_ACTIVE_MM we never touch ->active_mm and therefore > ->active_mm == ->mm. > > > + /* > > + * kernel -> kernel lazy + transfer active > > + * user -> kernel lazy + mmgrab() active > > + * > > + * kernel -> user switch + mmdrop() active > > + * user -> user switch > > + */ > > + if (!next->mm) { // to > > kernel > > + enter_lazy_tlb(prev->active_mm, next); > > + > > #ifndef ARCH_NO_ACTIVE_MM > > + next->active_mm = prev->active_mm; > > + if (prev->mm) // from > > user > > + mmgrab(prev->active_mm); > > else > prev->active_mm = NULL; > > +#endif > > + } else { // to user > > + switch_mm_irqs_off(prev->active_mm, next->mm, > > next); > > + > > #ifndef ARCH_NO_ACTIVE_MM > > + if (!prev->mm) { // from > > kernel > > + /* will mmdrop() in finish_task_switch(). > > */ > > + rq->prev_mm = prev->active_mm; > > + prev->active_mm = NULL; > > + } > > +#endif > > > -- All Rights Reversed.[unhandled content-type:application/pgp-signature] | |