Re: Question about x86/mm/gup.c's use of disabled interrupts

From: Jeremy Fitzhardinge
Date: Wed Mar 18 2009 - 19:32:41 EST


Avi Kivity wrote:
Jeremy Fitzhardinge wrote:
Avi Kivity wrote:
Hm, awkward if flush_tlb_others doesn't IPI...


How can it avoid flushing the tlb on cpu [01]? It's it's gup_fast()ing a pte, it may as well load it into the tlb.

xen_flush_tlb_others uses a hypercall rather than an IPI, so none of the logic which depends on there being an IPI will work.

Right, of course, that's what we were talking about. I thought optimizations to avoid IPIs if an mm never visited a cpu.


Simplest fix is to make gup_get_pte() a pvop, but that does seem like putting a red flag in front of an inner-loop hotspot, or something...

The per-cpu tlb-flush exclusion flag might really be the way to go.

I don't see how it will work, without changing Xen to look at the flag?

local_irq_disable() is used here to lock out a remote cpu, I don't see why deferring the flush helps.

Well, no, not deferring. Making xen_flush_tlb_others() spin waiting for "doing_gup" to clear on the target cpu. Or add an explicit notion of a "pte update barrier" rather than implicitly relying on the tlb IPI (which is extremely convenient when available...).

Pick up a percpu flag from all cpus and spin on each? Nasty.

Yeah, not great. Each of those flag fetches is likely to be cold, so a bunch of cache misses. The only mitigating factor is that cross-cpu tlb flushes are expected to be expensive, but some workloads are apparently very sensitive to extra latency in that path. And the hypercall could result in no Xen-level IPIs at all, so it could be very quick by comparison to an IPI-based Linux implementation, in which case the flag polling would be particularly harsh.

Also, the straightforward implementation of "poll until all target cpu's flags are clear" may never make progress, so you'd have to "scan flags, remove busy cpus from set, repeat until all cpus done".

All annoying because this race is pretty unlikely, and it seems a shame to slow down all tlb flushes to deal with it. Some kind of global "doing gup_fast" counter would get flush_tlb_others bypass the check, at the cost of putting a couple of atomic ops around the outside of gup_fast.

You could use the irq enabled flag; it's available and what native spins on (but also means I'll need to add one if I implement this).

Yes, but then we'd end up spuriously polling on cpus which happened to disable interrupts for any reason. And if the vcpu is not running then we could end up polling for a long time. (Same applies for things in gup_fast, but I'm assuming that's a lot less common than disabling interrupts in general).

J

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/