linux-kernel - Re: [RFC PATCH v3] sched: Fix performance regression introduced by mm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230411115330.GB580235@hirez.programming.kicks-ass.net>
Date:   Tue, 11 Apr 2023 13:53:30 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     linux-kernel@...r.kernel.org, Aaron Lu <aaron.lu@...el.com>,
        Olivier Dion <odion@...icios.com>, michael.christie@...cle.com,
        npiggin@...il.com
Subject: Re: [RFC PATCH v3] sched: Fix performance regression introduced by
 mm_cid

On Tue, Apr 11, 2023 at 01:03:45PM +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().
> > +		 */
> > +	}

The possibly nicer way to write all this is:

	if (!prev->mm != !next->mm)
		smp_mb();

And then clean up the mm{grab,drop)_lazy_tlb() helpers. But we can
always do that later if we indeed end up with all this ...