Convert do_no_page() to a hook to avoid DFS race

RFC PATCH In review Paul E. McKenney 6 participants 14 messages May 30, 2003 – Aug 10, 2003

Subsystems (16)

Networking Memory management Media (V4L/DVB) VFS +12 more subsystems

lore

Share & tools

lore

Paul E. McKenney

author

2003-05-30 23:41 UTC #0 lore raw

RFC PATCH 1/1 Convert do_no_page() to a hook to avoid DFS race

Rediffed to 2.5.70-mm2.

This patch allows a distributed filesystem to avoid the pagefault/cross-node-invalidate race described in:

	http://marc.theaimsgroup.com/?l=linux-kernel&m=105286345316249&w=2

This patch converts the bulk of do_no_page() into a hook that may be called from the ->nopage vm_operations_struct callout. There is still an inlined do_no_page() wrapper due to the fact that do_anonymous_page() requires that the mm->page_table_lock be held on entry, while the ->nopage callouts require that this lock be dropped.

This patch is untested.

An alternative to this patch includes the nopagedone() patch posted moments ago. hch has also suggested that do_anonymous_page() be converted to a ->nopage callout, but this would require that all of the other ->nopage callouts drop mm->page_table_lock as their first action. If people believe that this is the right thing to do, I will happily produce such a patch.

Thoughts?

Thanx, Paul

arch/ia64/ia32/binfmt_elf32.c +3 -3

diff -urN -X dontdiff linux-2.5.70-mm2/arch/ia64/ia32/binfmt_elf32.c linux-2.5.70-mm2.install_new_page/arch/ia64/ia32/binfmt_elf32.c
@@ -56,13 +56,13 @@
 extern struct page *ia32_shared_page[];
 extern unsigned long *ia32_gdt;
 
-struct page *
-ia32_install_shared_page (struct vm_area_struct *vma, unsigned long address, int no_share)
+int
+ia32_install_shared_page (struct mm_struct *mm, struct vm_area_struct *vma, unsigned long address, int write_access, pmd_t *pmd)
 {
 	struct page *pg = ia32_shared_page[(address - vma->vm_start)/PAGE_SIZE];
 
 	get_page(pg);
-	return pg;
+	return install_new_page(mm, vma, address, write_access, pmd, pg);
 }
 
 static struct vm_operations_struct ia32_shared_page_vm_ops = {

arch/sparc64/mm/hugetlbpage.c +3 -3

diff -urN -X dontdiff linux-2.5.70-mm2/arch/sparc64/mm/hugetlbpage.c linux-2.5.70-mm2.install_new_page/arch/sparc64/mm/hugetlbpage.c
@@ -633,11 +633,11 @@
 	return (int) htlbzone_pages;
 }
 
-static struct page *
-hugetlb_nopage(struct vm_area_struct *vma, unsigned long address, int unused)
+static int
+hugetlb_nopage(struct mm_struct * mm, struct vm_area_struct *vma, unsigned long address, int write_access, pmd_t * pmd)
 {
 	BUG();
-	return NULL;
+	return VM_FAULT_SIGBUS;
 }
 
 static struct vm_operations_struct hugetlb_vm_ops = {

drivers/char/agp/alpha-agp.c +5 -5

diff -urN -X dontdiff linux-2.5.70-mm2/drivers/char/agp/alpha-agp.c linux-2.5.70-mm2.install_new_page/drivers/char/agp/alpha-agp.c
@@ -11,9 +11,9 @@
 
 #include "agp.h"
 
-static struct page *alpha_core_agp_vm_nopage(struct vm_area_struct *vma,
-					     unsigned long address,
-					     int write_access)
+static int alpha_core_agp_vm_nopage(struct mm_struct *mm, struct vm_area_struct *vma,
+				    unsigned long address,
+				    int write_access, pmd_t pmd)
 {
 	alpha_agp_info *agp = agp_bridge->dev_private_data;
 	dma_addr_t dma_addr;

@@ -23,14 +23,14 @@
 	dma_addr = address - vma->vm_start + agp->aperture.bus_base;
 	pa = agp->ops->translate(agp, dma_addr);
 
-	if (pa == (unsigned long)-EINVAL) return NULL;	/* no translation */
+	if (pa == (unsigned long)-EINVAL) return VM_FAULT_SIGBUS; /* no translation */
 	
 	/*
 	 * Get the page, inc the use count, and return it
 	 */
 	page = virt_to_page(__va(pa));
 	get_page(page);
-	return page;
+	return install_new_page(mm, vma, address, write_access, pmd, page);
 }
 
 static struct aper_size_info_fixed alpha_core_agp_sizes[] =

drivers/char/drm/drmP.h +11 ~~-12~~

diff -urN -X dontdiff linux-2.5.70-mm2/drivers/char/drm/drmP.h linux-2.5.70-mm2.install_new_page/drivers/char/drm/drmP.h
@@ -620,18 +620,17 @@
 extern int	     DRM(fasync)(int fd, struct file *filp, int on);
 
 				/* Mapping support (drm_vm.h) */
-extern struct page *DRM(vm_nopage)(struct vm_area_struct *vma,
-				   unsigned long address,
-				   int write_access);
-extern struct page *DRM(vm_shm_nopage)(struct vm_area_struct *vma,
-				       unsigned long address,
-				       int write_access);
-extern struct page *DRM(vm_dma_nopage)(struct vm_area_struct *vma,
-				       unsigned long address,
-				       int write_access);
-extern struct page *DRM(vm_sg_nopage)(struct vm_area_struct *vma,
-				      unsigned long address,
-				      int write_access);
+extern int DRM(vm_nopage)(struct mm_struct *mm, struct vm_area_struct *vma,
+			  unsigned long address, int write_access, pmd_t *pmd);
+extern int DRM(vm_shm_nopage)(struct mm_struct *mm, struct vm_area_struct *vma,
+			      unsigned long address,
+			      int write_access, pmd_t *pmd);
+extern int DRM(vm_dma_nopage)(struct mm_struct *mm, struct vm_area_struct *vma,
+			      unsigned long address,
+			      int write_access, pmd_t *pmd);
+extern int DRM(vm_sg_nopage)(struct mm_struct *mm, struct vm_area_struct *vma,
+			     unsigned long address,
+			     int write_access, pmd_t *pmd);
 extern void	     DRM(vm_open)(struct vm_area_struct *vma);
 extern void	     DRM(vm_close)(struct vm_area_struct *vma);
 extern void	     DRM(vm_shm_close)(struct vm_area_struct *vma);

drivers/char/drm/drm_vm.h +26 ~~-26~~

116 lines not on this page · raw message

drivers/ieee1394/dma.c +8 -9

diff -urN -X dontdiff linux-2.5.70-mm2/drivers/ieee1394/dma.c linux-2.5.70-mm2.install_new_page/drivers/ieee1394/dma.c
@@ -184,28 +184,27 @@
 
 /* nopage() handler for mmap access */
 
-static struct page*
-dma_region_pagefault(struct vm_area_struct *area, unsigned long address, int write_access)
+static int
+dma_region_pagefault(struct mm_struct *mm, struct vm_area_struct *area, unsigned long address, int write_access, pmd_t *pmd)
 {
 	unsigned long offset;
 	unsigned long kernel_virt_addr;
-	struct page *ret = NOPAGE_SIGBUS;
+	struct page *page;
 
 	struct dma_region *dma = (struct dma_region*) area->vm_private_data;
 
 	if(!dma->kvirt)
-		goto out;
+		return VM_FAULT_SIGBUS;
 
 	if( (address < (unsigned long) area->vm_start) ||
 	    (address > (unsigned long) area->vm_start + (PAGE_SIZE * dma->n_pages)) )
-		goto out;
+		return VM_FAULT_SIGBUS;
 
 	offset = address - area->vm_start;
 	kernel_virt_addr = (unsigned long) dma->kvirt + offset;
-	ret = vmalloc_to_page((void*) kernel_virt_addr);
-	get_page(ret);
-out:
-	return ret;
+	page = vmalloc_to_page((void*) kernel_virt_addr);
+	get_page(page);
+	return install_new_page(mm, vma, address, write_access, pmd, page);
 }
 
 static struct vm_operations_struct dma_region_vm_ops = {

drivers/media/video/video-buf.c +6 -6

diff -urN -X dontdiff linux-2.5.70-mm2/drivers/media/video/video-buf.c linux-2.5.70-mm2.install_new_page/drivers/media/video/video-buf.c
@@ -979,21 +979,21 @@
  * now ...).  Bounce buffers don't work very well for the data rates
  * video capture has.
  */
-static struct page*
-videobuf_vm_nopage(struct vm_area_struct *vma, unsigned long vaddr,
-		  int write_access)
+static int
+videobuf_vm_nopage(struct mm_struct *mm, struct vm_area_struct *vma,
+		   unsigned long vaddr, int write_access, pmd_t pmd)
 {
 	struct page *page;
 
 	dprintk(3,"nopage: fault @ %08lx [vma %08lx-%08lx]\n",
 		vaddr,vma->vm_start,vma->vm_end);
         if (vaddr > vma->vm_end)
-		return NOPAGE_SIGBUS;
+		return VM_FAULT_SIGBUS;
 	page = alloc_page(GFP_USER);
 	if (!page)
-		return NOPAGE_OOM;
+		return VM_FAULT_OOM;
 	clear_user_page(page_address(page), vaddr, page);
-	return page;
+	return install_new_page(mm, vma, vaddr, write_access, pmd, page);
 }
 
 static struct vm_operations_struct videobuf_vm_ops =

drivers/scsi/sg.c +6 -6

diff -urN -X dontdiff linux-2.5.70-mm2/drivers/scsi/sg.c linux-2.5.70-mm2.install_new_page/drivers/scsi/sg.c
@@ -1121,21 +1121,21 @@
 	}
 }
 
