Re: [RFC PATCH v3] sched: Fix performance regression introduced by mm_cid

[Mathieu Desnoyers]

On 2023-04-11 06:25, Peter Zijlstra wrote:
> On Tue, Apr 11, 2023 at 11:37:05AM +0200, Peter Zijlstra wrote:
> > On Fri, Apr 07, 2023 at 09:14:36PM -0400, Mathieu Desnoyers wrote:
> >
> > > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> > > index bc0e1cd0d6ac..f3e7dc2cd1cc 100644
> > > --- a/kernel/sched/sched.h
> > > +++ b/kernel/sched/sched.h
> > > @@ -3354,6 +3354,37 @@ static inline int mm_cid_get(struct mm_struct *mm)
> > >   static inline void switch_mm_cid(struct task_struct *prev, struct task_struct *next)
> > >   {
> > > +	/*
> > > +	 * Provide a memory barrier between rq->curr store and load of
> > > +	 * {prev,next}->mm->pcpu_cid[cpu] on rq->curr->mm transition.
> > > +	 *
> > > +	 * Should be adapted if context_switch() is modified.
> > > +	 */
> > > +	if (!next->mm) {                                // to kernel
> > > +		/*
> > > +		 * user -> kernel transition does not guarantee a barrier, but
> > > +		 * we can use the fact that it performs an atomic operation in
> > > +		 * mmgrab().
> > > +		 */
> > > +		if (prev->mm)                           // from user
> > > +			smp_mb__after_mmgrab();
> > > +		/*
> > > +		 * kernel -> kernel transition does not change rq->curr->mm
> > > +		 * state. It stays NULL.
> > > +		 */
> > > +	} else {                                        // to user
> > > +		/*
> > > +		 * kernel -> user transition does not provide a barrier
> > > +		 * between rq->curr store and load of {prev,next}->mm->pcpu_cid[cpu].
> > > +		 * Provide it here.
> > > +		 */
> > > +		if (!prev->mm)                          // from kernel
> > > +			smp_mb();
> > > +		/*
> > > +		 * user -> user transition guarantees a memory barrier through
> > > +		 * switch_mm().
> > > +		 */
> >
> > What about the user->user case where next->mm == prev->mm ? There
> > sys_membarrier() relies on finish_task_switch()'s mmdrop(), but we
> > can't.
>
> Ah, I suppose that's either a N->N or Y->Y transition and we don't care.
>
> Not the clearest comment though.

For sake of completeness, here is the updated comment:



                /*
                 * user -> user transition guarantees a memory barrier through
                 * switch_mm() when current->mm changes. If current->mm is
                 * unchanged, no barrier is needed.
                 */

Thanks,

Mathieu


--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com