Messages in this thread | | | Date | Wed, 7 Oct 2020 17:07:04 +0200 | From | Peter Zijlstra <> | Subject | Re: [RFC PATCH 2/3] sched: membarrier: cover kthread_use_mm (v3) |
| |
On Thu, Sep 24, 2020 at 01:25:07PM -0400, Mathieu Desnoyers wrote:
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 2d95dc3f4644..bab6f4f2809f 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -3736,6 +3736,8 @@ context_switch(struct rq *rq, struct task_struct *prev, > */ > arch_start_context_switch(prev); > > + membarrier_switch_mm(rq, prev->mm, next->mm); > + > /* > * kernel -> kernel lazy + transfer active > * user -> kernel lazy + mmgrab() active > @@ -3752,7 +3754,6 @@ context_switch(struct rq *rq, struct task_struct *prev, > else > prev->active_mm = NULL; > } else { // to user > - membarrier_switch_mm(rq, prev->active_mm, next->mm); > /* > * sys_membarrier() requires an smp_mb() between setting > * rq->curr / membarrier_switch_mm() and returning to userspace.
I was thinking... do we need the above, when:
> diff --git a/kernel/sched/membarrier.c b/kernel/sched/membarrier.c > index 8bc8b8a888b7..e5246580201b 100644 > --- a/kernel/sched/membarrier.c > +++ b/kernel/sched/membarrier.c > @@ -112,13 +112,9 @@ static int membarrier_global_expedited(void) > MEMBARRIER_STATE_GLOBAL_EXPEDITED)) > continue; > > - /* > - * Skip the CPU if it runs a kernel thread. The scheduler > - * leaves the prior task mm in place as an optimization when > - * scheduling a kthread. > - */ > + /* Skip the CPU if it runs the idle thread. */ > p = rcu_dereference(cpu_rq(cpu)->curr); > - if (p->flags & PF_KTHREAD)
We retain this in the form:
if ((p->flags & PF_KTHREAD) && !p-mm) continue;
> + if (is_idle_task(p)) > continue; > > __cpumask_set_cpu(cpu, tmpmask);
Specifically, we only care about kthreads when they're between kthread_use_mm() / kthread_unuse_mm(), and in that case they will have updated state already.
It's too late in the day to be sure about the memory ordering though; but if we see !->mm, they'll do/have-done switch_mm() which implies sufficient barriers().
Hmm?
| |