-static struct page *
-sg_vma_nopage(struct vm_area_struct *vma, unsigned long addr, int unused)
+static int
+sg_vma_nopage(struct mm_struct *mm, struct vm_area_struct *vma, unsigned long addr, int write_access, pmd_t *pmd)
 {
 	Sg_fd *sfp;
-	struct page *page = NOPAGE_SIGBUS;
+	struct page *page = VM_FAULT_SIGBUS;
 	void *page_ptr = NULL;
 	unsigned long offset;
 	Sg_scatter_hold *rsv_schp;
 
 	if ((NULL == vma) || (!(sfp = (Sg_fd *) vma->vm_private_data)))
-		return page;
+		return install_new_page(mm, vma, addr, write_access, pmd, page);
 	rsv_schp = &sfp->reserve;
 	offset = addr - vma->vm_start;
 	if (offset >= rsv_schp->bufflen)
-		return page;
+		return install_new_page(mm, vma, addr, write_access, pmd, page);
 	SCSI_LOG_TIMEOUT(3, printk("sg_vma_nopage: offset=%lu, scatg=%d\n",
 				   offset, rsv_schp->k_use_sg));
 	if (rsv_schp->k_use_sg) {	/* reserve buffer is a scatter gather list */

@@ -1162,7 +1162,7 @@
 		page = virt_to_page(page_ptr);
 		get_page(page);	/* increment page count */
 	}
-	return page;
+	return install_new_page(mm, vma, addr, write_access, pmd, page);
 }
 
 static struct vm_operations_struct sg_mmap_vm_ops = {

drivers/sgi/char/graphics.c +4 -4

diff -urN -X dontdiff linux-2.5.70-mm2/drivers/sgi/char/graphics.c linux-2.5.70-mm2.install_new_page/drivers/sgi/char/graphics.c
@@ -211,9 +211,9 @@
 /* 
  * This is the core of the direct rendering engine.
  */
-struct page *
-sgi_graphics_nopage (struct vm_area_struct *vma, unsigned long address, int
-		     no_share)
+struct int
+sgi_graphics_nopage (struct mm_struct *mm, struct vm_area_struct *vma,
+		     unsigned long address, int write_access, pmd_t *pmdpf)
 {
 	pgd_t *pgd; pmd_t *pmd; pte_t *pte; 
 	int board = GRAPHICS_CARD (vma->vm_dentry->d_inode->i_rdev);

@@ -249,7 +249,7 @@
 	pte = pte_kmap_offset(pmd, address);
 	page = pte_page(*pte);
 	pte_kunmap(pte);
-	return page;
+	return install_new_page(mm, vma, address, write_access, pmdpf, page);
 }
 
 /*

fs/ncpfs/mmap.c +3 -3

diff -urN -X dontdiff linux-2.5.70-mm2/fs/ncpfs/mmap.c linux-2.5.70-mm2.install_new_page/fs/ncpfs/mmap.c
@@ -25,8 +25,8 @@
 /*
  * Fill in the supplied page for mmap
  */
-static struct page* ncp_file_mmap_nopage(struct vm_area_struct *area,
-				     unsigned long address, int write_access)
+static int ncp_file_mmap_nopage(struct mm_struct *mm, struct vm_area_struct *area,
+				unsigned long address, int write_access, pmd_t *pmd)
 {
 	struct file *file = area->vm_file;
 	struct dentry *dentry = file->f_dentry;

@@ -85,7 +85,7 @@
 		memset(pg_addr + already_read, 0, PAGE_SIZE - already_read);
 	flush_dcache_page(page);
 	kunmap(page);
-	return page;
+	return install_new_page(mm, area, address, write_access, pmd, page);
 }
 
 static struct vm_operations_struct ncp_file_mmap =

include/linux/mm.h +5 ~~-10~~

diff -urN -X dontdiff linux-2.5.70-mm2/include/linux/mm.h linux-2.5.70-mm2.install_new_page/include/linux/mm.h
@@ -142,7 +142,7 @@
 struct vm_operations_struct {
 	void (*open)(struct vm_area_struct * area);
 	void (*close)(struct vm_area_struct * area);
-	struct page * (*nopage)(struct vm_area_struct * area, unsigned long address, int unused);
+	int (*nopage)(struct mm_struct * mm, struct vm_area_struct * area, unsigned long address, int write_access, pmd_t *pmd);
 	int (*populate)(struct vm_area_struct * area, unsigned long address, unsigned long len, pgprot_t prot, unsigned long pgoff, int nonblock);
 };

@@ -380,12 +380,6 @@
 }
 
 /*
- * Error return values for the *_nopage functions
- */
-#define NOPAGE_SIGBUS	(NULL)
-#define NOPAGE_OOM	((struct page *) (-1))
-
-/*
  * Different kinds of faults, as returned by handle_mm_fault().
  * Used to decide whether a process gets delivered SIGBUS or
  * just gets major/minor fault counters bumped up.

@@ -402,8 +396,8 @@
 
 extern void show_free_areas(void);
 
-struct page *shmem_nopage(struct vm_area_struct * vma,
-			unsigned long address, int unused);
+int shmem_nopage(struct mm_struct * mm, struct vm_area_struct * vma,
+		 unsigned long address, int write_access, pmd_t * pmd);
 struct file *shmem_file_setup(char * name, loff_t size, unsigned long flags);
 void shmem_lock(struct file * file, int lock);
 int shmem_zero_setup(struct vm_area_struct *);

@@ -421,6 +415,7 @@
 int zeromap_page_range(struct vm_area_struct *vma, unsigned long from,
 			unsigned long size, pgprot_t prot);
 
+extern int install_new_page(struct mm_struct *mm, struct vm_area_struct *vma, unsigned long address, int write_access, pmd_t *pmd, struct page * new_page);
 extern void invalidate_mmap_range(struct address_space *mapping,
 				  loff_t const holebegin,
 				  loff_t const holelen);

@@ -559,7 +554,7 @@
 extern void truncate_inode_pages(struct address_space *, loff_t);
 
 /* generic vm_area_ops exported for stackable file systems */
-extern struct page *filemap_nopage(struct vm_area_struct *, unsigned long, int);
+int filemap_nopage(struct mm_struct *, struct vm_area_struct *, unsigned long, int, pmd_t *);
 
 /* mm/page-writeback.c */
 int write_one_page(struct page *page, int wait);

kernel/ksyms.c +1 -0

diff -urN -X dontdiff linux-2.5.70-mm2/kernel/ksyms.c linux-2.5.70-mm2.install_new_page/kernel/ksyms.c
@@ -116,6 +116,7 @@
 EXPORT_SYMBOL(max_mapnr);
 #endif
 EXPORT_SYMBOL(high_memory);
+EXPORT_SYMBOL(install_new_page);
 EXPORT_SYMBOL(invalidate_mmap_range);
 EXPORT_SYMBOL(vmtruncate);
 EXPORT_SYMBOL(find_vma);

mm/filemap.c +6 -6

diff -urN -X dontdiff linux-2.5.70-mm2/mm/filemap.c linux-2.5.70-mm2.install_new_page/mm/filemap.c
@@ -1013,7 +1013,7 @@
  * it in the page cache, and handles the special cases reasonably without
  * having a lot of duplicated code.
  */
-struct page * filemap_nopage(struct vm_area_struct * area, unsigned long address, int unused)
+int filemap_nopage(struct mm_struct * mm, struct vm_area_struct * area, unsigned long address, int write_access, pmd_t * pmd)
 {
 	int error;
 	struct file *file = area->vm_file;

@@ -1034,7 +1034,7 @@
 	 */
 	size = (inode->i_size + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
 	if ((pgoff >= size) && (area->vm_mm == current->mm))
-		return NULL;
+		return VM_FAULT_SIGBUS;
 
 	/*
 	 * The "size" of the file, as far as mmap is concerned, isn't bigger

@@ -1088,7 +1088,7 @@
 	 * Found the page and have a reference on it.
 	 */
 	mark_page_accessed(page);
-	return page;
+	return install_new_page(mm, area, address, write_access, pmd, page);
 
 no_cached_page:
 	/*

@@ -1111,8 +1111,8 @@
 	 * to schedule I/O.
 	 */
 	if (error == -ENOMEM)
-		return NOPAGE_OOM;
-	return NULL;
+		return VM_FAULT_OOM;
+	return VM_FAULT_SIGBUS;
 
 page_not_uptodate:
 	inc_page_state(pgmajfault);

@@ -1169,7 +1169,7 @@
 	 * mm layer so, possibly freeing the page cache page first.
 	 */
 	page_cache_release(page);
-	return NULL;
+	return VM_FAULT_SIGBUS;
 }
 
 static struct page * filemap_getpage(struct file *file, unsigned long pgoff,

mm/memory.c +19 ~~-25~~

58 lines not on this page · raw message

mm/shmem.c +3 -3

diff -urN -X dontdiff linux-2.5.70-mm2/mm/shmem.c linux-2.5.70-mm2.install_new_page/mm/shmem.c
@@ -936,7 +936,7 @@
 	return error;
 }
 
-struct page *shmem_nopage(struct vm_area_struct *vma, unsigned long address, int unused)
+int shmem_nopage(struct mm_struct *mm, struct vm_area_struct *vma, unsigned long address, int write_access, pmd_t *pmd)
 {
 	struct inode *inode = vma->vm_file->f_dentry->d_inode;
 	struct page *page = NULL;

@@ -949,10 +949,10 @@
 
 	error = shmem_getpage(inode, idx, &page, SGP_CACHE);
 	if (error)
-		return (error == -ENOMEM)? NOPAGE_OOM: NOPAGE_SIGBUS;
+		return (error == -ENOMEM)? VM_FAULT_OOM: VM_FAULT_SIGBUS;
 
 	mark_page_accessed(page);
-	return page;
+	return install_new_page(mm, vma, address, write_access, pmd, page);
 }
 
 static int shmem_populate(struct vm_area_struct *vma,

sound/core/pcm_native.c +11 -6

diff -urN -X dontdiff linux-2.5.70-mm2/sound/core/pcm_native.c linux-2.5.70-mm2.install_new_page/sound/core/pcm_native.c
@@ -60,6 +60,11 @@
 static int snd_pcm_hw_refine_old_user(snd_pcm_substream_t * substream, struct sndrv_pcm_hw_params_old * _oparams);
 static int snd_pcm_hw_params_old_user(snd_pcm_substream_t * substream, struct sndrv_pcm_hw_params_old * _oparams);
 
+#ifndef LINUX_2_2
+#define NOPAGE_OOM VM_FAULT_OOM
+#define NOPAGE_SIGBUS VM_FAULT_SIGBUS
+#endif
+
 /*
  *
  */

@@ -2693,7 +2698,7 @@
 #endif
 
 #ifndef LINUX_2_2
-static struct page * snd_pcm_mmap_status_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
+static int snd_pcm_mmap_status_nopage(struct mm_struct *mm, struct vm_area_struct *area, unsigned long address, int write_access, pmd_t *pmd)
 #else
 static unsigned long snd_pcm_mmap_status_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
 #endif

@@ -2708,7 +2713,7 @@
 	page = virt_to_page(runtime->status);
 	get_page(page);
 #ifndef LINUX_2_2
-	return page;
+	return install_new_page(mm, area, address, write_access, pmd, page);
 #else
 	return page_address(page);
 #endif

@@ -2747,7 +2752,7 @@
 }
 
 #ifndef LINUX_2_2
-static struct page * snd_pcm_mmap_control_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
+static int snd_pcm_mmap_control_nopage(struct mm_struct *mm, struct vm_area_struct *area, unsigned long address, int write_access, pmd_t *pmd)
 #else
 static unsigned long snd_pcm_mmap_control_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
 #endif

@@ -2762,7 +2767,7 @@
 	page = virt_to_page(runtime->control);
 	get_page(page);
 #ifndef LINUX_2_2
-	return page;
+	return install_new_page(mm, area, address, write_access, pmd, page);
 #else
 	return page_address(page);
 #endif

@@ -2813,7 +2818,7 @@
 }
 
 #ifndef LINUX_2_2
