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

[Peter Zijlstra]

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.