From: Christoph Lameter Updating a page table entry (pte) can be difficult since the MMU may modify the pte concurrently. The current approach taken is to first exchange the pte contents with zero. Clearing the pte by writing zero to it also resets the present bit, which will stop the MMU from modifying the pte and allows the processing of the bits that were set. Then the pte is set to its new value. While the present bit is not set, accesses to the page mapped by the pte will results in page faults, which may install a new pte over the non present entry. In order to avoid that scenario the page_table_lock is held. An access will still result in a page fault but the fault handler will also try to acquire the page_table_lock. The page_table_lock is released after the pte has been setup by the first process. The second process will now acquire the page_table_lock and find that there is already a pte setup for this page and return without having done anything. This means that a useless page fault has been generated. However, most architectures have the capability to atomically exchange the value of the pte. For those the clearing of pte before setting them to a new value is not necessary. The use of atomic exchanges avoids useless page faults. The following patch introduces two new atomic operations ptep_xchg and ptep_cmpxchg that may be provided by an architecture. The fallback in include/asm-generic/pgtable.h is to simulate both operations through the existing ptep_get_and_clear function. So there is essentially no change if atomic operations on ptes have not been defined. Architectures that do not support atomic operations on ptes may continue to use the clearing of a pte. Atomic operations are enabled for i386, ia64 and x86_64 if a suitable CPU is configured in SMP mode. Generic atomic definitions for ptep_xchg and ptep_cmpxchg have been provided based on the existing xchg() and cmpxchg() functions that already work atomically on many platforms. The provided generic atomic functions may be overridden as usual by defining the appropriate__HAVE_ARCH_xxx constant and providing a different implementation. This patch is a piece of my attempt to reduce the use of the page_table_lock in the page fault handler through atomic operations. This is only possible if it can be ensured that a pte is never cleared if the pte is in use even when the page_table_lock is not held. Clearing a pte before setting it to another value could result in a situation in which a fault generated by another cpu could install a pte which is then immediately overwritten by the first CPU setting the pte to a valid value again. The patch is necessary for the other patches removing the use of the page_table_lock to work properly. Signed-off-by: Christoph Lameter Signed-off-by: Andrew Morton --- arch/i386/Kconfig | 5 ++ arch/ia64/Kconfig | 5 ++ arch/x86_64/Kconfig | 5 ++ include/asm-generic/pgtable.h | 86 ++++++++++++++++++++++++++++++++++++++++++ mm/memory.c | 14 ++++-- mm/mprotect.c | 22 +++++----- mm/rmap.c | 22 +++++----- 7 files changed, 133 insertions(+), 26 deletions(-) diff -puN arch/i386/Kconfig~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg arch/i386/Kconfig --- 25/arch/i386/Kconfig~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg Wed Aug 17 14:53:01 2005 +++ 25-akpm/arch/i386/Kconfig Wed Aug 17 15:09:24 2005 @@ -905,6 +905,11 @@ config HAVE_DEC_LOCK depends on (SMP || PREEMPT) && X86_CMPXCHG default y +config ATOMIC_TABLE_OPS + bool + depends on SMP && X86_CMPXCHG && !X86_PAE + default y + # turning this on wastes a bunch of space. # Summit needs it only when NUMA is on config BOOT_IOREMAP diff -puN arch/ia64/Kconfig~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg arch/ia64/Kconfig --- 25/arch/ia64/Kconfig~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg Wed Aug 17 14:53:01 2005 +++ 25-akpm/arch/ia64/Kconfig Wed Aug 17 15:09:24 2005 @@ -297,6 +297,11 @@ config PREEMPT source "mm/Kconfig" +config ATOMIC_TABLE_OPS + bool + depends on SMP + default y + config HAVE_DEC_LOCK bool depends on (SMP || PREEMPT) diff -puN arch/x86_64/Kconfig~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg arch/x86_64/Kconfig --- 25/arch/x86_64/Kconfig~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg Wed Aug 17 14:53:01 2005 +++ 25-akpm/arch/x86_64/Kconfig Wed Aug 17 15:09:24 2005 @@ -217,6 +217,11 @@ config SCHED_SMT cost of slightly increased overhead in some places. If unsure say N here. +config ATOMIC_TABLE_OPS + bool + depends on SMP + default y + source "kernel/Kconfig.preempt" config K8_NUMA diff -puN include/asm-generic/pgtable.h~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg include/asm-generic/pgtable.h --- 25/include/asm-generic/pgtable.h~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg Wed Aug 17 14:53:01 2005 +++ 25-akpm/include/asm-generic/pgtable.h Wed Aug 17 15:09:38 2005 @@ -111,6 +111,92 @@ do { \ }) #endif +#ifdef CONFIG_ATOMIC_TABLE_OPS + +/* + * The architecture does support atomic table operations. + * We may be able to provide atomic ptep_xchg and ptep_cmpxchg using + * cmpxchg and xchg. + */ +#ifndef __HAVE_ARCH_PTEP_XCHG +#define ptep_xchg(__mm, __address, __ptep, __pteval) \ + __pte(xchg(&pte_val(*(__ptep)), pte_val(__pteval))) +#endif + +#ifndef __HAVE_ARCH_PTEP_CMPXCHG +#define ptep_cmpxchg(__mm, __address, __ptep,__oldval,__newval) \ + (cmpxchg(&pte_val(*(__ptep)), \ + pte_val(__oldval), \ + pte_val(__newval) \ + ) == pte_val(__oldval) \ + ) +#endif + +#ifndef __HAVE_ARCH_PTEP_XCHG_FLUSH +#define ptep_xchg_flush(__vma, __address, __ptep, __pteval) \ +({ \ + pte_t __pte = ptep_xchg(__vma, __address, __ptep, __pteval); \ + flush_tlb_page(__vma, __address); \ + __pte; \ +}) +#endif + +#else + +/* + * No support for atomic operations on the page table. + * Exchanging of pte values is done by first swapping zeros into + * a pte and then putting new content into the pte entry. + * However, these functions will generate an empty pte for a + * short time frame. This means that the page_table_lock must be held + * to avoid a page fault that would install a new entry. + */ +#ifndef __HAVE_ARCH_PTEP_XCHG +#define ptep_xchg(__mm, __address, __ptep, __pteval) \ +({ \ + pte_t __pte = ptep_get_and_clear(__mm, __address, __ptep); \ + set_pte_at(__mm, __address, __ptep, __pteval); \ + __pte; \ +}) +#endif + +#ifndef __HAVE_ARCH_PTEP_XCHG_FLUSH +#ifndef __HAVE_ARCH_PTEP_XCHG +#define ptep_xchg_flush(__vma, __address, __ptep, __pteval) \ +({ \ + pte_t __pte = ptep_clear_flush(__vma, __address, __ptep); \ + set_pte_at((__vma)->vm_mm, __address, __ptep, __pteval); \ + __pte; \ +}) +#else +#define ptep_xchg_flush(__vma, __address, __ptep, __pteval) \ +({ \ + pte_t __pte = ptep_xchg((__vma)->vm_mm, __address, __ptep, __pteval);\ + flush_tlb_page(__vma, __address); \ + __pte; \ +}) +#endif +#endif + +/* + * The fallback function for ptep_cmpxchg avoids any real use of cmpxchg + * since cmpxchg may not be available on certain architectures. Instead + * the clearing of a pte is used as a form of locking mechanism. + * This approach will only work if the page_table_lock is held to insure + * that the pte is not populated by a page fault generated on another + * CPU. + */ +#ifndef __HAVE_ARCH_PTEP_CMPXCHG +#define ptep_cmpxchg(__mm, __address, __ptep, __old, __new) \ +({ \ + pte_t prev = ptep_get_and_clear(__mm, __address, __ptep); \ + int r = pte_val(prev) == pte_val(__old); \ + set_pte_at(__mm, __address, __ptep, r ? (__new) : prev); \ + r; \ +}) +#endif +#endif + #ifndef __HAVE_ARCH_PTEP_SET_WRPROTECT static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long address, pte_t *ptep) { diff -puN mm/memory.c~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg mm/memory.c --- 25/mm/memory.c~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg Wed Aug 17 14:53:01 2005 +++ 25-akpm/mm/memory.c Wed Aug 17 15:09:33 2005 @@ -551,15 +551,19 @@ static void zap_pte_range(struct mmu_gat page->index > details->last_index)) continue; } - ptent = ptep_get_and_clear(tlb->mm, addr, pte); - tlb_remove_tlb_entry(tlb, pte, addr); - if (unlikely(!page)) + if (unlikely(!page)) { + ptent = ptep_get_and_clear(tlb->mm, addr, pte); + tlb_remove_tlb_entry(tlb, pte, addr); continue; + } if (unlikely(details) && details->nonlinear_vma && linear_page_index(details->nonlinear_vma, addr) != page->index) - set_pte_at(tlb->mm, addr, pte, - pgoff_to_pte(page->index)); + ptent = ptep_xchg(tlb->mm, addr, pte, + pgoff_to_pte(page->index)); + else + ptent = ptep_get_and_clear(tlb->mm, addr, pte); + tlb_remove_tlb_entry(tlb, pte, addr); if (pte_dirty(ptent)) set_page_dirty(page); if (PageAnon(page)) diff -puN mm/mprotect.c~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg mm/mprotect.c --- 25/mm/mprotect.c~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg Wed Aug 17 14:53:01 2005 +++ 25-akpm/mm/mprotect.c Wed Aug 17 14:53:01 2005 @@ -32,17 +32,19 @@ static void change_pte_range(struct mm_s pte = pte_offset_map(pmd, addr); do { - if (pte_present(*pte)) { - pte_t ptent; + pte_t ptent; +redo: + ptent = *pte; + if (!pte_present(ptent)) + continue; - /* Avoid an SMP race with hardware updated dirty/clean - * bits by wiping the pte and then setting the new pte - * into place. - */ - ptent = pte_modify(ptep_get_and_clear(mm, addr, pte), newprot); - set_pte_at(mm, addr, pte, ptent); - lazy_mmu_prot_update(ptent); - } + /* Deal with a potential SMP race with hardware/arch + * interrupt updating dirty/clean bits through the use + * of ptep_cmpxchg. + */ + if (!ptep_cmpxchg(mm, addr, pte, ptent, pte_modify(ptent, newprot))) + goto redo; + lazy_mmu_prot_update(ptent); } while (pte++, addr += PAGE_SIZE, addr != end); pte_unmap(pte - 1); } diff -puN mm/rmap.c~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg mm/rmap.c --- 25/mm/rmap.c~page-fault-patches-introduce-pte_xchg-and-pte_cmpxchg Wed Aug 17 14:53:01 2005 +++ 25-akpm/mm/rmap.c Wed Aug 17 15:02:57 2005 @@ -539,11 +539,6 @@ static int try_to_unmap_one(struct page /* Nuke the page table entry. */ flush_cache_page(vma, address, page_to_pfn(page)); - pteval = ptep_clear_flush(vma, address, pte); - - /* Move the dirty bit to the physical page now the pte is gone. */ - if (pte_dirty(pteval)) - set_page_dirty(page); if (PageAnon(page)) { swp_entry_t entry = { .val = page->private }; @@ -558,10 +553,15 @@ static int try_to_unmap_one(struct page list_add(&mm->mmlist, &init_mm.mmlist); spin_unlock(&mmlist_lock); } - set_pte_at(mm, address, pte, swp_entry_to_pte(entry)); + pteval = ptep_xchg_flush(vma, address, pte, swp_entry_to_pte(entry)); BUG_ON(pte_file(*pte)); dec_mm_counter(mm, anon_rss); - } + } else + pteval = ptep_clear_flush(vma, address, pte); + + /* Move the dirty bit to the physical page now the pte is gone. */ + if (pte_dirty(pteval)) + set_page_dirty(page); dec_mm_counter(mm, rss); page_remove_rmap(page); @@ -653,15 +653,15 @@ static void try_to_unmap_cluster(unsigne if (ptep_clear_flush_young(vma, address, pte)) continue; - /* Nuke the page table entry. */ flush_cache_page(vma, address, pfn); - pteval = ptep_clear_flush(vma, address, pte); /* If nonlinear, store the file page offset in the pte. */ if (page->index != linear_page_index(vma, address)) - set_pte_at(mm, address, pte, pgoff_to_pte(page->index)); + pteval = ptep_xchg_flush(vma, address, pte, pgoff_to_pte(page->index)); + else + pteval = ptep_clear_flush(vma, address, pte); - /* Move the dirty bit to the physical page now the pte is gone. */ + /* Move the dirty bit to the physical page now that the pte is gone. */ if (pte_dirty(pteval)) set_page_dirty(page); _