-static struct page * snd_pcm_mmap_data_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
+static int snd_pcm_mmap_data_nopage(struct mm_struct *mm, struct vm_area_struct *area, unsigned long address, int write_access, pmd_t *pmd)
 #else
 static unsigned long snd_pcm_mmap_data_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
 #endif

@@ -2848,7 +2853,7 @@
 	}
 	get_page(page);
 #ifndef LINUX_2_2
-	return page;
+	return install_new_page(mm, area, address, write_access, pmd, page);
 #else
 	return page_address(page);
 #endif

sound/oss/emu10k1/audio.c +4 -4

diff -urN -X dontdiff linux-2.5.70-mm2/sound/oss/emu10k1/audio.c linux-2.5.70-mm2.install_new_page/sound/oss/emu10k1/audio.c
@@ -970,7 +970,7 @@
 	return 0;
 }
 
-static struct page *emu10k1_mm_nopage (struct vm_area_struct * vma, unsigned long address, int write_access)
+static int emu10k1_mm_nopage (struct mm_struct * mm, struct vm_area_struct * vma, unsigned long address, int write_access, pmd_t * pmd)
 {
 	struct emu10k1_wavedevice *wave_dev = vma->vm_private_data;
 	struct woinst *woinst = wave_dev->woinst;

@@ -983,8 +983,8 @@
 	DPD(3, "addr: %#lx\n", address);
 
 	if (address > vma->vm_end) {
-		DPF(1, "EXIT, returning NOPAGE_SIGBUS\n");
-		return NOPAGE_SIGBUS; /* Disallow mremap */
+		DPF(1, "EXIT, returning VM_FAULT_SIGBUS\n");
+		return VM_FAULT_SIGBUS; /* Disallow mremap */
 	}
 
 	pgoff = vma->vm_pgoff + ((address - vma->vm_start) >> PAGE_SHIFT);

@@ -1013,7 +1013,7 @@
 	get_page (dmapage);
 
 	DPD(3, "page: %#lx\n", (unsigned long) dmapage);
-	return dmapage;
+	return install_new_page(mm, vma, address, write_access, pmd, dmapage);
 }
 
 struct vm_operations_struct emu10k1_mm_ops = {

sound/oss/via82cxxx_audio.c +8 -8

diff -urN -X dontdiff linux-2.5.70-mm2/sound/oss/via82cxxx_audio.c linux-2.5.70-mm2.install_new_page/sound/oss/via82cxxx_audio.c
@@ -1846,8 +1846,8 @@
 }
 
 
-static struct page * via_mm_nopage (struct vm_area_struct * vma,
-				    unsigned long address, int write_access)
+static int via_mm_nopage (struct mm_struct *mm, struct vm_area_struct * vma,
+			  unsigned long address, int write_access, pmd_t *pmd)
 {
 	struct via_info *card = vma->vm_private_data;
 	struct via_channel *chan = &card->ch_out;

@@ -1863,12 +1863,12 @@
 		 write_access);
 
         if (address > vma->vm_end) {
-		DPRINTK ("EXIT, returning NOPAGE_SIGBUS\n");
-		return NOPAGE_SIGBUS; /* Disallow mremap */
+		DPRINTK ("EXIT, returning VM_FAULT_SIGBUS\n");
+		return VM_FAULT_SIGBUS; /* Disallow mremap */
 	}
         if (!card) {
-		DPRINTK ("EXIT, returning NOPAGE_OOM\n");
-		return NOPAGE_OOM;	/* Nothing allocated */
+		DPRINTK ("EXIT, returning VM_FAULT_OOM\n");
+		return VM_FAULT_OOM;	/* Nothing allocated */
 	}
 
 	pgoff = vma->vm_pgoff + ((address - vma->vm_start) >> PAGE_SHIFT);

@@ -1895,10 +1895,10 @@
 	assert ((((unsigned long)chan->pgtbl[pgoff].cpuaddr) % PAGE_SIZE) == 0);
 
 	dmapage = virt_to_page (chan->pgtbl[pgoff].cpuaddr);
-	DPRINTK ("EXIT, returning page %p for cpuaddr %lXh\n",
+	DPRINTK ("EXIT, installing page %p for cpuaddr %lXh\n",
 		 dmapage, (unsigned long) chan->pgtbl[pgoff].cpuaddr);
 	get_page (dmapage);
-	return dmapage;
+	return install_new_page(mm, vma, address, write_access, pmd, dmapage);
 }

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

Andrew Morton

2003-05-31 00:47 UTC #1 lore raw

Quoted 10 lines · Paul E. McKenney

Rediffed to 2.5.70-mm2.
This patch allows a distributed filesystem to avoid the pagefault/cross-node-invalidate race described in:
	http://marc.theaimsgroup.com/?l=linux-kernel&m=105286345316249&w=2
This patch converts the bulk of do_no_page() into a hook that may be called from the ->nopage vm_operations_struct callout.

Seems reasonable.

Quoted 5 lines

There
is still an inlined do_no_page() wrapper due to the fact that do_anonymous_page() requires that the mm->page_table_lock be held on entry, while the ->nopage callouts require that this lock be dropped.

I sugest you change the ->nopage definition so that page_table_lock is held on entry to ->nopage, and ->nopage must drop it at some point. This gives the nopage implementations some more flexibility and may perhaps eliminate that special case?

Quoted 1 line

This patch is untested.

I don't think there's a lot of point in making changes until the code which requires those changes is accepted into the tree. Otherwise it may be pointless churn, and there's nothing in-tree to exercise the new features.

Quoted 6 lines

An alternative to this patch includes the nopagedone() patch posted moments ago. hch has also suggested that do_anonymous_page() be converted to a ->nopage callout, but this would require that all of the other ->nopage callouts drop mm->page_table_lock as their first action. If people believe that this is the right thing to do, I will happily produce such a patch.

That sounds better to me.

Paul E. McKenney

author

2003-05-31 23:38 UTC #2 lore raw

Quoted 11 lines · Andrew Morton

Quoted 5 lines · Paul E. McKenney
There
is still an inlined do_no_page() wrapper due to the fact that do_anonymous_page() requires that the mm->page_table_lock be held on entry, while the ->nopage callouts require that this lock be dropped.
I sugest you change the ->nopage definition so that page_table_lock is held on entry to ->nopage, and ->nopage must drop it at some point. This gives the nopage implementations some more flexibility and may perhaps eliminate that special case?

Will do!

Quoted 5 lines

Quoted 1 line
This patch is untested.
I don't think there's a lot of point in making changes until the code which requires those changes is accepted into the tree. Otherwise it may be pointless churn, and there's nothing in-tree to exercise the new features.

A GPLed use of these DFS features is expected Real Soon Now...

Thanx, Paul

Christoph Hellwig

2003-08-09 18:50 UTC #3 lore raw

Quoted 5 lines · Paul E. McKenney

Quoted 3 lines
I don't think there's a lot of point in making changes until the code which requires those changes is accepted into the tree. Otherwise it may be pointless churn, and there's nothing in-tree to exercise the new features.
A GPLed use of these DFS features is expected Real Soon Now...

So we get to see all the kernel C++ code from GPRS? [1] Better not, IBM might badly scare customers away if it the same quality as the C glue code layer..

[1] http://oss.software.ibm.com/linux/patches/?patch_id=923

Paul E. McKenney

author

2003-08-10 12:06 UTC #4 lore raw

Quoted 5 lines · Christoph Hellwig

Quoted 4 lines · Paul E. McKenney
Quoted 3 lines
I don't think there's a lot of point in making changes until the code which requires those changes is accepted into the tree. Otherwise it may be pointless churn, and there's nothing in-tree to exercise the new features.

Quoted 7 lines

Quoted 1 line
A GPLed use of these DFS features is expected Real Soon Now...
So we get to see all the kernel C++ code from GPRS? [1] Better not, IBM might badly scare customers away if it the same quality as the C glue code layer..
[1] http://oss.software.ibm.com/linux/patches/?patch_id=923

I will let the GPFS guys worry about that. ;-)

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

Paul E. McKenney

author

2003-06-01 19:21 UTC #5 lore raw

In reply to Andrew Morton

Quoted 9 lines · Andrew Morton

Quoted 6 lines · Paul E. McKenney
An alternative to this patch includes the nopagedone() patch posted moments ago. hch has also suggested that do_anonymous_page() be converted to a ->nopage callout, but this would require that all of the other ->nopage callouts drop mm->page_table_lock as their first action. If people believe that this is the right thing to do, I will happily produce such a patch.
That sounds better to me.

Here is a patch, compiled on i386, untested. I had to put the pte_unmap() as well as the spin_unlock() into each ->nopage function. I would guess that this might be more attractive if combined with a fix for the pagefault-truncate() race. ;-)

Thanx, Paul

arch/i386/mm/hugetlbpage.c +5 -3

diff -urN -X dontdiff linux-2.5.70-mm3/arch/i386/mm/hugetlbpage.c linux-2.5.70-mm3.install_new_page_hch/arch/i386/mm/hugetlbpage.c
@@ -487,11 +487,13 @@
  * hugegpage VMA.  do_page_fault() is supposed to trap this, so BUG is we get
  * this far.
  */
