Re: [PATCH tip/core/rcu 4/5] sys_membarrier: Add expedited option

From: Peter Zijlstra
Date: Tue Jul 25 2017 - 17:55:45 EST


On Tue, Jul 25, 2017 at 02:19:26PM -0700, Paul E. McKenney wrote:
> On Tue, Jul 25, 2017 at 10:24:51PM +0200, Peter Zijlstra wrote:
> > On Tue, Jul 25, 2017 at 12:36:12PM -0700, Paul E. McKenney wrote:
> >
> > > There are a lot of variations, to be sure. For whatever it is worth,
> > > the original patch that started this uses mprotect():
> > >
> > > https://github.com/msullivan/userspace-rcu/commit/04656b468d418efbc5d934ab07954eb8395a7ab0
> >
> > FWIW that will not work on s390 (and maybe others), they don't in fact
> > require IPIs for remote TLB invalidation.
>
> Nor will it for ARM. Nor (I think) for PowerPC. But that is in fact
> what people are doing right now in real life. Hence my renewed interest
> in sys_membarrier().

People always do crazy stuff, but what surprised me is that such s patch
got merged in urcu even though its known broken for a number of
architectures.

> But it would not be hard for userspace code to force IPIs by repeatedly
> awakening higher-priority threads that sleep immediately after being
> awakened, right?

RT tasks are not readily available to !root, and the user might have
been constrained to a subset of available CPUs.

> > Well, I'm not sure there is an easy means of doing machine wide IPIs for
> > !root out there. This would be a first.
> >
> > Something along the lines of:
> >
> > void dummy(void *arg)
> > {
> > /* IPIs are assumed to be serializing */
> > }
> >
> > void ipi_mm(struct mm_struct *mm)
> > {
> > cpumask_var_t cpus;
> > int cpu;
> >
> > zalloc_cpumask_var(&cpus, GFP_KERNEL);
> >
> > for_each_cpu(cpu, mm_cpumask(mm)) {
> > struct task_struct *p;
> >
> > /*
> > * If the current task of @cpu isn't of this @mm, then
> > * it needs a context switch to become one, which will
> > * provide the ordering we require.
> > */
> > rcu_read_lock();
> > p = task_rcu_dereference(&cpu_curr(cpu));
> > if (p && p->mm == mm)
> > __cpumask_set_cpu(cpu, cpus);
> > rcu_read_unlock();
> > }
> >
> > on_each_cpu_mask(cpus, dummy, NULL, 1);
> > }
> >
> > Would appear to be minimally invasive and only shoot at CPUs we're
> > currently running our process on, which greatly reduces the impact.
>
> I am good with this approach as well, and I do very much like that it
> avoids IPIing CPUs that aren't running our process (at least in the
> common case). But don't we also need added memory ordering? It is
> sort of OK to IPI a CPU that just now switched away from our process,
> but not so good to miss IPIing a CPU that switched to our process just
> a little before sys_membarrier().

My thinking was that if we observe '!= mm' that CPU will have to do a
context switch in order to make it true. That context switch will
provide the ordering we're after so all is well.

Quite possible there's a hole in, but since I'm running on fumes someone
needs to spell it out for me :-)

> I was intending to base this on the last few versions of a 2010 patch,
> but maybe things have changed:
>
> https://marc.info/?l=linux-kernel&m=126358017229620&w=2
> https://marc.info/?l=linux-kernel&m=126436996014016&w=2
> https://marc.info/?l=linux-kernel&m=126601479802978&w=2
> https://marc.info/?l=linux-kernel&m=126970692903302&w=2
>
> Discussion here:
>
> https://marc.info/?l=linux-kernel&m=126349766324224&w=2
>
> The discussion led to acquiring the runqueue locks, as there was
> otherwise a need to add code to the scheduler fastpaths.

TL;DR.. that's far too much to trawl through.

> Some architectures are less precise than others in tracking which
> CPUs are running a given process due to ASIDs, though this is
> thought to be a non-problem:
>
> https://marc.info/?l=linux-arch&m=126716090413065&w=2
> https://marc.info/?l=linux-arch&m=126716262815202&w=2
>
> Thoughts?

Yes, there are architectures that only accumulate bits in mm_cpumask(),
with the additional check to see if the remote task belongs to our MM
this should be a non-issue.