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

[Mathieu Desnoyers]

On 2023-04-11 04:53, Peter Zijlstra wrote:
> On Wed, Apr 05, 2023 at 12:26:35PM -0400, Mathieu Desnoyers wrote:
> > +void sched_mm_cid_migrate_from(struct task_struct *t)
> > +{
>
> > +	/*
> > +	 * If the source cpu cid is set, and matches the last cid of the
> > +	 * migrated task, clear the source cpu cid to keep cid allocation
> > +	 * compact to cover the case where this task is the last task using
> > +	 * this mm on the source cpu. If there happens to be other tasks left
> > +	 * on the source cpu using this mm, the next task using this mm will
> > +	 * reallocate its cid on context switch.
> > +	 *
> > +	 * We cannot keep ownership of concurrency ID without runqueue
> > +	 * lock held when it is not used by a current task, because it
> > +	 * would lead to allocation of more concurrency ids than there
> > +	 * are possible cpus in the system. The last_mm_cid is used as
> > +	 * a hint to conditionally unset the dst cpu cid, keeping
> > +	 * allocated concurrency ids compact.
> > +	 */
> > +	if (cmpxchg(src_pcpu_cid, src_cid, mm_cid_set_lazy_put(src_cid)) != src_cid)
> > +		return;
> > +
>
> FWIW, I'm thinking that if we write this using try_cmpxchg() it'll be a
> little nicer:
>
> 	lazy_cid = mm_cid_set_lazy_put(src_cid);
> 	if (!try_cmpxchg(src_pcpu_cid, &src_cid, lazy_cid))
> 		return;

Yes, done.

> > +	/*
> > +	 * The implicit barrier after cmpxchg per-mm/cpu cid before loading
> > +	 * rq->curr->mm matches the scheduler barrier in context_switch()
> > +	 * between store to rq->curr and load of prev and next task's
> > +	 * per-mm/cpu cid.
> > +	 *
> > +	 * The implicit barrier after cmpxchg per-mm/cpu cid before loading
> > +	 * rq->curr->mm_cid_active matches the barrier in
> > +	 * sched_mm_cid_exit_signals(), sched_mm_cid_before_execve(), and
> > +	 * sched_mm_cid_after_execve() between store to t->mm_cid_active and
> > +	 * load of per-mm/cpu cid.
> > +	 */
> > +
> > +	/*
> > +	 * If we observe an active task using the mm on this rq after setting the lazy-put
> > +	 * flag, this task will be responsible for transitioning from lazy-put
> > +	 * flag set to MM_CID_UNSET.
> > +	 */
> > +	rcu_read_lock();
> > +	src_task = rcu_dereference(src_rq->curr);
> > +	if (src_task->mm_cid_active && src_task->mm == mm) {
> > +		rcu_read_unlock();
> > +		/*
> > +		 * We observed an active task for this mm, clearing the destination
> > +		 * cpu mm_cid is not relevant for compactness.
> > +		 */
> > +		t->last_mm_cid = -1;
> > +		return;
> > +	}
> > +	rcu_read_unlock();
> > +
> > +	/*
> > +	 * The src_cid is unused, so it can be unset.
> > +	 */
> > +	if (cmpxchg(src_pcpu_cid, mm_cid_set_lazy_put(src_cid), MM_CID_UNSET) != mm_cid_set_lazy_put(src_cid))
> > +		return;
>
> 	if (!try_cmpxchg(src_pcpu_cid, &lazy_cid, MM_CID_UNSET))
> 		return;

done,

Thanks!

Mathieu

> > +	__mm_cid_put(mm, src_cid);
> > +}

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