-static struct page *hugetlb_nopage(struct vm_area_struct *vma,
-				unsigned long address, int unused)
+static int hugetlb_nopage(struct mm_struct *mm, struct vm_area_struct *vma,
+			  unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)
 {
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	BUG();
-	return NULL;
+	return VM_FAULT_SIGBUS;
 }
 
 struct vm_operations_struct hugetlb_vm_ops = {

arch/ia64/ia32/binfmt_elf32.c +5 -3

diff -urN -X dontdiff linux-2.5.70-mm3/arch/ia64/ia32/binfmt_elf32.c linux-2.5.70-mm3.install_new_page_hch/arch/ia64/ia32/binfmt_elf32.c
@@ -56,13 +56,15 @@
 extern struct page *ia32_shared_page[];
 extern unsigned long *ia32_gdt;
 
-struct page *
-ia32_install_shared_page (struct vm_area_struct *vma, unsigned long address, int no_share)
+int
+ia32_install_shared_page (struct mm_struct *mm, struct vm_area_struct *vma, unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)
 {
 	struct page *pg = ia32_shared_page[(address - vma->vm_start)/PAGE_SIZE];
 
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	get_page(pg);
-	return pg;
+	return install_new_page(mm, vma, address, write_access, pmd, pg);
 }
 
 static struct vm_operations_struct ia32_shared_page_vm_ops = {

arch/ia64/kernel/perfmon.c +2 -1

diff -urN -X dontdiff linux-2.5.70-mm3/arch/ia64/kernel/perfmon.c linux-2.5.70-mm3.install_new_page_hch/arch/ia64/kernel/perfmon.c
@@ -424,7 +424,8 @@
 static void pfm_vm_close(struct vm_area_struct * area);
 
 static struct vm_operations_struct pfm_vm_ops={
-	.close = pfm_vm_close
+	.close = pfm_vm_close,
+	.nopage = do_anonymous_page
 };
 
 /*

arch/ia64/mm/hugetlbpage.c +4 -2

diff -urN -X dontdiff linux-2.5.70-mm3/arch/ia64/mm/hugetlbpage.c linux-2.5.70-mm3.install_new_page_hch/arch/ia64/mm/hugetlbpage.c
@@ -479,10 +479,12 @@
 	return 1;
 }
 
-static struct page *hugetlb_nopage(struct vm_area_struct * area, unsigned long address, int unused)
+static int hugetlb_nopage(struct mm_struct * mm, struct vm_area_struct * area, unsigned long address, int write_access, pte_t * page_table, pmd_t * pmd)
 {
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	BUG();
-	return NULL;
+	return VM_FAULT_SIGBUS;
 }
 
 struct vm_operations_struct hugetlb_vm_ops = {

arch/sparc64/mm/hugetlbpage.c +5 -3

diff -urN -X dontdiff linux-2.5.70-mm3/arch/sparc64/mm/hugetlbpage.c linux-2.5.70-mm3.install_new_page_hch/arch/sparc64/mm/hugetlbpage.c
@@ -633,11 +633,13 @@
 	return (int) htlbzone_pages;
 }
 
-static struct page *
-hugetlb_nopage(struct vm_area_struct *vma, unsigned long address, int unused)
+static int
+hugetlb_nopage(struct mm_struct * mm, struct vm_area_struct *vma, unsigned long address, int write_access, pte_t *page_table, pmd_t * pmd)
 {
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	BUG();
-	return NULL;
+	return VM_FAULT_SIGBUS;
 }
 
 static struct vm_operations_struct hugetlb_vm_ops = {

drivers/char/agp/alpha-agp.c +7 -5

diff -urN -X dontdiff linux-2.5.70-mm3/drivers/char/agp/alpha-agp.c linux-2.5.70-mm3.install_new_page_hch/drivers/char/agp/alpha-agp.c
@@ -11,26 +11,28 @@
 
 #include "agp.h"
 
-static struct page *alpha_core_agp_vm_nopage(struct vm_area_struct *vma,
-					     unsigned long address,
-					     int write_access)
+static int alpha_core_agp_vm_nopage(struct mm_struct *mm, struct vm_area_struct *vma,
+				    unsigned long address,
+				    int write_access, pte_t *page_table, pmd_t *pmd)
 {
 	alpha_agp_info *agp = agp_bridge->dev_private_data;
 	dma_addr_t dma_addr;
 	unsigned long pa;
 	struct page *page;
 
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	dma_addr = address - vma->vm_start + agp->aperture.bus_base;
 	pa = agp->ops->translate(agp, dma_addr);
 
-	if (pa == (unsigned long)-EINVAL) return NULL;	/* no translation */
+	if (pa == (unsigned long)-EINVAL) return VM_FAULT_SIGBUS; /* no translation */
 	
 	/*
 	 * Get the page, inc the use count, and return it
 	 */
 	page = virt_to_page(__va(pa));
 	get_page(page);
-	return page;
+	return install_new_page(mm, vma, address, write_access, pmd, page);
 }
 
 static struct aper_size_info_fixed alpha_core_agp_sizes[] =

drivers/char/drm/drmP.h +11 ~~-12~~

diff -urN -X dontdiff linux-2.5.70-mm3/drivers/char/drm/drmP.h linux-2.5.70-mm3.install_new_page_hch/drivers/char/drm/drmP.h
@@ -620,18 +620,17 @@
 extern int	     DRM(fasync)(int fd, struct file *filp, int on);
 
 				/* Mapping support (drm_vm.h) */
-extern struct page *DRM(vm_nopage)(struct vm_area_struct *vma,
-				   unsigned long address,
-				   int write_access);
-extern struct page *DRM(vm_shm_nopage)(struct vm_area_struct *vma,
-				       unsigned long address,
-				       int write_access);
-extern struct page *DRM(vm_dma_nopage)(struct vm_area_struct *vma,
-				       unsigned long address,
-				       int write_access);
-extern struct page *DRM(vm_sg_nopage)(struct vm_area_struct *vma,
-				      unsigned long address,
-				      int write_access);
+extern int DRM(vm_nopage)(struct mm_struct *mm, struct vm_area_struct *vma,
+			  unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd);
+extern int DRM(vm_shm_nopage)(struct mm_struct *mm, struct vm_area_struct *vma,
+			      unsigned long address,
+			      int write_access, pte_t *page_table, pmd_t *pmd);
+extern int DRM(vm_dma_nopage)(struct mm_struct *mm, struct vm_area_struct *vma,
+			      unsigned long address,
+			      int write_access, pte_t *page_table, pmd_t *pmd);
+extern int DRM(vm_sg_nopage)(struct mm_struct *mm, struct vm_area_struct *vma,
+			     unsigned long address,
+			     int write_access, pte_t *page_table, pmd_t *pmd);
 extern void	     DRM(vm_open)(struct vm_area_struct *vma);
 extern void	     DRM(vm_close)(struct vm_area_struct *vma);
 extern void	     DRM(vm_shm_close)(struct vm_area_struct *vma);

drivers/char/drm/drm_vm.h +34 ~~-26~~

125 lines not on this page · raw message

drivers/char/ftape/zftape/zftape-init.c +1 -1

diff -urN -X dontdiff linux-2.5.70-mm3/drivers/char/ftape/zftape/zftape-init.c linux-2.5.70-mm3.install_new_page_hch/drivers/char/ftape/zftape/zftape-init.c
@@ -198,7 +198,7 @@
 	sigfillset(&current->blocked);
 	if ((result = ftape_mmap(vma)) >= 0) {
 #ifndef MSYNC_BUG_WAS_FIXED
-		static struct vm_operations_struct dummy = { NULL, };
+		static struct vm_operations_struct dummy = { .nopage = do_anonymous_page, };
 		vma->vm_ops = &dummy;
 #endif
 	}

drivers/ieee1394/dma.c +10 -9

diff -urN -X dontdiff linux-2.5.70-mm3/drivers/ieee1394/dma.c linux-2.5.70-mm3.install_new_page_hch/drivers/ieee1394/dma.c
@@ -184,28 +184,29 @@
 
 /* nopage() handler for mmap access */
 
-static struct page*
-dma_region_pagefault(struct vm_area_struct *area, unsigned long address, int write_access)
+static int
+dma_region_pagefault(struct mm_struct *mm, struct vm_area_struct *area, unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)
 {
 	unsigned long offset;
 	unsigned long kernel_virt_addr;
-	struct page *ret = NOPAGE_SIGBUS;
+	struct page *page;
 
 	struct dma_region *dma = (struct dma_region*) area->vm_private_data;
 
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	if(!dma->kvirt)
-		goto out;
+		return VM_FAULT_SIGBUS;
 
 	if( (address < (unsigned long) area->vm_start) ||
 	    (address > (unsigned long) area->vm_start + (PAGE_SIZE * dma->n_pages)) )
-		goto out;
+		return VM_FAULT_SIGBUS;
 
 	offset = address - area->vm_start;
 	kernel_virt_addr = (unsigned long) dma->kvirt + offset;
-	ret = vmalloc_to_page((void*) kernel_virt_addr);
-	get_page(ret);
-out:
-	return ret;
+	page = vmalloc_to_page((void*) kernel_virt_addr);
+	get_page(page);
+	return install_new_page(mm, vma, address, write_access, pmd, page);
 }
 
 static struct vm_operations_struct dma_region_vm_ops = {

drivers/media/video/video-buf.c +8 -6

diff -urN -X dontdiff linux-2.5.70-mm3/drivers/media/video/video-buf.c linux-2.5.70-mm3.install_new_page_hch/drivers/media/video/video-buf.c
@@ -979,21 +979,23 @@
  * now ...).  Bounce buffers don't work very well for the data rates
  * video capture has.
  */
-static struct page*
-videobuf_vm_nopage(struct vm_area_struct *vma, unsigned long vaddr,
-		  int write_access)
+static int
+videobuf_vm_nopage(struct mm_struct *mm, struct vm_area_struct *vma,
+		   unsigned long vaddr, int write_access, pte_t *page_table, pmd_t *pmd)
 {
 	struct page *page;
 
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	dprintk(3,"nopage: fault @ %08lx [vma %08lx-%08lx]\n",
 		vaddr,vma->vm_start,vma->vm_end);
         if (vaddr > vma->vm_end)
-		return NOPAGE_SIGBUS;
+		return VM_FAULT_SIGBUS;
 	page = alloc_page(GFP_USER);
 	if (!page)
-		return NOPAGE_OOM;
+		return VM_FAULT_OOM;
 	clear_user_page(page_address(page), vaddr, page);
-	return page;
+	return install_new_page(mm, vma, vaddr, write_access, pmd, page);
 }
 
 static struct vm_operations_struct videobuf_vm_ops =

drivers/scsi/sg.c +8 -6

diff -urN -X dontdiff linux-2.5.70-mm3/drivers/scsi/sg.c linux-2.5.70-mm3.install_new_page_hch/drivers/scsi/sg.c
@@ -1121,21 +1121,23 @@
 	}
 }
 
