Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d97b567d-a86b-c180-a019-f66962c4a6ac@efficios.com>
Date:   Tue, 11 Apr 2023 09:10:10 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [RFC PATCH v3] sched: Fix performance regression introduced by
 mm_cid
Content-Language: en-US
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     linux-kernel@vger.kernel.org, Aaron Lu <aaron.lu@intel.com>,
        Olivier Dion <odion@efficios.com>, michael.christie@oracle.com
References: <20230405162635.225245-1-mathieu.desnoyers@efficios.com>
 <386a6e32-a746-9eb1-d5ae-e5bedaa8fc75@efficios.com>
 <20230406095122.GF386572@hirez.programming.kicks-ass.net>
 <fdaa7242-4ddd-fbe2-bc0e-6c62054dbde8@efficios.com>
 <3b4684ea-5c0d-376b-19cf-195684ec4e0e@efficios.com>
 <20230411093705.GB578657@hirez.programming.kicks-ass.net>
 <20230411102532.GA580235@hirez.programming.kicks-ass.net>
From:   Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
In-Reply-To: <20230411102532.GA580235@hirez.programming.kicks-ass.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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