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

[Linus Torvalds]

On Tue, 25 May 2004, David S. Miller wrote:
> 
> Not true on 32-bit Sparc sun4m systems, it's exactly like i386 except
> the hardware is stupid and we only have an atomic swap instruction.
> :-)

Ouch.

Ok. My second suggestion ("don't bother with accessed bit on hw that does 
it on its own") should work fine there too, I guess.

So what I can tell, the fix is really something like this (this does both 
x86 and ppc64 just to show how two different approaches would handle it, 
but I have literally _tested_ neither).

What do people think?

Ben, does this fix your problem? Does my ppc64 assembly code look sane? 
Shamelessly stolen from the function above it, and it seems to compile ;)

		Linus

-----
===== include/asm-generic/pgtable.h 1.3 vs edited =====
--- 1.3/include/asm-generic/pgtable.h	Sun Jan 18 22:35:59 2004
+++ edited/include/asm-generic/pgtable.h	Tue May 25 10:39:38 2004
@@ -10,9 +10,9 @@
  *
  * We hold the mm semaphore for reading and vma->vm_mm->page_table_lock
  */
-#define ptep_establish(__vma, __address, __ptep, __entry)		\
+#define ptep_establish(__vma, __address, __ptep, __entry, __dirty)	\
 do {									\
-	set_pte(__ptep, __entry);					\
+	ptep_update_dirty_accessed(__ptep, __entry, __dirty);		\
 	flush_tlb_page(__vma, __address);				\
 } while (0)
 #endif
===== include/asm-i386/pgtable.h 1.44 vs edited =====
--- 1.44/include/asm-i386/pgtable.h	Sat May 22 14:56:24 2004
+++ edited/include/asm-i386/pgtable.h	Tue May 25 10:39:56 2004
@@ -225,6 +225,12 @@
 static inline void ptep_set_wrprotect(pte_t *ptep)		{ clear_bit(_PAGE_BIT_RW, &ptep->pte_low); }
 static inline void ptep_mkdirty(pte_t *ptep)			{ set_bit(_PAGE_BIT_DIRTY, &ptep->pte_low); }
 
+static inline void ptep_update_dirty_accessed(pte_t *ptep, pte_t entry, int dirty)
+{
+	if (dirty)
+		set_pte(ptep, entry);
+}
+
 /*
  * Macro to mark a page protection value as "uncacheable".  On processors which do not support
  * it, this is a no-op.
===== include/asm-ppc64/pgtable.h 1.33 vs edited =====
--- 1.33/include/asm-ppc64/pgtable.h	Sat May 22 14:56:24 2004
+++ edited/include/asm-ppc64/pgtable.h	Tue May 25 10:45:58 2004
@@ -306,6 +306,21 @@
 	return old;
 }
 
+static inline void ptep_update_dirty_accessed(pte_t *p, pte_t entry, int dirty)
+{
+	unsigned long old, tmp;
+
+	__asm__ __volatile__(
+	"1:	ldarx   %0,0,%3\n\
+	or	%1,%0,%4 \n\
+	stdcx.	%1,0,%3 \n\
+	bne-	1b"
+	: "=&r" (old), "=&r" (tmp), "=m" (*p)
+	: "r" (p), "r" (pte_val(entry)), "m" (*p)
+	: "cc" );
+}
+
+
 /* PTE updating functions */
 extern void hpte_update(pte_t *ptep, unsigned long pte, int wrprot);
 
===== mm/memory.c 1.176 vs edited =====
--- 1.176/mm/memory.c	Sat May 22 14:56:30 2004
+++ edited/mm/memory.c	Tue May 25 10:37:19 2004
@@ -1004,7 +1004,7 @@
 	flush_cache_page(vma, address);
 	entry = maybe_mkwrite(pte_mkdirty(mk_pte(new_page, vma->vm_page_prot)),
 			      vma);
-	ptep_establish(vma, address, page_table, entry);
+	ptep_establish(vma, address, page_table, entry, 1);
 	update_mmu_cache(vma, address, entry);
 }
 
@@ -1056,7 +1056,7 @@
 			flush_cache_page(vma, address);
 			entry = maybe_mkwrite(pte_mkyoung(pte_mkdirty(pte)),
 					      vma);
-			ptep_establish(vma, address, page_table, entry);
+			ptep_establish(vma, address, page_table, entry, 1);
 			update_mmu_cache(vma, address, entry);
 			pte_unmap(page_table);
 			spin_unlock(&mm->page_table_lock);
@@ -1646,7 +1646,7 @@
 		entry = pte_mkdirty(entry);
 	}
 	entry = pte_mkyoung(entry);
-	ptep_establish(vma, address, pte, entry);
+	ptep_establish(vma, address, pte, entry, write_access);
 	update_mmu_cache(vma, address, entry);
 	pte_unmap(pte);
 	spin_unlock(&mm->page_table_lock);
-
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/