-static struct page *
-sg_vma_nopage(struct vm_area_struct *vma, unsigned long addr, int unused)
+static int
+sg_vma_nopage(struct mm_struct *mm, struct vm_area_struct *vma, unsigned long addr, int write_access, pte_t *page_table, pmd_t *pmd)
 {
 	Sg_fd *sfp;
-	struct page *page = NOPAGE_SIGBUS;
+	struct page *page = VM_FAULT_SIGBUS;
 	void *page_ptr = NULL;
 	unsigned long offset;
 	Sg_scatter_hold *rsv_schp;
 
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	if ((NULL == vma) || (!(sfp = (Sg_fd *) vma->vm_private_data)))
-		return page;
+		return install_new_page(mm, vma, addr, write_access, pmd, page);
 	rsv_schp = &sfp->reserve;
 	offset = addr - vma->vm_start;
 	if (offset >= rsv_schp->bufflen)
-		return page;
+		return install_new_page(mm, vma, addr, write_access, pmd, page);
 	SCSI_LOG_TIMEOUT(3, printk("sg_vma_nopage: offset=%lu, scatg=%d\n",
 				   offset, rsv_schp->k_use_sg));
 	if (rsv_schp->k_use_sg) {	/* reserve buffer is a scatter gather list */

@@ -1162,7 +1164,7 @@
 		page = virt_to_page(page_ptr);
 		get_page(page);	/* increment page count */
 	}
-	return page;
+	return install_new_page(mm, vma, addr, write_access, pmd, page);
 }
 
 static struct vm_operations_struct sg_mmap_vm_ops = {

drivers/sgi/char/graphics.c +6 -4

diff -urN -X dontdiff linux-2.5.70-mm3/drivers/sgi/char/graphics.c linux-2.5.70-mm3.install_new_page_hch/drivers/sgi/char/graphics.c
@@ -211,9 +211,9 @@
 /* 
  * This is the core of the direct rendering engine.
  */
-struct page *
-sgi_graphics_nopage (struct vm_area_struct *vma, unsigned long address, int
-		     no_share)
+struct int
+sgi_graphics_nopage (struct mm_struct *mm, struct vm_area_struct *vma,
+		     unsigned long address, int write_access, pte_t *page_table, pmd_t *pmdpf)
 {
 	pgd_t *pgd; pmd_t *pmd; pte_t *pte; 
 	int board = GRAPHICS_CARD (vma->vm_dentry->d_inode->i_rdev);

@@ -221,6 +221,8 @@
 	unsigned long virt_add, phys_add;
 	struct page * page;
 
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 #ifdef DEBUG
 	printk ("Got a page fault for board %d address=%lx guser=%lx\n", board,
 		address, (unsigned long) cards[board].g_user);

@@ -249,7 +251,7 @@
 	pte = pte_kmap_offset(pmd, address);
 	page = pte_page(*pte);
 	pte_kunmap(pte);
-	return page;
+	return install_new_page(mm, vma, address, write_access, pmdpf, page);
 }
 
 /*

drivers/sgi/char/shmiq.c +3 -3

diff -urN -X dontdiff linux-2.5.70-mm3/drivers/sgi/char/shmiq.c linux-2.5.70-mm3.install_new_page_hch/drivers/sgi/char/shmiq.c
@@ -303,11 +303,11 @@
 }
 
 struct page *
-shmiq_nopage (struct vm_area_struct *vma, unsigned long address,
-              int write_access)
+shmiq_nopage (struct mm_struct *mm, struct vm_area_struct *vma, unsigned long address,
+              int write_access, pte_t *page_table, pmd_t *pmd)
 {
 	/* Do not allow for mremap to expand us */
-	return NULL;
+	return VM_FAULT_SIGBUS;
 }
 
 static struct vm_operations_struct qcntl_mmap = {

fs/ncpfs/mmap.c +5 -3

diff -urN -X dontdiff linux-2.5.70-mm3/fs/ncpfs/mmap.c linux-2.5.70-mm3.install_new_page_hch/fs/ncpfs/mmap.c
@@ -25,8 +25,8 @@
 /*
  * Fill in the supplied page for mmap
  */
-static struct page* ncp_file_mmap_nopage(struct vm_area_struct *area,
-				     unsigned long address, int write_access)
+static int ncp_file_mmap_nopage(struct mm_struct *mm, struct vm_area_struct *area,
+				unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)
 {
 	struct file *file = area->vm_file;
 	struct dentry *dentry = file->f_dentry;

@@ -38,6 +38,8 @@
 	int bufsize;
 	int pos;
 
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	page = alloc_page(GFP_HIGHUSER); /* ncpfs has nothing against high pages
 	           as long as recvmsg and memset works on it */
 	if (!page)

@@ -85,7 +87,7 @@
 		memset(pg_addr + already_read, 0, PAGE_SIZE - already_read);
 	flush_dcache_page(page);
 	kunmap(page);
-	return page;
+	return install_new_page(mm, area, address, write_access, pmd, page);
 }
 
 static struct vm_operations_struct ncp_file_mmap =

include/linux/mm.h +6 ~~-10~~

diff -urN -X dontdiff linux-2.5.70-mm3/include/linux/mm.h linux-2.5.70-mm3.install_new_page_hch/include/linux/mm.h
@@ -142,7 +142,7 @@
 struct vm_operations_struct {
 	void (*open)(struct vm_area_struct * area);
 	void (*close)(struct vm_area_struct * area);
-	struct page * (*nopage)(struct vm_area_struct * area, unsigned long address, int unused);
+	int (*nopage)(struct mm_struct * mm, struct vm_area_struct * area, unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd);
 	int (*populate)(struct vm_area_struct * area, unsigned long address, unsigned long len, pgprot_t prot, unsigned long pgoff, int nonblock);
 };

@@ -380,12 +380,6 @@
 }
 
 /*
- * Error return values for the *_nopage functions
- */
-#define NOPAGE_SIGBUS	(NULL)
-#define NOPAGE_OOM	((struct page *) (-1))
-
-/*
  * Different kinds of faults, as returned by handle_mm_fault().
  * Used to decide whether a process gets delivered SIGBUS or
  * just gets major/minor fault counters bumped up.

@@ -402,8 +396,8 @@
 
 extern void show_free_areas(void);
 
-struct page *shmem_nopage(struct vm_area_struct * vma,
-			unsigned long address, int unused);
+int shmem_nopage(struct mm_struct * mm, struct vm_area_struct * vma,
+		 unsigned long address, int write_access, pte_t * page_table, pmd_t * pmd);
 struct file *shmem_file_setup(char * name, loff_t size, unsigned long flags);
 void shmem_lock(struct file * file, int lock);
 int shmem_zero_setup(struct vm_area_struct *);

@@ -421,6 +415,7 @@
 int zeromap_page_range(struct vm_area_struct *vma, unsigned long from,
 			unsigned long size, pgprot_t prot);
 
+extern int install_new_page(struct mm_struct *mm, struct vm_area_struct *vma, unsigned long address, int write_access, pmd_t *pmd, struct page * new_page);
 extern void invalidate_mmap_range(struct address_space *mapping,
 				  loff_t const holebegin,
 				  loff_t const holelen);

@@ -559,7 +554,8 @@
 extern void truncate_inode_pages(struct address_space *, loff_t);
 
 /* generic vm_area_ops exported for stackable file systems */
-extern struct page *filemap_nopage(struct vm_area_struct *, unsigned long, int);
+int filemap_nopage(struct mm_struct *, struct vm_area_struct *, unsigned long, int, pte_t *, pmd_t *);
+int do_anonymous_page(struct mm_struct *, struct vm_area_struct *, unsigned long, int, pte_t *, pmd_t *);
 
 /* mm/page-writeback.c */
 int write_one_page(struct page *page, int wait);

kernel/ksyms.c +1 -0

diff -urN -X dontdiff linux-2.5.70-mm3/kernel/ksyms.c linux-2.5.70-mm3.install_new_page_hch/kernel/ksyms.c
@@ -116,6 +116,7 @@
 EXPORT_SYMBOL(max_mapnr);
 #endif
 EXPORT_SYMBOL(high_memory);
+EXPORT_SYMBOL(install_new_page);
 EXPORT_SYMBOL(invalidate_mmap_range);
 EXPORT_SYMBOL(vmtruncate);
 EXPORT_SYMBOL(find_vma);

mm/filemap.c +8 -6

diff -urN -X dontdiff linux-2.5.70-mm3/mm/filemap.c linux-2.5.70-mm3.install_new_page_hch/mm/filemap.c
@@ -1013,7 +1013,7 @@
  * it in the page cache, and handles the special cases reasonably without
  * having a lot of duplicated code.
  */
