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

[Aaron Lu]

On Mon, Apr 03, 2023 at 08:13:08PM -0400, Mathieu Desnoyers wrote:
> On 2023-04-03 14:13, Mathieu Desnoyers wrote:
> [...]
> > +/*
> > + * Migration to dst cpu. Called with dst_rq lock held.
> > + */
> > +void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t, int src_cid)
> > +{
> > +	struct mm_struct *mm = t->mm;
> > +	int dst_cid, *dst_pcpu_cid;
> > +
> > +	lockdep_assert_rq_held(dst_rq);
> > +
> > +	if (!mm || src_cid == -1)
> > +		return;
> > +
> > +	dst_pcpu_cid = per_cpu_ptr(mm->pcpu_cid, cpu_of(dst_rq));
> > +
> > +	/*
> > +	 * If destination cpu cid is greater than the source cpu cid, unset it
> > +	 * so it can be reallocated.
> > +	 */
> > +	dst_cid = *dst_pcpu_cid;
> > +	if (dst_cid == -1 || dst_cid < src_cid)
> 
> Small detail: I plan to change this from "dst_cid < src_cid" to
> "dst_cid  <= src_cid" in my next version of the patch to handle the
> unlikely case where a task would be migrated back and forth between
> two runqueues without being scheduled. It would be possible that the
> task's last_mm_cid is equal to the dst_cid here, in which case it
> would be better to leave the mm's destination cpu cid set.
>

This patch is still good regarding lock contention.
(I applied the above small change while testing)

> > +		return;
> > +	*dst_pcpu_cid = -1;
> > +	/*
> > +	 * Put dst_cid if it is not currently in use, else it will be lazy put
> > +	 * on the next context switch.
> > +	 */
> > +	if (dst_rq->curr->mm != mm)
> > +		__mm_cid_put(mm, dst_cid);
> > +}

Thanks,
Aaron