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

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

----- On Jul 25, 2017, at 5:55 PM, Peter Zijlstra peterz@infradead.org wrote:

> 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:
[...]
> 
>> 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.

This would implement a MEMBARRIER_CMD_PRIVATE_EXPEDITED (or such) flag
for expedited process-local effect. This differs from the "SHARED" flag,
since the SHARED flag affects threads accessing memory mappings shared
across processes as well.

I wonder if we could create a MEMBARRIER_CMD_SHARED_EXPEDITED behavior
by iterating on all memory mappings mapped into the current process,
and build a cpumask based on the union of all mm masks encountered ?
Then we could send the IPI to all cpus belonging to that cpumask. Or
am I missing something obvious ?

Thanks,

Mathieu


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