-struct page * filemap_nopage(struct vm_area_struct * area, unsigned long address, int unused)
+int filemap_nopage(struct mm_struct * mm, struct vm_area_struct * area, unsigned long address, int write_access, pte_t *page_table, pmd_t * pmd)
 {
 	int error;
 	struct file *file = area->vm_file;

@@ -1024,6 +1024,8 @@
 	unsigned long size, pgoff, endoff;
 	int did_readahead;
 
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	pgoff = ((address - area->vm_start) >> PAGE_CACHE_SHIFT) + area->vm_pgoff;
 	endoff = ((area->vm_end - area->vm_start) >> PAGE_CACHE_SHIFT) + area->vm_pgoff;

@@ -1034,7 +1036,7 @@
 	 */
 	size = (inode->i_size + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
 	if ((pgoff >= size) && (area->vm_mm == current->mm))
-		return NULL;
+		return VM_FAULT_SIGBUS;
 
 	/*
 	 * The "size" of the file, as far as mmap is concerned, isn't bigger

@@ -1088,7 +1090,7 @@
 	 * Found the page and have a reference on it.
 	 */
 	mark_page_accessed(page);
-	return page;
+	return install_new_page(mm, area, address, write_access, pmd, page);
 
 no_cached_page:
 	/*

@@ -1111,8 +1113,8 @@
 	 * to schedule I/O.
 	 */
 	if (error == -ENOMEM)
-		return NOPAGE_OOM;
-	return NULL;
+		return VM_FAULT_OOM;
+	return VM_FAULT_SIGBUS;
 
 page_not_uptodate:
 	inc_page_state(pgmajfault);

@@ -1169,7 +1171,7 @@
 	 * mm layer so, possibly freeing the page cache page first.
 	 */
 	page_cache_release(page);
-	return NULL;
+	return VM_FAULT_SIGBUS;
 }
 
 static struct page * filemap_getpage(struct file *file, unsigned long pgoff,

mm/memory.c +11 ~~-32~~

75 lines not on this page · raw message

mm/shmem.c +5 -3

diff -urN -X dontdiff linux-2.5.70-mm3/mm/shmem.c linux-2.5.70-mm3.install_new_page_hch/mm/shmem.c
@@ -936,23 +936,25 @@
 	return error;
 }
 
-struct page *shmem_nopage(struct vm_area_struct *vma, unsigned long address, int unused)
+int shmem_nopage(struct mm_struct *mm, struct vm_area_struct *vma, unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)
 {
 	struct inode *inode = vma->vm_file->f_dentry->d_inode;
 	struct page *page = NULL;
 	unsigned long idx;
 	int error;
 
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	idx = (address - vma->vm_start) >> PAGE_SHIFT;
 	idx += vma->vm_pgoff;
 	idx >>= PAGE_CACHE_SHIFT - PAGE_SHIFT;
 
 	error = shmem_getpage(inode, idx, &page, SGP_CACHE);
 	if (error)
-		return (error == -ENOMEM)? NOPAGE_OOM: NOPAGE_SIGBUS;
+		return (error == -ENOMEM)? VM_FAULT_OOM: VM_FAULT_SIGBUS;
 
 	mark_page_accessed(page);
-	return page;
+	return install_new_page(mm, vma, address, write_access, pmd, page);
 }
 
 static int shmem_populate(struct vm_area_struct *vma,

net/packet/af_packet.c +1 -0

diff -urN -X dontdiff linux-2.5.70-mm3/net/packet/af_packet.c linux-2.5.70-mm3.install_new_page_hch/net/packet/af_packet.c
@@ -1523,6 +1523,7 @@
 static struct vm_operations_struct packet_mmap_ops = {
 	.open =	packet_mm_open,
 	.close =packet_mm_close,
+	.nopage = do_anonymous_page,
 };
 
 static void free_pg_vec(unsigned long *pg_vec, unsigned order, unsigned len)

sound/core/pcm_native.c +25 -8

diff -urN -X dontdiff linux-2.5.70-mm3/sound/core/pcm_native.c linux-2.5.70-mm3.install_new_page_hch/sound/core/pcm_native.c
@@ -60,6 +60,11 @@
 static int snd_pcm_hw_refine_old_user(snd_pcm_substream_t * substream, struct sndrv_pcm_hw_params_old * _oparams);
 static int snd_pcm_hw_params_old_user(snd_pcm_substream_t * substream, struct sndrv_pcm_hw_params_old * _oparams);
 
+#ifndef LINUX_2_2
+#define NOPAGE_OOM VM_FAULT_OOM
+#define NOPAGE_SIGBUS VM_FAULT_SIGBUS
+#endif
+
 /*
  *
  */

@@ -2693,7 +2698,7 @@
 #endif
 
 #ifndef LINUX_2_2
-static struct page * snd_pcm_mmap_status_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
+static int snd_pcm_mmap_status_nopage(struct mm_struct *mm, struct vm_area_struct *area, unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)
 #else
 static unsigned long snd_pcm_mmap_status_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
 #endif

@@ -2702,13 +2707,17 @@
 	snd_pcm_runtime_t *runtime;
 	struct page * page;
 	
+#ifndef LINUX_2_2
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
+#endif
 	if (substream == NULL)
 		return NOPAGE_OOM;
 	runtime = substream->runtime;
 	page = virt_to_page(runtime->status);
 	get_page(page);
 #ifndef LINUX_2_2
-	return page;
+	return install_new_page(mm, area, address, write_access, pmd, page);
 #else
 	return page_address(page);
 #endif

@@ -2747,7 +2756,7 @@
 }
 
 #ifndef LINUX_2_2
-static struct page * snd_pcm_mmap_control_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
+static int snd_pcm_mmap_control_nopage(struct mm_struct *mm, struct vm_area_struct *area, unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)
 #else
 static unsigned long snd_pcm_mmap_control_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
 #endif

@@ -2755,14 +2764,18 @@
 	snd_pcm_substream_t *substream = (snd_pcm_substream_t *)area->vm_private_data;
 	snd_pcm_runtime_t *runtime;
 	struct page * page;
-	
+
+#ifndef LINUX_2_2
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
+#endif
 	if (substream == NULL)
 		return NOPAGE_OOM;
 	runtime = substream->runtime;
 	page = virt_to_page(runtime->control);
 	get_page(page);
 #ifndef LINUX_2_2
-	return page;
+	return install_new_page(mm, area, address, write_access, pmd, page);
 #else
 	return page_address(page);
 #endif

@@ -2813,7 +2826,7 @@
 }
 
 #ifndef LINUX_2_2
-static struct page * snd_pcm_mmap_data_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
+static int snd_pcm_mmap_data_nopage(struct mm_struct *mm, struct vm_area_struct *area, unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)
 #else
 static unsigned long snd_pcm_mmap_data_nopage(struct vm_area_struct *area, unsigned long address, int no_share)
 #endif

@@ -2824,7 +2837,11 @@
 	struct page * page;
 	void *vaddr;
 	size_t dma_bytes;
-	
+
+#ifndef LINUX_2_2
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
+#endif
 	if (substream == NULL)
 		return NOPAGE_OOM;
 	runtime = substream->runtime;

@@ -2848,7 +2865,7 @@
 	}
 	get_page(page);
 #ifndef LINUX_2_2
-	return page;
+	return install_new_page(mm, area, address, write_access, pmd, page);
 #else
 	return page_address(page);
 #endif

sound/oss/emu10k1/audio.c +6 -4

diff -urN -X dontdiff linux-2.5.70-mm3/sound/oss/emu10k1/audio.c linux-2.5.70-mm3.install_new_page_hch/sound/oss/emu10k1/audio.c
@@ -970,7 +970,7 @@
 	return 0;
 }
 
-static struct page *emu10k1_mm_nopage (struct vm_area_struct * vma, unsigned long address, int write_access)
+static int emu10k1_mm_nopage (struct mm_struct * mm, struct vm_area_struct * vma, unsigned long address, int write_access, pte_t * page_table, pmd_t * pmd)
 {
 	struct emu10k1_wavedevice *wave_dev = vma->vm_private_data;
 	struct woinst *woinst = wave_dev->woinst;

@@ -979,12 +979,14 @@
 	unsigned long pgoff;
 	int rd, wr;
 
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	DPF(3, "emu10k1_mm_nopage()\n");
 	DPD(3, "addr: %#lx\n", address);
 
 	if (address > vma->vm_end) {
-		DPF(1, "EXIT, returning NOPAGE_SIGBUS\n");
-		return NOPAGE_SIGBUS; /* Disallow mremap */
+		DPF(1, "EXIT, returning VM_FAULT_SIGBUS\n");
+		return VM_FAULT_SIGBUS; /* Disallow mremap */
 	}
 
 	pgoff = vma->vm_pgoff + ((address - vma->vm_start) >> PAGE_SHIFT);

@@ -1013,7 +1015,7 @@
 	get_page (dmapage);
 
 	DPD(3, "page: %#lx\n", (unsigned long) dmapage);
-	return dmapage;
+	return install_new_page(mm, vma, address, write_access, pmd, dmapage);
 }
 
 struct vm_operations_struct emu10k1_mm_ops = {

sound/oss/via82cxxx_audio.c +10 -8

diff -urN -X dontdiff linux-2.5.70-mm3/sound/oss/via82cxxx_audio.c linux-2.5.70-mm3.install_new_page_hch/sound/oss/via82cxxx_audio.c
@@ -1846,8 +1846,8 @@
 }
 
 
-static struct page * via_mm_nopage (struct vm_area_struct * vma,
-				    unsigned long address, int write_access)
+static int via_mm_nopage (struct mm_struct *mm, struct vm_area_struct * vma,
+			  unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)
 {
 	struct via_info *card = vma->vm_private_data;
 	struct via_channel *chan = &card->ch_out;

@@ -1855,6 +1855,8 @@
 	unsigned long pgoff;
 	int rd, wr;
 
+	pte_unmap(page_table);
+	spin_unlock(&mm->page_table_lock);
 	DPRINTK ("ENTER, start %lXh, ofs %lXh, pgoff %ld, addr %lXh, wr %d\n",
 		 vma->vm_start,
 		 address - vma->vm_start,

@@ -1863,12 +1865,12 @@
 		 write_access);
 
         if (address > vma->vm_end) {
-		DPRINTK ("EXIT, returning NOPAGE_SIGBUS\n");
-		return NOPAGE_SIGBUS; /* Disallow mremap */
+		DPRINTK ("EXIT, returning VM_FAULT_SIGBUS\n");
+		return VM_FAULT_SIGBUS; /* Disallow mremap */
 	}
         if (!card) {
-		DPRINTK ("EXIT, returning NOPAGE_OOM\n");
-		return NOPAGE_OOM;	/* Nothing allocated */
+		DPRINTK ("EXIT, returning VM_FAULT_OOM\n");
+		return VM_FAULT_OOM;	/* Nothing allocated */
 	}
 
 	pgoff = vma->vm_pgoff + ((address - vma->vm_start) >> PAGE_SHIFT);

@@ -1895,10 +1897,10 @@
 	assert ((((unsigned long)chan->pgtbl[pgoff].cpuaddr) % PAGE_SIZE) == 0);
 
 	dmapage = virt_to_page (chan->pgtbl[pgoff].cpuaddr);
-	DPRINTK ("EXIT, returning page %p for cpuaddr %lXh\n",
+	DPRINTK ("EXIT, installing page %p for cpuaddr %lXh\n",
 		 dmapage, (unsigned long) chan->pgtbl[pgoff].cpuaddr);
 	get_page (dmapage);
-	return dmapage;
+	return install_new_page(mm, vma, address, write_access, pmd, dmapage);
 }

Ingo Oeser

2003-05-31 08:46 UTC #6 lore raw

Always passing mm and vma down (was: [RFC][PATCH] Convert do_no_page() to a hook to avoid DFS race)

In reply to Paul E. McKenney

Hi there,

arch/ia64/ia32/binfmt_elf32.c line 66

Quoted 11 lines · Paul E. McKenney

arch/ia64/ia32/binfmt_elf32.c +3 -3

-struct page *
-ia32_install_shared_page (struct vm_area_struct *vma, unsigned long address, int no_share)
+int
+ia32_install_shared_page (struct mm_struct *mm, struct vm_area_struct *vma, unsigned long address, int write_access, pmd_t *pmd)
 {
 	struct page *pg = ia32_shared_page[(address - vma->vm_start)/PAGE_SIZE];
 
 	get_page(pg);
-	return pg;
+	return install_new_page(mm, vma, address, write_access, pmd, pg);
 }

Why do we always pass mm and vma down, even if vma->vm_mm contains the mm, where the vma belongs to? Is the connection between a vma and its mm also protected by the mmap_sem?

Is this really necessary or an oversight and we waste a lot of stack in a lot of places?

If we just need it for accounting: We need current->mm, if we need it to locate the next vma relatively to this vma, vma->vm_mm is the one.

Puzzled

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

Paul E. McKenney

author

2003-05-31 23:35 UTC #7 lore raw

Re: Always passing mm and vma down (was: [RFC][PATCH] Convert do_no_page() to a hook to avoid DFS race)

Quoted 10 lines · Ingo Oeser

arch/ia64/ia32/binfmt_elf32.c line 60
Quoted 4 lines · Paul E. McKenney
arch/ia64/ia32/binfmt_elf32.c +2 -2
-struct page *
-ia32_install_shared_page (struct vm_area_struct *vma, unsigned long address, int no_share)
+int
+ia32_install_shared_page (struct mm_struct *mm, struct vm_area_struct *vma, unsigned long address, int write_access, pmd_t *pmd)
Why do we always pass mm and vma down, even if vma->vm_mm contains the mm, where the vma belongs to? Is the connection between a vma and its mm also protected by the mmap_sem?

Quoted 6 lines

Is this really necessary or an oversight and we waste a lot of stack in a lot of places?
If we just need it for accounting: We need current->mm, if we need it to locate the next vma relatively to this vma, vma->vm_mm is the one.

Interesting point. The original do_no_page() API does this as well:

static int
do_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)

