Some hash table races (ouch !)

[Benjamin Herrenschmidt]

While looking for a different problems, I spotted a subtle race
that can be caused by ptep_test_and_clar_* removing the HASHPTE
bit from a PTE while leaving this in the hash table. The simple
fix is to not remove the HASHPTE bit since it's perfectly safe
to evict an entry without clearing the HASHPTE bit (the opposite
isn't though).

I also slightly optimized those routines so they don't do the
atomic operation nor the flush when not necessary.

Tests appreciated before I push upstream

Ben.

===== include/asm-ppc64/pgtable.h 1.31 vs edited =====
--- 1.31/include/asm-ppc64/pgtable.h	Fri Feb 27 23:16:07 2004
+++ edited/include/asm-ppc64/pgtable.h	Thu Apr  8 17:30:57 2004
@@ -313,7 +313,9 @@
 {
 	unsigned long old;

-	old = pte_update(ptep, _PAGE_ACCESSED | _PAGE_HPTEFLAGS);
+       	if ((pte_val(*ptep) & (_PAGE_ACCESSED | _PAGE_HASHPTE)) == 0)
+		return 0;
+	old = pte_update(ptep, _PAGE_ACCESSED);
 	if (old & _PAGE_HASHPTE) {
 		hpte_update(ptep, old, 0);
 		flush_tlb_pending();	/* XXX generic code doesn't flush */
@@ -326,12 +328,13 @@
  * moment we always flush but we need to fix hpte_update and test if the
  * optimisation is worth it.
  */
-#if 1
 static inline int ptep_test_and_clear_dirty(pte_t *ptep)
 {
 	unsigned long old;

-	old = pte_update(ptep, _PAGE_DIRTY | _PAGE_HPTEFLAGS);
+       	if ((pte_val(*ptep) & _PAGE_DIRTY) == 0)
+		return 0;
+	old = pte_update(ptep, _PAGE_DIRTY);
 	if (old & _PAGE_HASHPTE)
 		hpte_update(ptep, old, 0);
 	return (old & _PAGE_DIRTY) != 0;
@@ -341,7 +344,9 @@
 {
 	unsigned long old;

-	old = pte_update(ptep, _PAGE_RW | _PAGE_HPTEFLAGS);
+       	if ((pte_val(*ptep) & _PAGE_RW) == 0)
+       		return;
+	old = pte_update(ptep, _PAGE_RW);
 	if (old & _PAGE_HASHPTE)
 		hpte_update(ptep, old, 0);
 }
@@ -358,7 +363,6 @@
 #define ptep_clear_flush_young(__vma, __address, __ptep)		\
 ({									\
 	int __young = ptep_test_and_clear_young(__ptep);		\
-	flush_tlb_page(__vma, __address);				\
 	__young;							\
 })

@@ -369,27 +373,6 @@
 	flush_tlb_page(__vma, __address);				\
 	__dirty;							\
 })
-
-#else
-static inline int ptep_test_and_clear_dirty(pte_t *ptep)
-{
-	unsigned long old;
-
-	old = pte_update(ptep, _PAGE_DIRTY);
-	if ((~old & (_PAGE_HASHPTE | _PAGE_RW | _PAGE_DIRTY)) == 0)
-		hpte_update(ptep, old, 1);
-	return (old & _PAGE_DIRTY) != 0;
-}
-
-static inline void ptep_set_wrprotect(pte_t *ptep)
-{
-	unsigned long old;
-
-	old = pte_update(ptep, _PAGE_RW);
-	if ((~old & (_PAGE_HASHPTE | _PAGE_RW | _PAGE_DIRTY)) == 0)
-		hpte_update(ptep, old, 1);
-}
-#endif

 static inline pte_t ptep_get_and_clear(pte_t *ptep)
 {


** Sent via the linuxppc64-dev mail list. See http://lists.linuxppc.org/