i386 PAE: avoid atomic for pte_store().

[kib]

Summary

Instead carefully write upper word, and only than the lower word with PG_V.

It provides some measurable system time saving on buildworld:

XX 114.33r  578.88u 101.51s  0.38i  680.77t 233.87I 277X i386-12n-nopae         
XX 116.05r  583.42u 112.93s  0.57i  696.92t 231.46I 301X i386-12n-pae           
XX 115.54r  584.18u 109.33s  0.52i  694.03t 230.25I 302X i386-12n-pae-patched

[markj]

It looks like this is assuming that the old value is not valid, but that's not always true. For example, pmap_promote_pde() simply overwrites the old PDE.

I don't really see how pmap_kextract() is safe wrt torn writes.

[kib]

In D19226#411354, @markj wrote:

It looks like this is assuming that the old value is not valid, but that's not always true. For example, pmap_promote_pde() simply overwrites the old PDE.

I don't really see how pmap_kextract() is safe wrt torn writes.

It could only work for user pmaps, so I can check this. The pmap lock must be owned there.
For promotions, we can safely write zero then write the update, I believe.

In fact half-written ptes open up L1TF CPU bug, but it is so transient that I do not think we should care.

[markj]

In D19226#411357, @kib wrote:

In D19226#411354, @markj wrote:

It looks like this is assuming that the old value is not valid, but that's not always true. For example, pmap_promote_pde() simply overwrites the old PDE.

I don't really see how pmap_kextract() is safe wrt torn writes.

It could only work for user pmaps, so I can check this. The pmap lock must be owned there.
For promotions, we can safely write zero then write the update, I believe.

Perhaps add a second function, pte_store_quick() or so, that requires the previous entry to be invalid, and use that in pmap_enter()? I suspect that most of the performance improvement comes from overwrites of invalid PTEs, i.e., there is not much benefit in avoiding the locked cmpxchg in pmap_promote_pde().

[kib]

Define pte_store_z() to be lockless pte_store() and only use it for zero ptes in usermode pmaps.

[markj]

sys/i386/include/pmap_pae.h

105

Is _z for _zero? I think it might as well be spelled out if so.

Perhaps assert that PG_V is not set in the old entry.

[markj]

sys/i386/i386/pmap.c

4107

Hmm, I think you can unconditionally use pte_store_z() here since the old entry is known to be invalid.

[kib]

sys/i386/i386/pmap.c

4107

But then it returns the question of random pmap_kextract() seeing torn write.
(I think that it would not access the mappings created by pmap_enter_quick_locked, but this is on principle to not allow torn writes for kernel_pmap).

[markj]

sys/i386/i386/pmap.c

4107

Don't you still have this problem in the pmap_enter() change then?

I suspect it is a bug to call pmap_kextract() on an unmapped address, and this reasoning can be used to justify the use of pte_store_z() on invalid entries, even in the kernel pmap. I believe the issue is only with stores that modify a valid PTE or PDE in the kernel pmap.

[kib]

sys/i386/i386/pmap.c

4107

I would have the answer to this question if Peter can reproduce the torn write issue with the patch. It is not even clear to me if e.g. pmap_kenter() or pmap_qenter() can use pte_store_zero().

[kib]

pte_store_zero. Unconditionally use it in pmap_enter_quick_locked.

[markj]

sys/i386/i386/pmap.c

4107

Do either of those routines get used often on i386?

sys/i386/include/pmap_pae.h

104

I think style(9) puts do { on the same line as the #define.

[kib]

sys/i386/i386/pmap.c

4107

pmap_qenter() absolutely, it is used when buffer needs to be mapped, which is always for a buffer with metadata. pmap_kenter() not so much, AFAIR.