As does do_anonymous_page(). I assumed that there were corner cases where this one-to-one correspondence did not exist, but must confess that I did not go looking for them.

Or is this a performance issue, avoiding a dereference and possible cache miss?

Thanx, Paul

Andrea Arcangeli

2003-06-01 12:16 UTC #8 lore raw

Re: Always passing mm and vma down (was: [RFC][PATCH] Convert do_no_page() to a hook to avoid DFS race)

Hi Paul,

Quoted 25 lines · Paul E. McKenney

Quoted 10 lines · Ingo Oeser
Review
Quoted 4 lines · Paul E. McKenney
(quoted diff) +2 -2
-struct page *
-ia32_install_shared_page (struct vm_area_struct *vma, unsigned long address, int no_share)
+int
+ia32_install_shared_page (struct mm_struct *mm, struct vm_area_struct *vma, unsigned long address, int write_access, pmd_t *pmd)
Why do we always pass mm and vma down, even if vma->vm_mm contains the mm, where the vma belongs to? Is the connection between a vma and its mm also protected by the mmap_sem?
Quoted 6 lines
Is this really necessary or an oversight and we waste a lot of stack in a lot of places?
If we just need it for accounting: We need current->mm, if we need it to locate the next vma relatively to this vma, vma->vm_mm is the one.
Interesting point. The original do_no_page() API does this as well:
static int
do_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)

Quoted 6 lines

As does do_anonymous_page(). I assumed that there were corner cases where this one-to-one correspondence did not exist, but must confess that I did not go looking for them.
Or is this a performance issue, avoiding a dereference and possible cache miss?

it's a performance microoptimization issue (the stack memory overhead is not significant), each new parameter to those calls will slowdown the kernel, especially with the x86 calling convetions it will hurt more than with other archs. Jeff did something similar for UML (dunno if he also can use vma->vm_mm, anyways I #ifdeffed it out enterely for non UML compiles for exact this reason that I can't slowdown the whole production host kernel just for the uml compile) then there's also the additional extern call. So basically the patch would hurt until there are active users.

But the real question here I guess is: why should a distributed filesystem need to install the pte by itself?

The memory coherency with distributed shared memory (i.e. MAP_SHARED against truncate with MAP_SHARED and truncate running on different boxes on a DFS) is a generic problem (most of the time using the same algorithm too), as such I believe the infrastructure and logics to keep it coherent could make sense to be a common code functionalty.

And w/o proper high level locking, the non distributed filesystems will corrupt the VM too with truncate against nopage. I already fixed this in my tree. (see the attachment) So I wonder if the fixes could be shared. I mean, you definitely need my fixes even when using the DFS on a isolated box, and if you don't need them while using the fs locally, it means we're duplicating effort somehow.

Since I don't see the users of the new hook, it's a bit hard to judje if the duplication is legitimate or not. So overall I'd agree with Andrew that to judje the patch it'd make sense to see (or know more) about the users of the hook too.

