Re: [PATCH] ppc64: Fix possible race with set_pte on a present PTE

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Wed, 2004-05-26 at 10:50, Linus Torvalds wrote: 
> On Wed, 26 May 2004, Benjamin Herrenschmidt wrote:
> > 
> > Heh, I can still send a patch "fixing" it if you want ;)
> 
> If you include a tested version of the ppc64 fix, I'd likely apply that.

Ok, that doesn't work. Trigger BUG_ON in rmap.c:421

I think we are using ptep_establish for more than just setting those
2 bits (like for setting up the new PTE in break_cow and such while
the current implementationL/definition is more like just setting
those bits and nothing else....

If we end up doing more than setting those bits, we need to flush the
PTE completely from the hash etc....

Anyway, here's the non working patch, including the rename.

Ben.

===== include/asm-generic/pgtable.h 1.5 vs edited =====
--- 1.5/include/asm-generic/pgtable.h	2004-05-26 06:04:54 +10:00
+++ edited/include/asm-generic/pgtable.h	2004-05-26 10:58:17 +10:00
@@ -3,8 +3,8 @@
 
 #ifndef __HAVE_ARCH_PTEP_ESTABLISH
 
-#ifndef ptep_update_dirty_accessed
-#define ptep_update_dirty_accessed(__ptep, __entry, __dirty) set_pte(__ptep, __entry)
+#ifndef ptep_set_dirty_accessed
+#define ptep_set_dirty_accessed(__ptep, __entry, __dirty) set_pte(__ptep, __entry)
 #endif
 
 /*
@@ -17,7 +17,7 @@
  */
 #define ptep_establish(__vma, __address, __ptep, __entry, __dirty)	\
 do {									\
-	ptep_update_dirty_accessed(__ptep, __entry, __dirty);		\
+	ptep_set_dirty_accessed(__ptep, __entry, __dirty);		\
 	flush_tlb_page(__vma, __address);				\
 } while (0)
 #endif
===== include/asm-i386/pgtable.h 1.45 vs edited =====
--- 1.45/include/asm-i386/pgtable.h	2004-05-26 06:04:54 +10:00
+++ edited/include/asm-i386/pgtable.h	2004-05-26 10:59:12 +10:00
@@ -325,7 +325,7 @@
  * bit at the same time.
  */
 #define update_mmu_cache(vma,address,pte) do { } while (0)
-#define ptep_update_dirty_accessed(__ptep, __entry, __dirty)	\
+#define ptep_set_dirty_accessed(__ptep, __entry, __dirty)	\
 	do {							\
 		if (__dirty) set_pte(__ptep, __entry);		\
 	} while (0)
===== include/asm-ppc64/pgtable.h 1.33 vs edited =====
--- 1.33/include/asm-ppc64/pgtable.h	2004-05-23 07:56:24 +10:00
+++ edited/include/asm-ppc64/pgtable.h	2004-05-26 13:09:59 +10:00
@@ -306,7 +306,10 @@
 	return old;
 }
 
-/* PTE updating functions */
+/* PTE updating functions, this function puts the PTE in the
+ * batch, doesn't actually triggers the hash flush immediately,
+ * you need to call flush_tlb_pending() to do that.
+ */
 extern void hpte_update(pte_t *ptep, unsigned long pte, int wrprot);
 
 static inline int ptep_test_and_clear_young(pte_t *ptep)
@@ -318,7 +321,7 @@
 	old = pte_update(ptep, _PAGE_ACCESSED);
 	if (old & _PAGE_HASHPTE) {
 		hpte_update(ptep, old, 0);
-		flush_tlb_pending();	/* XXX generic code doesn't flush */
+		flush_tlb_pending();
 	}
 	return (old & _PAGE_ACCESSED) != 0;
 }
@@ -396,10 +399,36 @@
  */
 static inline void set_pte(pte_t *ptep, pte_t pte)
 {
-	if (pte_present(*ptep))
+	if (pte_present(*ptep)) {
 		pte_clear(ptep);
+		flush_tlb_pending();
+	}
 	*ptep = __pte(pte_val(pte)) & ~_PAGE_HPTEFLAGS;
 }
+
+/* Set the dirty and/or accessed bits atomically in a linux PTE, this
+ * function doesn't need to flush the hash entry
+ */
+static inline void ptep_set_dirty_accessed(pte_t *ptep, pte_t entry, int dirty)
+{
+	unsigned long bits = pte_val(entry) & (_PAGE_DIRTY | _PAGE_ACCESSED);
+	unsigned long tmp;
+
+	WARN_ON(!pte_present(*ptep));
+
+	__asm__ __volatile__(
+	       "1:     ldarx   %0,0,%3\n\
+                or      %0,%2,%0\n\
+                stdcx.  %0,0,%3\n\
+                bne-    1b"
+	:"=&r" (tmp), "=m" (*ptep)
+	:"r" (bits), "r" (ptep)
+	:"cc");
+}
+
+/* Make asm-generic/pgtable.h know about it.. */
+#define ptep_set_dirty_accessed ptep_set_dirty_accessed
+
 
 /*
  * Macro to mark a page protection value as "uncacheable".


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>