2.3.10 pre5 SMP/vm fixes
Subsystems (4)
PATCH 1/1 2.3.10 pre5 SMP/vm fixes
This set of 5 patches tries to fix SMP races in the 2.3.10-pre5 kernel. I have prepended some comments before each patch.
Kanoj
[email protected]
*****************************************************************
1. Provide smp protection for ia32 tlb flushing (mremap() invokes flush_tlb_all() without holding kernel_lock).
*****************************************************************
include/asm-i386/bitops.h +8 -0
@@ -49,6 +49,14 @@ :"Ir" (nr)); } +extern __inline__void set_bits (int orval,volatile void * addr) +{ + __asm__ __volatile__(LOCK_PREFIX +"orl %1,%0" + :"=m" (ADDR ) + :"r" (orval)); +} +extern __inline__void clear_bit (int nr,volatile void * addr) { __asm__ __volatile__( LOCK_PREFIX
arch/i386/kernel/smp.c +1 -1
@@ -1592,7 +1592,7 @@* locked or. */ - smp_invalidate_needed = cpu_online_map; +set_bits (cpu_online_map, &smp_invalidate_needed);/* * Processors spinning on some lock with IRQs disabled
*****************************************************************
2. Provide protection between file msync and kswapd/shrink_mmap reclaiming a filecache page.
*****************************************************************
mm/filemap.c +12 -6
@@ -1498,16 +1498,18 @@static inline int filemap_sync_pte (pte_t * ptep,struct vm_area_struct *vma,unsigned long address,unsigned int flags) { -pte_t pte = *ptep; +pte_t pte;unsigned long pageaddr;struct page *page;int error; +spin_lock (&vma->vm_mm->page_table_lock); + pte = *ptep;if (!(flags &MS_INVALIDATE )) {if (!pte_present (pte)) -return 0 ; + goto out;if (!pte_dirty (pte)) -return 0 ; + goto out;flush_page_to_ram (pte_page (pte));flush_cache_page (vma, address);set_pte (ptep,pte_mkclean (pte));
@@ -1517,23 +1519,27 @@get_page (page); }else {if (pte_none (pte)) -return 0 ; + goto out;flush_cache_page (vma, address);pte_clear (ptep);flush_tlb_page (vma, address);if (!pte_present (pte)) {swap_free (pte_val (pte)); -return 0 ; + goto out; } pageaddr =pte_page (pte);if (!pte_dirty (pte) || flags ==MS_INVALIDATE ) {page_cache_free (pageaddr); -return 0 ; + goto out; } } +spin_unlock (&vma->vm_mm->page_table_lock); error =filemap_write_page (vma, address - vma->vm_start + vma->vm_offset, pageaddr,1 );page_cache_free (pageaddr);return error; +out: +spin_unlock (&vma->vm_mm->page_table_lock); +return (0 ); } static inline int filemap_sync_pte_range(pmd_t * pmd,
*****************************************************************
3. In mm/memory.c, a new comment claims:
"The adding of pages is protected by the MM semaphore"
which is not quite correct, since swapoff does not hold this semaphore. A patch against this has been posted at
http://humbolt.nl.linux.org/lists/linux-mm/1999-06/msg00075.html
This patch also fixes races between swapoff and fork, exit, mremap and async swap readaheads during page faults.
In the absence of this patch in 2.3, and if we ever want to run swapoff while kswapd is active (ie, kernel_lock is eliminated from either of these paths), the following changes are needed.
*****************************************************************
mm/swapfile.c +7 -4
@@ -171,24 +171,27 @@ {pte_t pte = *dir; +spin_lock (&vma->vm_mm->page_table_lock);if (pte_none (pte)) -return ; + goto out;if (pte_present (pte)) {/* If this entry is swap-cached, then page must already hold the right address for any copies in physical memory */ if (pte_page (pte) != page) -return ; + goto out;/* We will be removing the swap cache in a moment, so... */ set_pte (dir,pte_mkdirty (pte)); -return ; + goto out; }if (pte_val (pte) != entry) -return ; + goto out;set_pte (dir,pte_mkdirty (mk_pte (page, vma->vm_page_prot)));swap_free (entry);get_page (mem_map +MAP_NR (page)); ++vma->vm_mm->rss; +out: +spin_unlock (&vma->vm_mm->page_table_lock); } static inline void unuse_pmd(struct vm_area_struct * vma, pmd_t *dir,
*****************************************************************
4. If we want to eliminate kernel_lock from the fork/dup_mmap and mmap() paths, the following changes are needed to protect against kswapd.
*****************************************************************
mm/memory.c +13 -3
@@ -246,6 +246,7 @@ src_pte =pte_offset (src_pmd, address); dst_pte =pte_offset (dst_pmd, address); +spin_lock (&vma->vm_mm->page_table_lock); do { pte_t pte = *src_pte; unsigned long page_nr;
@@ -277,11 +278,14 @@get_page (mem_map + page_nr); cont_copy_pte_range: address +=PAGE_SIZE ; -if (address >= end) +if (address >= end) { +spin_unlock (&vma->vm_mm->page_table_lock); goto out; + } src_pte++; dst_pte++; }while ((unsigned long )src_pte &PTE_TABLE_MASK ); +spin_unlock (&vma->vm_mm->page_table_lock); cont_copy_pmd_range: src_pmd++; dst_pmd++;
@@ -421,8 +425,11 @@ do { pte_t zero_pte = pte_wrprotect(mk_pte(ZERO_PAGE(address), prot)); - pte_t oldpage = *pte; + pte_t oldpage; + spin_lock(¤t->mm->page_table_lock); + oldpage = *pte; set_pte(pte, zero_pte); + spin_unlock(¤t->mm->page_table_lock); forget_pte(oldpage); address += PAGE_SIZE; pte++;
@@ -489,12 +496,15 @@ end =PMD_SIZE ; do { unsigned long mapnr; - pte_t oldpage = *pte; + pte_t oldpage; + spin_lock(¤t->mm->page_table_lock); + oldpage = *pte; pte_clear(pte); mapnr = MAP_NR(__va(phys_addr)); if (mapnr >= max_mapnr || PageReserved(mem_map+mapnr)) set_pte(pte, mk_pte_phys(phys_addr, prot)); + spin_unlock(¤t->mm->page_table_lock); forget_pte(oldpage); address += PAGE_SIZE; phys_addr += PAGE_SIZE;
*****************************************************************
5. vmtruncate() grabs a spinlock i_shared_lock, then invokes zap_page_range, which might finally go to sleep via calls to zap_pmd_range -> zap_pte_range -> free_pte -> free_page_and_swap_cache -> lock_page. This patch tries to fix that by removing the lock_page from free_page_and_swap_cache(), which lets i_shared_lock stay a spinlock. As far as I can see, the need to have the lock_page in the swap-cache code is to satisfy the validity check PageLocked(page) in remove_inode_page <- remove_from_swap_cache <- delete_from_swap_cache. And the same check in block_flushpage. Note that pages in the swap cache do not have associated buffers, so a block_flushpage is not really needed for these pages. If this patch is taken, the page_table_lock grabbing in zap_pte_range, zeromap_pte_range and remap_pte_range can be done at the zap_page_range, zeromap_pmd_range and remap_pmd_range, although I have not included those changes here.
If there is any more intricate reason for the lock_page's in the swap cache code, a different fix needs to be applied.
*****************************************************************
mm/swap_state.c +2 -10
@@ -42,7 +42,7 @@NULL ,/* get_block */ NULL ,/* readpage */ NULL ,/* writepage */ - block_flushpage,/* flushpage */ +NULL ,/* flushpage */ NULL ,/* truncate */ NULL ,/* permission */ NULL, /* smap */
@@ -214,9 +214,7 @@ page_address(page), page_count(page));#endif PageClearSwapCache (page); -if (inode->i_op->flushpage) - inode->i_op->flushpage (inode, page,0 ); -remove_inode_page (page); +remove_inode_page_nolock (page); }/*
@@ -245,11 +243,7 @@*/ void delete_from_swap_cache (struct page *page) { -lock_page (page); -__delete_from_swap_cache (page); - -UnlockPage (page);page_cache_release (page); }
@@ -265,7 +259,6 @@/* * If we are the only user, then free up the swap cache. */ -lock_page (page);if (PageSwapCache (page) && !is_page_shared (page)) {long entry = page->offset;remove_from_swap_cache (page);
@@ -272,7 +265,6 @@swap_free (entry);page_cache_release (page); } -UnlockPage (page);__free_page (page); }
mm/filemap.c +9 -0
@@ -99,6 +99,15 @@spin_unlock (&pagecache_lock); } +void remove_inode_page_nolock (struct page *page) +{ +spin_lock (&pagecache_lock); +remove_page_from_inode_queue (page); +remove_page_from_hash_queue (page); + page->inode =NULL ; +spin_unlock (&pagecache_lock); +} +void invalidate_inode_pages (struct inode * inode) {struct page ** p;
include/linux/mm.h +1 -0
@@ -344,6 +344,7 @@/* filemap.c */ extern void remove_inode_page (struct page *); +extern void remove_inode_page_nolock (struct page *);extern unsigned long page_unuse (struct page *);extern int shrink_mmap (int ,int );extern void truncate_inode_pages (struct inode *,unsigned long );
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://humbolt.geo.uu.nl/Linux-MM/
Quoted 5 lines · Kanoj Sarcar
*****************************************************************
3. Inmm/memory.c, a new comment claims:
"The adding of pages is protected by the MM semaphore"
which is not quite correct, since swapoff does not hold this semaphore.
The fix is to fix swapoff, not to change the comment.
page_table_lock is _not_ going to be added to this path - I refuse to add locks to common cases just to protect against things that never run in practice.
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://humbolt.geo.uu.nl/Linux-MM/
Quoted 12 lines · Kanoj Sarcar
Quoted 5 lines
*****************************************************************
3. Inmm/memory.c, a new comment claims:
"The adding of pages is protected by the MM semaphore"
which is not quite correct, since swapoff does not hold this semaphore.The fix is to fix swapoff, not to change the comment.
Couldn't agree more ... that is why I sent a patch for this (ie to fix swapoff) on linux-mm, and reiterated the pointer to the patch in this thread. Here it is again:
http://humbolt.nl.linux.org/lists/linux-mm/1999-06/msg00075.html
Let me know if you want me to create it against pre5. Or if you see a problem with it ... we talked about it quite a bit on linux-mm.
Quoted 3 lines
page_table_lockis _not_ going to be added to this path - I refuse to add> locks to common cases just to protect against things that never run in practice.
I assume you are talking about the fault path here ... none of the patches I posted adds page_table_lock to that path, right? Of course without the lock, there might be other problems, I am not sure yet, will let you know if I see something ...
Kanoj
Quoted 2 lines
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://humbolt.geo.uu.nl/Linux-MM/