as for the anon memory, yes, it's probably more efficient and cleaner to have it be a nopage callback too, the double branch is probably more costly than the duplicated unlock anyways. However it has nothing to do with these issues, so I recommend to keep it separated from the DFS patches (for readibility). (it can't have anything to do with DFS since that memory is anon local to the box [if not using openmosix but that's a different issue ;) ])

thanks,
Andrea

fs/inode.c +1 -0

diff -urNp --exclude CVS --exclude BitKeeper xx-ref/fs/inode.c xx/fs/inode.c
@@ -147,6 +147,7 @@ void inode_init_once(struct inode *inode
 	INIT_LIST_HEAD(&inode->i_data.clean_pages);
 	INIT_LIST_HEAD(&inode->i_data.dirty_pages);
 	INIT_LIST_HEAD(&inode->i_data.locked_pages);
+	frlock_init(&inode->i_data.truncate_lock);
 	INIT_LIST_HEAD(&inode->i_dentry);
 	INIT_LIST_HEAD(&inode->i_dirty_buffers);
 	INIT_LIST_HEAD(&inode->i_dirty_data_buffers);

include/linux/fs.h +2 -0

diff -urNp --exclude CVS --exclude BitKeeper xx-ref/include/linux/fs.h xx/include/linux/fs.h
@@ -21,6 +21,7 @@
 #include <linux/cache.h>
 #include <linux/stddef.h>
 #include <linux/string.h>
+#include <linux/frlock.h>
 
 #include <asm/atomic.h>
 #include <asm/bitops.h>

@@ -422,6 +423,7 @@ struct address_space {
 	struct vm_area_struct	*i_mmap;	/* list of private mappings */
 	struct vm_area_struct	*i_mmap_shared; /* list of shared mappings */
 	spinlock_t		i_shared_lock;  /* and spinlock protecting it */
+	frlock_t		truncate_lock; /* serialize ->nopage against truncate */
 	int			gfp_mask;	/* how to allocate the pages */
 };

mm/memory.c +25 -0

diff -urNp --exclude CVS --exclude BitKeeper xx-ref/mm/memory.c xx/mm/memory.c
@@ -1127,6 +1127,7 @@ int vmtruncate(struct inode * inode, lof
 	if (inode->i_size < offset)
 		goto do_expand;
 	i_size_write(inode, offset);
+	fr_write_lock(&mapping->truncate_lock);
 	spin_lock(&mapping->i_shared_lock);
 	if (!mapping->i_mmap && !mapping->i_mmap_shared)
 		goto out_unlock;

@@ -1140,6 +1141,7 @@ int vmtruncate(struct inode * inode, lof
 out_unlock:
 	spin_unlock(&mapping->i_shared_lock);
 	truncate_inode_pages(mapping, offset);
+	fr_write_unlock(&mapping->truncate_lock);
 	goto out_truncate;
 
 do_expand:

@@ -1335,12 +1337,19 @@ static int do_no_page(struct mm_struct *
 {
 	struct page * new_page;
 	pte_t entry;
+	unsigned truncate_sequence;
+	struct file *file;
+	struct address_space *mapping;
 
 	if (!vma->vm_ops || !vma->vm_ops->nopage)
 		return do_anonymous_page(mm, vma, page_table, pmd, write_access, address);
 	spin_unlock(&mm->page_table_lock);
 	pte_kunmap(page_table);
 
+	file = vma->vm_file;
+	mapping = file->f_dentry->d_inode->i_mapping;
+	truncate_sequence = fr_read_begin(&mapping->truncate_lock);
+
 	new_page = vma->vm_ops->nopage(vma, address & PAGE_MASK, 0);
 
 	if (new_page == NULL)	/* no page was available -- SIGBUS */

@@ -1366,6 +1375,21 @@ static int do_no_page(struct mm_struct *
 
 	page_table = pte_offset_atomic(pmd, address);
 	spin_lock(&mm->page_table_lock);
+	if (unlikely(truncate_sequence != fr_read_end(&mapping->truncate_lock))) {
+		struct inode *inode;
+
+		spin_unlock(&mm->page_table_lock);
+
+		/*
+		 * Don't worthless loop here forever overloading the cpu
+		 * until the truncate has completed.
+		 */
+		inode = mapping->host;
+		down(&inode->i_sem);
+		up(&inode->i_sem);
+
+		goto retry;
+	}
 
 	/*
 	 * This silly early PAGE_DIRTY setting removes a race

@@ -1388,6 +1412,7 @@ static int do_no_page(struct mm_struct *
 		set_pte(page_table, entry);
 	} else {
 		spin_unlock(&mm->page_table_lock);
+	retry:
 		pte_kunmap(page_table);
 		/* One of our sibling threads was faster, back out. */
 		page_cache_release(new_page);

mm/vmscan.c +2 -3

diff -urNp --exclude CVS --exclude BitKeeper xx-ref/mm/vmscan.c xx/mm/vmscan.c
@@ -165,11 +165,10 @@ drop_pte:
 		goto drop_pte;
 
 	/*
-	 * Anonymous buffercache pages can be left behind by
+	 * Anonymous buffercache pages can't be left behind by
 	 * concurrent truncate and pagefault.
 	 */
-	if (page->buffers)
-		goto preserve;
+	BUG_ON(page->buffers);
 
 	/*
 	 * This is a dirty, swappable page.  First of all,

Paul E. McKenney

author

2003-06-01 19:47 UTC #9 lore raw

Re: Always passing mm and vma down (was: [RFC][PATCH] Convert do_no_page() to a hook to avoid DFS race)

Quoted 1 line · Andrea Arcangeli

Hi Paul,

Hello, Andrea!

Quoted 1 line · Paul E. McKenney

[ . . . ]

Quoted 24 lines

Quoted 7 lines
Interesting point. The original do_no_page() API does this as well:
static int
do_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
unsigned long address, int write_access, pte_t *page_table, pmd_t *pmd)
Quoted 6 lines
As does do_anonymous_page(). I assumed that there were corner cases where this one-to-one correspondence did not exist, but must confess that I did not go looking for them.
Or is this a performance issue, avoiding a dereference and possible cache miss?
it's a performance microoptimization issue (the stack memory overhead is not significant), each new parameter to those calls will slowdown the kernel, especially with the x86 calling convetions it will hurt more than with other archs. Jeff did something similar for UML (dunno if he also can use vma->vm_mm, anyways I #ifdeffed it out enterely for non UML compiles for exact this reason that I can't slowdown the whole production host kernel just for the uml compile) then there's also the additional extern call. So basically the patch would hurt until there are active users.

Quoted 2 lines

But the real question here I guess is: why should a distributed filesystem need to install the pte by itself?

The immediate motivation is to avoid the race with zap_page_range() when another node writes to the corresponding portion of the file, similar to the situation with vmtruncate(). The thought was to leverage locking within the distributed filesystem, but if the race is solved locally, then, as you say, perhaps this is not necessary.

Quoted 5 lines

The memory coherency with distributed shared memory (i.e. MAP_SHARED against truncate with MAP_SHARED and truncate running on different boxes on a DFS) is a generic problem (most of the time using the same algorithm too), as such I believe the infrastructure and logics to keep it coherent could make sense to be a common code functionalty.

This sounds good to me, though am checking with some DFS people.

Quoted 6 lines

And w/o proper high level locking, the non distributed filesystems will corrupt the VM too with truncate against nopage. I already fixed this in my tree. (see the attachment) So I wonder if the fixes could be shared. I mean, you definitely need my fixes even when using the DFS on a isolated box, and if you don't need them while using the fs locally, it means we're duplicating effort somehow.

True -- my patches simply provided hooks to allow DFSs and local filesystems to fix the problem.

So, the idea is for the DFS to hold a fr_write_lock on the truncate_lock across the invalidate_mmap_range() call, thus preventing the PTEs from any racing pagefaults from being installed? This seems plausible at first glance, but need to stare at it some more. This might permit the current do_no_page(), do_anonymous_page(), and ->nopage APIs to be used, but again, need to stare at it some more.

(If I am not too confused, fr_write_lock() became
write_seqlock() in the 2.5 tree...)

Quoted 4 lines

Since I don't see the users of the new hook, it's a bit hard to judje if the duplication is legitimate or not. So overall I'd agree with Andrew that to judje the patch it'd make sense to see (or know more) about the users of the hook too.

A simple change that takes care of all the cases certainly does seem better than a more complex change that only takes care of distributed filesystems!

Quoted 7 lines

as for the anon memory, yes, it's probably more efficient and cleaner to have it be a nopage callback too, the double branch is probably more costly than the duplicated unlock anyways. However it has nothing to do with these issues, so I recommend to keep it separated from the DFS patches (for readibility). (it can't have anything to do with DFS since that memory is anon local to the box [if not using openmosix but that's a different issue ;) ])

Well, we do seem to have a number of ways of attacking the problem, so I have hope that one of them will do the trick. ;-)

Thanx, Paul

Arjan van de Ven

2003-06-02 08:19 UTC #10 lore raw

Re: Always passing mm and vma down (was: [RFC][PATCH] Convert do_no_page() to a hook to avoid DFS race)

Quoted 6 lines · Paul E. McKenney

The immediate motivation is to avoid the race with zap_page_range() when another node writes to the corresponding portion of the file, similar to the situation with vmtruncate(). The thought was to leverage locking within the distributed filesystem, but if the race is solved locally, then, as you say, perhaps this is not necessary.

is said distributed filesystem open source by chance ?

Paul E. McKenney

author

2003-06-02 12:59 UTC #11 lore raw

Re: Always passing mm and vma down (was: [RFC][PATCH] Convert do_no_page() to a hook to avoid DFS race)

Quoted 9 lines · Arjan van de Ven

Quoted 6 lines · Paul E. McKenney
The immediate motivation is to avoid the race with zap_page_range() when another node writes to the corresponding portion of the file, similar to the situation with vmtruncate(). The thought was to leverage locking within the distributed filesystem, but if the race is solved locally, then, as you say, perhaps this is not necessary.
is said distributed filesystem open source by chance ?

At least one soon will be.

Thanx, Paul

Andrea Arcangeli

2003-06-04 10:24 UTC #12 lore raw

Re: Always passing mm and vma down (was: [RFC][PATCH] Convert do_no_page() to a hook to avoid DFS race)

In reply to Paul E. McKenney

Quoted 6 lines · Paul E. McKenney

The immediate motivation is to avoid the race with zap_page_range() when another node writes to the corresponding portion of the file, similar to the situation with vmtruncate(). The thought was to leverage locking within the distributed filesystem, but if the race is solved locally, then, as you say, perhaps this is not necessary.

exactly, this was my idea. Since we've the same race locally even on ext2, maybe it worth to share the fix for all the fs somehow, the problem sounds the same. You may still need callbacks to get right the distributed fs though. still I was just wondering if the conceptual fix could live in the highlevel rather than replicating it in the lowlevel.

Quoted 1 line

This sounds good to me, though am checking with some DFS people.

cool thanks!

Quoted 10 lines

Quoted 6 lines
And w/o proper high level locking, the non distributed filesystems will corrupt the VM too with truncate against nopage. I already fixed this in my tree. (see the attachment) So I wonder if the fixes could be shared. I mean, you definitely need my fixes even when using the DFS on a isolated box, and if you don't need them while using the fs locally, it means we're duplicating effort somehow.
True -- my patches simply provided hooks to allow DFSs and local filesystems to fix the problem.

Quoted 7 lines

So, the idea is for the DFS to hold a fr_write_lock on the truncate_lock across the invalidate_mmap_range() call, thus preventing the PTEs from any racing pagefaults from being installed? This seems plausible at first glance, but need to stare at it some more. This might permit the current do_no_page(), do_anonymous_page(), and ->nopage APIs to be used, but again, need to stare at it some more.

btw, we can discuss this some more next month at OLS too, if we didn't clear all the issues first.

Quoted 2 lines

(If I am not too confused, fr_write_lock() became
write_seqlock() in the 2.5 tree...)

exactly, I didn't rename it yet in 2.4 since it would provide no runtime benefit, but it is exactly the same thing ;).

Quoted 8 lines

Quoted 4 lines
Since I don't see the users of the new hook, it's a bit hard to judje if the duplication is legitimate or not. So overall I'd agree with Andrew that to judje the patch it'd make sense to see (or know more) about the users of the hook too.
A simple change that takes care of all the cases certainly does seem better than a more complex change that only takes care of distributed filesystems!

agreed.

thanks,

Andrea

Paul E. McKenney

author

2003-06-07 16:16 UTC #13 lore raw

Re: Always passing mm and vma down (was: [RFC][PATCH] Convert do_no_page() to a hook to avoid DFS race)

Quoted 8 lines · Andrea Arcangeli

Quoted 6 lines · Paul E. McKenney
The immediate motivation is to avoid the race with zap_page_range() when another node writes to the corresponding portion of the file, similar to the situation with vmtruncate(). The thought was to leverage locking within the distributed filesystem, but if the race is solved locally, then, as you say, perhaps this is not necessary.

Quoted 5 lines

exactly, this was my idea. Since we've the same race locally even on ext2, maybe it worth to share the fix for all the fs somehow, the problem sounds the same. You may still need callbacks to get right the distributed fs though. still I was just wondering if the conceptual fix could live in the highlevel rather than replicating it in the lowlevel.

I believe that we would need callbacks for DFSes because many of them have different locking granularities. For example, a number of them allow writes from different clients to different pages of the same file fully in parallel, with no communication between the clients required. Communication is required only when one client attempts to write to a page most recently written to by another client.

The locking required to support this is more complex than would be justified in most local-filesystem cases, I suspect. ;-)

Quoted 3 lines

Quoted 1 line
This sounds good to me, though am checking with some DFS people.
cool thanks!

Good news, they are OK with your approach for handling the truncation race! They still believe they need the callback for handling fine-grained locking required for invalidations.

Quoted 18 lines

Quoted 10 lines
Quoted 6 lines
And w/o proper high level locking, the non distributed filesystems will corrupt the VM too with truncate against nopage. I already fixed this in my tree. (see the attachment) So I wonder if the fixes could be shared. I mean, you definitely need my fixes even when using the DFS on a isolated box, and if you don't need them while using the fs locally, it means we're duplicating effort somehow.
True -- my patches simply provided hooks to allow DFSs and local filesystems to fix the problem.
Quoted 7 lines
So, the idea is for the DFS to hold a fr_write_lock on the truncate_lock across the invalidate_mmap_range() call, thus preventing the PTEs from any racing pagefaults from being installed? This seems plausible at first glance, but need to stare at it some more. This might permit the current do_no_page(), do_anonymous_page(), and ->nopage APIs to be used, but again, need to stare at it some more.

Quoted 2 lines

btw, we can discuss this some more next month at OLS too, if we didn't clear all the issues first.

Sounds great!!! Of course, if we get it all agreed to before then, we will just have to find other things to talk about at OLS. ;-)

Hopefully by that time, one of the DFSes will be released under GPL. Don't ask. :-/

Quoted 5 lines

Quoted 2 lines
(If I am not too confused, fr_write_lock() became
write_seqlock() in the 2.5 tree...)
exactly, I didn't rename it yet in 2.4 since it would provide no runtime benefit, but it is exactly the same thing ;).

Cool! Then there is no need to worry about fr_lock getting accepted into 2.5. ;-)

Quoted 10 lines

Quoted 8 lines
Quoted 4 lines
Since I don't see the users of the new hook, it's a bit hard to judje if the duplication is legitimate or not. So overall I'd agree with Andrew that to judje the patch it'd make sense to see (or know more) about the users of the hook too.
A simple change that takes care of all the cases certainly does seem better than a more complex change that only takes care of distributed filesystems!
agreed.

Unless you have other thoughts, I will take a stab at making the patches safe for each other.

Thanx, Paul

Quoted 3 lines

thanks,
Andrea