On Tue, Jul 25, 2017 at 11:55:10PM +0200, Peter Zijlstra 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: > > > > > > > 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.
It did not get merged into urcu. It is instead used directly by a number of people for a number of concurrent algorithms. > > 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. So non-idle non-nohz CPUs never get IPIed for wakeups of SCHED_OTHER threads? > > > 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 :-) This would be the https://marc.info/?l=linux-kernel&m=126349766324224&w=2 URL below. Which might or might not still be applicable. > > 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. So we re-derive it from first principles instead? ;-) > > 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. Makes sense. Thanx, Paul