Peter Zijlstra wrote:
On Mon, 2008-04-21 at 15:00 +0300, Avi Kivity wrote:
Linus Torvalds wrote:
Finally, I don't think that comment is correct in the first place. It's
not that simple. The thing is, even *with* the memory barrier in place, we
may have:
CPU#1 CPU#2
===== =====
fast_gup:
- read low word
native_set_pte_present:
- set low word to 0
- set high word to new value
- read high word
- set low word to new value
and so you read a low word that is associated with a *different* high
word! Notice?
So trivial memory ordering is _not_ enough.
So I think the code literally needs to be something like this
#ifdef CONFIG_X86_PAE
static inline pte_t native_get_pte(pte_t *ptep)
{
pte_t pte;
retry:
pte.pte_low = ptep->pte_low;
smp_rmb();
pte.pte_high = ptep->pte_high;
smp_rmb();
if (unlikely(pte.pte_low != ptep->pte_low)
goto retry;
return pte;
}
I think this is still broken. Suppose that after reading pte_high
native_set_pte() is called again on another cpu, changing pte_low back
to the original value (but with a different pte_high). You now have
pte_low from second native_set_pte() but pte_high from the first
native_set_pte().
I think the idea was that for user pages we only use set_pte_present()
which does the low=0 thing first.
Doesn't matter. The second native_set_pte() (or set_pte_present())
executes atomically:
fast_gup:
- read low word (l0)
native_set_pte_present:
- set low word to 0
- set high word to new value (h1)
- set low word to new value (l1)
- read high word (h1)
native_set_pte_present:
- set low word to 0
- set high word to new value (h2)
- set low word to new value (l2)
- re-read low word (l2)
If l2 happens to be equal to l0, then the check succeeds and we have a
splintered pte h1:l0.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
