Re: [PATCH 11/11] mm,sched: conditionally skip lazy TLB mm refcounting

[Peter Zijlstra]

On Fri, Aug 03, 2018 at 12:40:48PM -0400, Rik van Riel wrote:
> On Fri, 2018-08-03 at 17:56 +0200, Peter Zijlstra wrote:
> > On Wed, Aug 01, 2018 at 06:02:55AM -0400, Rik van Riel wrote:
> > > Conditionally skip lazy TLB mm refcounting. When an architecture
> > > has
> > > CONFIG_ARCH_NO_ACTIVE_MM_REFCOUNTING enabled, an mm that is used in
> > > lazy TLB mode anywhere will get shot down from exit_mmap, and there
> > > in no need to incur the cache line bouncing overhead of refcounting
> > > a lazy TLB mm.
> > > 
> > > Implement this by moving the refcounting of a lazy TLB mm to helper
> > > functions, which skip the refcounting when it is not necessary.
> > > 
> > > Deal with use_mm and unuse_mm by fully splitting out the
> > > refcounting
> > > of the lazy TLB mm a kernel thread may have when entering use_mm
> > > from
> > > the refcounting of the mm that use_mm is about to start using.
> > 
> > 
> > > @@ -2803,16 +2803,29 @@ context_switch(struct rq *rq, struct
> > > task_struct *prev,
> > >  	 * membarrier after storing to rq->curr, before returning
> > > to
> > >  	 * user-space.
> > >  	 */
> > > +	/*
> > > +	 * kernel -> kernel	lazy + transfer active
> > > +	 *   user -> kernel	lazy + grab_lazy_mm active
> > > +	 *
> > > +	 * kernel ->   user	switch + drop_lazy_mm active
> > > +	 *   user ->   user	switch
> > > +	 */
> > > +	if (!mm) {				// to kernel
> > >  		next->active_mm = oldmm;
> > >  		enter_lazy_tlb(oldmm, next);
> > > +
> > > +		if (prev->mm)			// from user
> > > +			grab_lazy_mm(oldmm);
> > > +		else
> > > +			prev->active_mm = NULL;
> > > +	} else {				// to user
> > >  		switch_mm_irqs_off(oldmm, mm, next);
> > >  
> > > +		if (!prev->mm) {		// from kernel
> > > +			/* will drop_lazy_mm() in
> > > finish_task_switch(). */
> > > +			rq->prev_mm = oldmm;
> > > +			prev->active_mm = NULL;
> > > +		}
> > >  	}
> > 
> > So this still confuses the heck out of me; and the Changelog doesn't
> > seem to even mention it. You still track and swizzle ->active_mm but
> > no
> > longer refcount it.
> > 
> > Why can't we skip the ->active_mm swizzle and keep ->active_mm ==
> > ->mm.
> > 
> > Doing the swizzle but not the refcount just makes me itch.
> 
> I am working on that now, it adds another 7-8
> patches on top of this series.

I thought those were taking ->active_mm out entirely, not avoiding the
swizzle, but I might have missed something in the middle :-)

> The big question is, do we want this optimization
> to wait for further cleanups, or should we run with
> code that seems to be stable right now, and put
> additional cleanups and enhancements on top of it
> later?

At the very least the Changelog needs to explain why we cannot do away
with the swizzle now and how doing the swizzle without the refcounting
is not completely broken (I think I see, but urgh).