[PATCH] rmap 24 no rmap fastcalls

[PATCH] rmap 24 no rmap fastcalls

[Hugh Dickins]

First of a batch of nine miscellaneous minor and mostly small patches,
based on 2.6.6-rc3-mm2: some taken from Andrea's tree (not verbatim:
blame screwups on me); others preparatory work by me, moving slowly
but inexorably towards anon_vma... yet still not getting there.

rmap 24 no rmap fastcalls

I like CONFIG_REGPARM, even when it's forced on: because it's easy to
force off for debugging - easier than editing out scattered fastcalls.
Plus I've never understood why we make function foo a fastcall,
but function bar not.  Remove fastcall directives from rmap.  And
fix comment about mremap_moved race: it only applies to anon pages.

 include/linux/rmap.h |   14 ++++++--------
 mm/rmap.c            |   16 ++++++++--------
 2 files changed, 14 insertions(+), 16 deletions(-)

--- 2.6.6-rc3-mm2/include/linux/rmap.h	2004-05-05 13:29:08.000000000 +0100
+++ rmap24/include/linux/rmap.h	2004-05-08 20:54:32.419571320 +0100
@@ -6,7 +6,6 @@
  */
 
 #include <linux/config.h>
-#include <linux/linkage.h>
 
 #define rmap_lock(page) \
 	bit_spin_lock(PG_maplock, (unsigned long *)&(page)->flags)
@@ -15,10 +14,9 @@
 
 #ifdef CONFIG_MMU
 
-void fastcall page_add_anon_rmap(struct page *,
-		struct mm_struct *, unsigned long addr);
-void fastcall page_add_file_rmap(struct page *);
-void fastcall page_remove_rmap(struct page *);
+void page_add_anon_rmap(struct page *, struct mm_struct *, unsigned long);
+void page_add_file_rmap(struct page *);
+void page_remove_rmap(struct page *);
 
 /**
  * page_dup_rmap - duplicate pte mapping to a page
@@ -34,7 +32,7 @@ static inline void page_dup_rmap(struct 
 	rmap_unlock(page);
 }
 
-int fastcall mremap_move_anon_rmap(struct page *page, unsigned long addr);
+int mremap_move_anon_rmap(struct page *page, unsigned long addr);
 
 /**
  * mremap_moved_anon_rmap - does new address clash with that noted?
@@ -85,8 +83,8 @@ void exit_rmap(struct mm_struct *);
 /*
  * Called from mm/vmscan.c to handle paging out
  */
-int fastcall page_referenced(struct page *);
-int fastcall try_to_unmap(struct page *);
+int page_referenced(struct page *);
+int try_to_unmap(struct page *);
 
 #else	/* !CONFIG_MMU */
 
--- 2.6.6-rc3-mm2/mm/rmap.c	2004-05-05 13:29:08.000000000 +0100
+++ rmap24/mm/rmap.c	2004-05-08 20:54:32.421571016 +0100
@@ -259,8 +259,8 @@ static inline int page_referenced_anon(s
 	}
 
 	/*
-	 * The warning below may appear if page_referenced catches the
-	 * page in between page_add_{anon,file}_rmap and its replacement
+	 * The warning below may appear if page_referenced_anon catches
+	 * the page in between page_add_anon_rmap and its replacement
 	 * demanded by mremap_moved_anon_page: so remove the warning once
 	 * we're convinced that anonmm rmap really is finding its pages.
 	 */
@@ -343,7 +343,7 @@ out:
  * returns the number of ptes which referenced the page.
  * Caller needs to hold the rmap lock.
  */
-int fastcall page_referenced(struct page *page)
+int page_referenced(struct page *page)
 {
 	int referenced = 0;
 
@@ -370,7 +370,7 @@ int fastcall page_referenced(struct page
  *
  * The caller needs to hold the mm->page_table_lock.
  */
-void fastcall page_add_anon_rmap(struct page *page,
+void page_add_anon_rmap(struct page *page,
 	struct mm_struct *mm, unsigned long address)
 {
 	struct anonmm *anonmm = mm->anonmm;
@@ -396,7 +396,7 @@ void fastcall page_add_anon_rmap(struct 
  *
  * The caller needs to hold the mm->page_table_lock.
  */
-void fastcall page_add_file_rmap(struct page *page)
+void page_add_file_rmap(struct page *page)
 {
 	BUG_ON(PageAnon(page));
 	if (!pfn_valid(page_to_pfn(page)) || PageReserved(page))
@@ -415,7 +415,7 @@ void fastcall page_add_file_rmap(struct 
  *
  * Caller needs to hold the mm->page_table_lock.
  */
-void fastcall page_remove_rmap(struct page *page)
+void page_remove_rmap(struct page *page)
 {
 	BUG_ON(PageReserved(page));
 	BUG_ON(!page->mapcount);
@@ -444,7 +444,7 @@ void fastcall page_remove_rmap(struct pa
  * If it is shared, then caller must take a copy of the page instead:
  * not very clever, but too rare a case to merit cleverness.
  */
-int fastcall mremap_move_anon_rmap(struct page *page, unsigned long address)
+int mremap_move_anon_rmap(struct page *page, unsigned long address)
 {
 	int move = 0;
 	if (page->mapcount == 1) {
@@ -797,7 +797,7 @@ out:
  * SWAP_AGAIN	- we missed a trylock, try again later
  * SWAP_FAIL	- the page is unswappable
  */
-int fastcall try_to_unmap(struct page *page)
+int try_to_unmap(struct page *page)
 {
 	int ret;
 

[PATCH] rmap 24 pte_young first

[Hugh Dickins]

From: Andrea Arcangeli <andrea@suse.de>

rmap test pte_young before doing the costlier ptep_test_and_clear_young.

 rmap.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

--- rmap24/mm/rmap.c	2004-05-08 20:54:32.421571016 +0100
+++ rmap25/mm/rmap.c	2004-05-08 20:54:43.358908288 +0100
@@ -198,7 +198,7 @@ static int page_referenced_one(struct pa
 	if (page_to_pfn(page) != pte_pfn(*pte))
 		goto out_unmap;
 
-	if (ptep_test_and_clear_young(pte))
+	if (pte_young(*pte) && ptep_test_and_clear_young(pte))
 		referenced++;
 
 	(*mapcount)--;
@@ -506,7 +506,7 @@ static int try_to_unmap_one(struct page 
 	 * skipped over this mm) then we should reactivate it.
 	 */
 	if ((vma->vm_flags & (VM_LOCKED|VM_RESERVED)) ||
-			ptep_test_and_clear_young(pte)) {
+	    (pte_young(*pte) && ptep_test_and_clear_young(pte))) {
 		ret = SWAP_FAIL;
 		goto out_unmap;
 	}
@@ -606,7 +606,7 @@ static int try_to_unmap_cluster(struct m
 		if (PageReserved(page))
 			continue;
 
-		if (ptep_test_and_clear_young(pte))
+		if (pte_young(*pte) && ptep_test_and_clear_young(pte))
 			continue;
 
 		/* Nuke the page table entry. */

[PATCH] rmap 26 __setup_arg_pages

[Hugh Dickins]

anon_vma will need to pass vma to "put_dirty_page"; but instead of just
adding that arg to its callers, try to tidy this area up a little with
__setup_arg_pages(bprm, stack_top, vm_stack_flags), cutting duplicated
code out of the 64-bit support for 32-bit execs.  Still fairly horrid
(they tend to #include fs/binfmt_elf.c after #defining setup_arg_pages
to local variant), I'd break their builds if we went further.

Rename put_dirty_page to install_arg_page, now static to fs/exec.c.
While there, moved its flush_dcache_page up before page_table_lock -
doesn't in fact matter at all, just saves one worry when researching
flush_dcache_page locking constraints.

 arch/ia64/ia32/binfmt_elf32.c  |   59 +++----------------------------
 arch/ia64/mm/init.c            |    2 -
 arch/s390/kernel/compat_exec.c |   78 +----------------------------------------
 arch/x86_64/ia32/ia32_binfmt.c |   65 ++++------------------------------
 fs/exec.c                      |   52 +++++++++++++++------------
 include/linux/binfmts.h        |    2 +
 include/linux/mm.h             |    2 -
 7 files changed, 49 insertions(+), 211 deletions(-)

--- rmap25/arch/ia64/ia32/binfmt_elf32.c	2004-05-05 13:29:08.000000000 +0100
+++ rmap26/arch/ia64/ia32/binfmt_elf32.c	2004-05-08 20:54:54.347237808 +0100
@@ -152,61 +152,14 @@ ia64_elf32_init (struct pt_regs *regs)
 int
 ia32_setup_arg_pages (struct linux_binprm *bprm, int executable_stack)
 {
-	unsigned long stack_base;
-	struct vm_area_struct *mpnt;
-	struct mm_struct *mm = current->mm;
-	int i;
+	unsigned long vm_stack_flags = VM_STACK_FLAGS;
 
-	stack_base = IA32_STACK_TOP - MAX_ARG_PAGES*PAGE_SIZE;
-	mm->arg_start = bprm->p + stack_base;
+	if (executable_stack == EXSTACK_ENABLE_X)
+		vm_stack_flags |= VM_EXEC;
+	else if (executable_stack == EXSTACK_DISABLE_X)
+		vm_stack_flags &= ~VM_EXEC;
 
-	bprm->p += stack_base;
-	if (bprm->loader)
-		bprm->loader += stack_base;
-	bprm->exec += stack_base;
-
-	mpnt = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
-	if (!mpnt)
-		return -ENOMEM;
-
-	if (security_vm_enough_memory((IA32_STACK_TOP - (PAGE_MASK & (unsigned long) bprm->p))>>PAGE_SHIFT)) {
-		kmem_cache_free(vm_area_cachep, mpnt);
-		return -ENOMEM;
-	}
-
-	down_write(&current->mm->mmap_sem);
-	{
-		mpnt->vm_mm = current->mm;
-		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
-		mpnt->vm_end = IA32_STACK_TOP;
-		if (executable_stack == EXSTACK_ENABLE_X)
-			mpnt->vm_flags = VM_STACK_FLAGS |  VM_EXEC;
-		else if (executable_stack == EXSTACK_DISABLE_X)
-			mpnt->vm_flags = VM_STACK_FLAGS & ~VM_EXEC;
-		else
-			mpnt->vm_flags = VM_STACK_FLAGS;
-		mpnt->vm_page_prot = (mpnt->vm_flags & VM_EXEC)?
-					PAGE_COPY_EXEC: PAGE_COPY;
-		mpnt->vm_ops = NULL;
-		mpnt->vm_pgoff = 0;
-		mpnt->vm_file = NULL;
-		mpnt->vm_private_data = 0;
-		mpol_set_vma_default(mpnt);
-		insert_vm_struct(current->mm, mpnt);
-		current->mm->total_vm = (mpnt->vm_end - mpnt->vm_start) >> PAGE_SHIFT;
-	}
-
-	for (i = 0 ; i < MAX_ARG_PAGES ; i++) {
-		struct page *page = bprm->page[i];
-		if (page) {
-			bprm->page[i] = NULL;
-			put_dirty_page(current, page, stack_base, mpnt->vm_page_prot);
-		}
-		stack_base += PAGE_SIZE;
-	}
-	up_write(&current->mm->mmap_sem);
-
-	return 0;
+	return __setup_arg_pages(bprm, IA32_STACK_TOP, vm_stack_flags);
 }
 
 static void
--- rmap25/arch/ia64/mm/init.c	2004-05-05 13:29:12.000000000 +0100
+++ rmap26/arch/ia64/mm/init.c	2004-05-08 20:54:54.349237504 +0100
@@ -222,7 +222,7 @@ free_initrd_mem (unsigned long start, un
 }
 
 /*
- * This is like put_dirty_page() but installs a clean page in the kernel's page table.
+ * This installs a clean page in the kernel's page table.
  */
 struct page *
 put_kernel_page (struct page *page, unsigned long address, pgprot_t pgprot)
--- rmap25/arch/s390/kernel/compat_exec.c	2004-05-05 13:29:08.000000000 +0100
+++ rmap26/arch/s390/kernel/compat_exec.c	2004-05-08 20:54:54.350237352 +0100
@@ -8,87 +8,13 @@
  *
  */
 
-#include <linux/config.h>
-#include <linux/slab.h>
-#include <linux/file.h>
-#include <linux/mman.h>
-#include <linux/a.out.h>
-#include <linux/stat.h>
-#include <linux/fcntl.h>
-#include <linux/smp_lock.h>
-#include <linux/init.h>
-#include <linux/pagemap.h>
 #include <linux/mm.h>
-#include <linux/highmem.h>
-#include <linux/spinlock.h>
 #include <linux/binfmts.h>
 #include <linux/module.h>
-#include <linux/security.h>
-
-#include <asm/uaccess.h>
-#include <asm/pgalloc.h>
-#include <asm/mmu_context.h>
-
-#ifdef CONFIG_KMOD
-#include <linux/kmod.h>
-#endif
-
-
-#undef STACK_TOP
-#define STACK_TOP TASK31_SIZE
 
 int setup_arg_pages32(struct linux_binprm *bprm, int executable_stack)
 {
-	unsigned long stack_base;
-	struct vm_area_struct *mpnt;
-	struct mm_struct *mm = current->mm;
-	int i;
-
-	stack_base = STACK_TOP - MAX_ARG_PAGES*PAGE_SIZE;
-	mm->arg_start = bprm->p + stack_base;
-
-	bprm->p += stack_base;
-	if (bprm->loader)
-		bprm->loader += stack_base;
-	bprm->exec += stack_base;
-
-	mpnt = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
-	if (!mpnt) 
-		return -ENOMEM; 
-	
-	if (security_vm_enough_memory((STACK_TOP - (PAGE_MASK & (unsigned long) bprm->p))>>PAGE_SHIFT)) {
-		kmem_cache_free(vm_area_cachep, mpnt);
-		return -ENOMEM;
-	}
-
-	down_write(&mm->mmap_sem);
-	{
-		mpnt->vm_mm = mm;
-		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
-		mpnt->vm_end = STACK_TOP;
-		/* executable stack setting would be applied here */
-		mpnt->vm_page_prot = PAGE_COPY;
-		mpnt->vm_flags = VM_STACK_FLAGS;
-		mpnt->vm_ops = NULL;
-		mpnt->vm_pgoff = 0;
-		mpnt->vm_file = NULL;
-		mpol_set_vma_default(mpnt);
-		mpnt->vm_private_data = (void *) 0;
-		insert_vm_struct(mm, mpnt);
-		mm->total_vm = (mpnt->vm_end - mpnt->vm_start) >> PAGE_SHIFT;
-	} 
-
-	for (i = 0 ; i < MAX_ARG_PAGES ; i++) {
-		struct page *page = bprm->page[i];
-		if (page) {
-			bprm->page[i] = NULL;
-			put_dirty_page(current,page,stack_base,PAGE_COPY);
-		}
-		stack_base += PAGE_SIZE;
-	}
-	up_write(&mm->mmap_sem);
-	
-	return 0;
+	/* executable_stack argument is currently ignored */
+	return __setup_arg_pages(bprm, TASK31_SIZE, VM_STACK_FLAGS);
 }
-
 EXPORT_SYMBOL(setup_arg_pages32);
--- rmap25/arch/x86_64/ia32/ia32_binfmt.c	2004-05-05 13:29:08.000000000 +0100
+++ rmap26/arch/x86_64/ia32/ia32_binfmt.c	2004-05-08 20:54:54.351237200 +0100
@@ -325,63 +325,16 @@ static void elf32_init(struct pt_regs *r
 	me->thread.es = __USER_DS;
 }
 
-int setup_arg_pages(struct linux_binprm *bprm, int executable_stack)
+int ia32_setup_arg_pages(struct linux_binprm *bprm, int executable_stack)
 {
-	unsigned long stack_base;
-	struct vm_area_struct *mpnt;
-	struct mm_struct *mm = current->mm;
-	int i;
-
-	stack_base = IA32_STACK_TOP - MAX_ARG_PAGES * PAGE_SIZE;
-	mm->arg_start = bprm->p + stack_base;
-
-	bprm->p += stack_base;
-	if (bprm->loader)
-		bprm->loader += stack_base;
-	bprm->exec += stack_base;
-
-	mpnt = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
-	if (!mpnt) 
-		return -ENOMEM; 
-	
-	if (security_vm_enough_memory((IA32_STACK_TOP - (PAGE_MASK & (unsigned long) bprm->p))>>PAGE_SHIFT)) {
-		kmem_cache_free(vm_area_cachep, mpnt);
-		return -ENOMEM;
-	}
-
-	down_write(&mm->mmap_sem);
-	{
-		mpnt->vm_mm = mm;
-		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
-		mpnt->vm_end = IA32_STACK_TOP;
-		if (executable_stack == EXSTACK_ENABLE_X)
-			mpnt->vm_flags = vm_stack_flags32 |  VM_EXEC;
-		else if (executable_stack == EXSTACK_DISABLE_X)
-			mpnt->vm_flags = vm_stack_flags32 & ~VM_EXEC;
-		else
-			mpnt->vm_flags = vm_stack_flags32;
- 		mpnt->vm_page_prot = (mpnt->vm_flags & VM_EXEC) ? 
- 			PAGE_COPY_EXEC : PAGE_COPY;
-		mpnt->vm_ops = NULL;
-		mpnt->vm_pgoff = 0;
-		mpnt->vm_file = NULL;
-		mpol_set_vma_default(mpnt);
-		mpnt->vm_private_data = (void *) 0;
-		insert_vm_struct(mm, mpnt);
-		mm->total_vm = (mpnt->vm_end - mpnt->vm_start) >> PAGE_SHIFT;
-	} 
-
-	for (i = 0 ; i < MAX_ARG_PAGES ; i++) {
-		struct page *page = bprm->page[i];
-		if (page) {
-			bprm->page[i] = NULL;
-			put_dirty_page(current,page,stack_base,mpnt->vm_page_prot);
-		}
-		stack_base += PAGE_SIZE;
-	}
-	up_write(&mm->mmap_sem);
-	
-	return 0;
+	unsigned long vm_stack_flags = vm_stack_flags32;
+
+	if (executable_stack == EXSTACK_ENABLE_X)
+		vm_stack_flags |= VM_EXEC;
+	else if (executable_stack == EXSTACK_DISABLE_X)
+		vm_stack_flags &= ~VM_EXEC;
+
+	return __setup_arg_pages(bprm, IA32_STACK_TOP, vm_stack_flags);
 }
 
 static unsigned long
--- rmap25/fs/exec.c	2004-05-05 13:29:10.000000000 +0100
+++ rmap26/fs/exec.c	2004-05-08 20:54:54.354236744 +0100
@@ -293,17 +293,19 @@ EXPORT_SYMBOL(copy_strings_kernel);
  * This routine is used to map in a page into an address space: needed by
  * execve() for the initial stack and environment pages.
  *
- * tsk->mm->mmap_sem is held for writing.
+ * vma->vm_mm->mmap_sem is held for writing.
  */
-void put_dirty_page(struct task_struct *tsk, struct page *page,
-			unsigned long address, pgprot_t prot)
+static void install_arg_page(struct vm_area_struct *vma,
+			struct page *page, unsigned long address)
 {
-	struct mm_struct *mm = tsk->mm;
+	struct mm_struct *mm = vma->vm_mm;
 	pgd_t * pgd;
 	pmd_t * pmd;
 	pte_t * pte;
 
+	flush_dcache_page(page);
 	pgd = pgd_offset(mm, address);
+
 	spin_lock(&mm->page_table_lock);
 	pmd = pmd_alloc(mm, pgd, address);
 	if (!pmd)
@@ -317,8 +319,8 @@ void put_dirty_page(struct task_struct *
 	}
 	mm->rss++;
 	lru_cache_add_active(page);
-	flush_dcache_page(page);
-	set_pte(pte, pte_mkdirty(pte_mkwrite(mk_pte(page, prot))));
+	set_pte(pte, pte_mkdirty(pte_mkwrite(mk_pte(
+					page, vma->vm_page_prot))));
 	page_add_anon_rmap(page, mm, address);
 	pte_unmap(pte);
 	spin_unlock(&mm->page_table_lock);
@@ -328,10 +330,11 @@ void put_dirty_page(struct task_struct *
 out:
 	spin_unlock(&mm->page_table_lock);
 	__free_page(page);
-	force_sig(SIGKILL, tsk);
+	force_sig(SIGKILL, current);
 }
 
-int setup_arg_pages(struct linux_binprm *bprm, int executable_stack)
+int __setup_arg_pages(struct linux_binprm *bprm,
+		unsigned long stack_top, unsigned long vm_stack_flags)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -375,7 +378,7 @@ int setup_arg_pages(struct linux_binprm 
 	stack_base = current->rlim[RLIMIT_STACK].rlim_max;
 	if (stack_base > (1 << 30))
 		stack_base = 1 << 30;
-	stack_base = PAGE_ALIGN(STACK_TOP - stack_base);
+	stack_base = PAGE_ALIGN(stack_top - stack_base);
 
 	mm->arg_start = stack_base;
 	arg_size = i << PAGE_SHIFT;
@@ -384,9 +387,9 @@ int setup_arg_pages(struct linux_binprm 
 	while (i < MAX_ARG_PAGES)
 		bprm->page[i++] = NULL;
 #else
-	stack_base = STACK_TOP - MAX_ARG_PAGES * PAGE_SIZE;
+	stack_base = stack_top - MAX_ARG_PAGES * PAGE_SIZE;
 	mm->arg_start = bprm->p + stack_base;
-	arg_size = STACK_TOP - (PAGE_MASK & (unsigned long) mm->arg_start);
+	arg_size = stack_top - (PAGE_MASK & (unsigned long) mm->arg_start);
 #endif
 
 	bprm->p += stack_base;
@@ -412,17 +415,9 @@ int setup_arg_pages(struct linux_binprm 
 			(PAGE_SIZE - 1 + (unsigned long) bprm->p);
 #else
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
-		mpnt->vm_end = STACK_TOP;
+		mpnt->vm_end = stack_top;
 #endif
-		/* Adjust stack execute permissions; explicitly enable
-		 * for EXSTACK_ENABLE_X, disable for EXSTACK_DISABLE_X
-		 * and leave alone (arch default) otherwise. */
-		if (unlikely(executable_stack == EXSTACK_ENABLE_X))
-			mpnt->vm_flags = VM_STACK_FLAGS |  VM_EXEC;
-		else if (executable_stack == EXSTACK_DISABLE_X)
-			mpnt->vm_flags = VM_STACK_FLAGS & ~VM_EXEC;
-		else
-			mpnt->vm_flags = VM_STACK_FLAGS;
+		mpnt->vm_flags = vm_stack_flags;
 		mpnt->vm_page_prot = protection_map[mpnt->vm_flags & 0x7];
 		mpnt->vm_ops = NULL;
 		mpnt->vm_pgoff = 0;
@@ -437,8 +432,7 @@ int setup_arg_pages(struct linux_binprm 
 		struct page *page = bprm->page[i];
 		if (page) {
 			bprm->page[i] = NULL;
-			put_dirty_page(current, page, stack_base,
-					mpnt->vm_page_prot);
+			install_arg_page(mpnt, page, stack_base);
 		}
 		stack_base += PAGE_SIZE;
 	}
@@ -446,7 +440,19 @@ int setup_arg_pages(struct linux_binprm 
 	
 	return 0;
 }
+EXPORT_SYMBOL(__setup_arg_pages);
 
+int setup_arg_pages(struct linux_binprm *bprm, int executable_stack)
+{
+	unsigned long vm_stack_flags = VM_STACK_FLAGS;
+
+	if (executable_stack == EXSTACK_ENABLE_X)
+		vm_stack_flags |= VM_EXEC;
+	else if (executable_stack == EXSTACK_DISABLE_X)
+		vm_stack_flags &= ~VM_EXEC;
+
+	return __setup_arg_pages(bprm, STACK_TOP, vm_stack_flags);
+}
 EXPORT_SYMBOL(setup_arg_pages);
 
 #define free_arg_pages(bprm) do { } while (0)
--- rmap25/include/linux/binfmts.h	2004-05-05 13:29:09.000000000 +0100
+++ rmap26/include/linux/binfmts.h	2004-05-08 20:54:54.355236592 +0100
@@ -69,6 +69,8 @@ extern int flush_old_exec(struct linux_b
 #define EXSTACK_ENABLE_X  2	/* Enable executable stacks */
 
 extern int setup_arg_pages(struct linux_binprm * bprm, int executable_stack);
+extern int __setup_arg_pages(struct linux_binprm *bprm,
+		unsigned long stack_top, unsigned long vm_flags);
 extern int copy_strings(int argc,char __user * __user * argv,struct linux_binprm *bprm); 
 extern int copy_strings_kernel(int argc,char ** argv,struct linux_binprm *bprm);
 extern void compute_creds(struct linux_binprm *binprm);
--- rmap25/include/linux/mm.h	2004-05-05 13:29:08.000000000 +0100
+++ rmap26/include/linux/mm.h	2004-05-08 20:54:54.356236440 +0100
@@ -495,8 +495,6 @@ extern int install_file_pte(struct mm_st
 extern int handle_mm_fault(struct mm_struct *mm,struct vm_area_struct *vma, unsigned long address, int write_access);
 extern int make_pages_present(unsigned long addr, unsigned long end);
 extern int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, int len, int write);
-void put_dirty_page(struct task_struct *tsk, struct page *page,
-			unsigned long address, pgprot_t prot);
 
 int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, unsigned long start,
 		int len, int write, int force, struct page **pages, struct vm_area_struct **vmas);

[PATCH] rmap 27 memset 0 vma

[Hugh Dickins]

We're NULLifying more and more fields when initializing a vma
(mpol_set_vma_default does that too, if configured to do anything).
Now use memset to avoid specifying fields, and save a little code too.

(Yes, I realize anon_vma will want to set vm_pgoff non-0, but I think
that will be better handled at the core, since anon vm_pgoff is
negotiable up until an anon_vma is actually assigned.)

 arch/ia64/ia32/binfmt_elf32.c |   10 ++--------
 arch/ia64/kernel/perfmon.c    |    7 ++-----
 arch/ia64/mm/init.c           |    7 +------
 fs/exec.c                     |    7 ++-----
 mm/mmap.c                     |   17 +++++------------
 5 files changed, 12 insertions(+), 36 deletions(-)

--- rmap26/arch/ia64/ia32/binfmt_elf32.c	2004-05-08 20:54:54.347237808 +0100
+++ rmap27/arch/ia64/ia32/binfmt_elf32.c	2004-05-08 20:55:05.446550456 +0100
@@ -73,15 +73,13 @@ ia64_elf32_init (struct pt_regs *regs)
 	 */
 	vma = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
 	if (vma) {
+		memset(vma, 0, sizeof(*vma));
 		vma->vm_mm = current->mm;
 		vma->vm_start = IA32_GDT_OFFSET;
 		vma->vm_end = vma->vm_start + PAGE_SIZE;
 		vma->vm_page_prot = PAGE_SHARED;
 		vma->vm_flags = VM_READ|VM_MAYREAD;
 		vma->vm_ops = &ia32_shared_page_vm_ops;
-		vma->vm_pgoff = 0;
-		vma->vm_file = NULL;
-		vma->vm_private_data = NULL;
 		down_write(&current->mm->mmap_sem);
 		{
 			insert_vm_struct(current->mm, vma);
@@ -95,16 +93,12 @@ ia64_elf32_init (struct pt_regs *regs)
 	 */
 	vma = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
 	if (vma) {
+		memset(vma, 0, sizeof(*vma));
 		vma->vm_mm = current->mm;
 		vma->vm_start = IA32_LDT_OFFSET;
 		vma->vm_end = vma->vm_start + PAGE_ALIGN(IA32_LDT_ENTRIES*IA32_LDT_ENTRY_SIZE);
 		vma->vm_page_prot = PAGE_SHARED;
 		vma->vm_flags = VM_READ|VM_WRITE|VM_MAYREAD|VM_MAYWRITE;
-		vma->vm_ops = NULL;
-		vma->vm_pgoff = 0;
-		vma->vm_file = NULL;
-		vma->vm_private_data = NULL;
-		mpol_set_vma_default(vma);
 		down_write(&current->mm->mmap_sem);
 		{
 			insert_vm_struct(current->mm, vma);
--- rmap26/arch/ia64/kernel/perfmon.c	2004-05-05 13:29:08.000000000 +0100
+++ rmap27/arch/ia64/kernel/perfmon.c	2004-05-08 20:55:05.453549392 +0100
@@ -2295,6 +2295,8 @@ pfm_smpl_buffer_alloc(struct task_struct
 		DPRINT(("Cannot allocate vma\n"));
 		goto error_kmem;
 	}
+	memset(vma, 0, sizeof(*vma));
+
 	/*
 	 * partially initialize the vma for the sampling buffer
 	 *
@@ -2305,11 +2307,6 @@ pfm_smpl_buffer_alloc(struct task_struct
 	vma->vm_mm	     = mm;
 	vma->vm_flags	     = VM_READ| VM_MAYREAD |VM_RESERVED;
 	vma->vm_page_prot    = PAGE_READONLY; /* XXX may need to change */
-	vma->vm_ops	     = NULL;
-	vma->vm_pgoff	     = 0;
-	vma->vm_file	     = NULL;
-	mpol_set_vma_default(vma);
-	vma->vm_private_data = NULL; 
 
 	/*
 	 * Now we have everything we need and we can initialize
--- rmap26/arch/ia64/mm/init.c	2004-05-08 20:54:54.349237504 +0100
+++ rmap27/arch/ia64/mm/init.c	2004-05-08 20:55:05.456548936 +0100
@@ -123,16 +123,12 @@ ia64_init_addr_space (void)
 	 */
 	vma = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
 	if (vma) {
+		memset(vma, 0, sizeof(*vma));
 		vma->vm_mm = current->mm;
 		vma->vm_start = current->thread.rbs_bot & PAGE_MASK;
 		vma->vm_end = vma->vm_start + PAGE_SIZE;
 		vma->vm_page_prot = protection_map[VM_DATA_DEFAULT_FLAGS & 0x7];
 		vma->vm_flags = VM_READ|VM_WRITE|VM_MAYREAD|VM_MAYWRITE|VM_GROWSUP;
-		vma->vm_ops = NULL;
-		vma->vm_pgoff = 0;
-		vma->vm_file = NULL;
-		vma->vm_private_data = NULL;
-		mpol_set_vma_default(vma);
 		insert_vm_struct(current->mm, vma);
 	}
 
@@ -145,7 +141,6 @@ ia64_init_addr_space (void)
 			vma->vm_end = PAGE_SIZE;
 			vma->vm_page_prot = __pgprot(pgprot_val(PAGE_READONLY) | _PAGE_MA_NAT);
 			vma->vm_flags = VM_READ | VM_MAYREAD | VM_IO | VM_RESERVED;
-			mpol_set_vma_default(vma);
 			insert_vm_struct(current->mm, vma);
 		}
 	}
--- rmap26/fs/exec.c	2004-05-08 20:54:54.354236744 +0100
+++ rmap27/fs/exec.c	2004-05-08 20:55:05.458548632 +0100
@@ -406,6 +406,8 @@ int __setup_arg_pages(struct linux_binpr
 		return -ENOMEM;
 	}
 
+	memset(mpnt, 0, sizeof(*mpnt));
+
 	down_write(&mm->mmap_sem);
 	{
 		mpnt->vm_mm = mm;
@@ -419,11 +421,6 @@ int __setup_arg_pages(struct linux_binpr
 #endif
 		mpnt->vm_flags = vm_stack_flags;
 		mpnt->vm_page_prot = protection_map[mpnt->vm_flags & 0x7];
-		mpnt->vm_ops = NULL;
-		mpnt->vm_pgoff = 0;
-		mpnt->vm_file = NULL;
-		mpol_set_vma_default(mpnt);
-		mpnt->vm_private_data = (void *) 0;
 		insert_vm_struct(mm, mpnt);
 		mm->total_vm = (mpnt->vm_end - mpnt->vm_start) >> PAGE_SHIFT;
 	}
--- rmap26/mm/mmap.c	2004-05-05 13:29:10.000000000 +0100
+++ rmap27/mm/mmap.c	2004-05-08 20:55:05.460548328 +0100
@@ -688,21 +688,18 @@ munmap_back:
 	 * not unmapped, but the maps are removed from the list.
 	 */
 	vma = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
-	error = -ENOMEM;
-	if (!vma)
+	if (!vma) {
+		error = -ENOMEM;
 		goto unacct_error;
+	}
+	memset(vma, 0, sizeof(*vma));
 
 	vma->vm_mm = mm;
 	vma->vm_start = addr;
 	vma->vm_end = addr + len;
 	vma->vm_flags = vm_flags;
 	vma->vm_page_prot = protection_map[vm_flags & 0x0f];
-	vma->vm_ops = NULL;
 	vma->vm_pgoff = pgoff;
-	vma->vm_file = NULL;
-	vma->vm_private_data = NULL;
-	vma->vm_next = NULL;
-	mpol_set_vma_default(vma);
 
 	if (file) {
 		error = -EINVAL;
@@ -1446,17 +1443,13 @@ unsigned long do_brk(unsigned long addr,
 		vm_unacct_memory(len >> PAGE_SHIFT);
 		return -ENOMEM;
 	}
+	memset(vma, 0, sizeof(*vma));
 
 	vma->vm_mm = mm;
 	vma->vm_start = addr;
 	vma->vm_end = addr + len;
 	vma->vm_flags = flags;
 	vma->vm_page_prot = protection_map[flags & 0x0f];
-	vma->vm_ops = NULL;
-	vma->vm_pgoff = 0;
-	vma->vm_file = NULL;
-	vma->vm_private_data = NULL;
-	mpol_set_vma_default(vma);
 	vma_link(mm, vma, prev, rb_link, rb_parent);
 out:
 	mm->total_vm += len >> PAGE_SHIFT;

[PATCH] rmap 28 remove_vm_struct

[Hugh Dickins]

The callers of remove_shared_vm_struct then proceed to do several
more identical things: gather them together in remove_vm_struct.

 mmap.c |   31 +++++++++++--------------------
 1 files changed, 11 insertions(+), 20 deletions(-)

--- rmap27/mm/mmap.c	2004-05-08 20:55:05.460548328 +0100
+++ rmap28/mm/mmap.c	2004-05-08 20:55:16.547862800 +0100
@@ -66,7 +66,7 @@ EXPORT_SYMBOL(vm_committed_space);
 /*
  * Requires inode->i_mapping->i_shared_lock
  */
-static inline void __remove_shared_vm_struct(struct vm_area_struct *vma,
+static void __remove_shared_vm_struct(struct vm_area_struct *vma,
 		struct file *file, struct address_space *mapping)
 {
 	if (vma->vm_flags & VM_DENYWRITE)
@@ -83,9 +83,9 @@ static inline void __remove_shared_vm_st
 }
 
 /*
- * Remove one vm structure from the inode's i_mapping address space.
+ * Remove one vm structure and free it.
  */
-static void remove_shared_vm_struct(struct vm_area_struct *vma)
+static void remove_vm_struct(struct vm_area_struct *vma)
 {
 	struct file *file = vma->vm_file;
 
@@ -95,6 +95,12 @@ static void remove_shared_vm_struct(stru
 		__remove_shared_vm_struct(vma, file, mapping);
 		spin_unlock(&mapping->i_shared_lock);
 	}
+	if (vma->vm_ops && vma->vm_ops->close)
+		vma->vm_ops->close(vma);
+	if (file)
+		fput(file);
+	mpol_free(vma_policy(vma));
+	kmem_cache_free(vm_area_cachep, vma);
 }
 
 /*
@@ -1164,14 +1170,7 @@ static void unmap_vma(struct mm_struct *
 				area->vm_start < area->vm_mm->free_area_cache)
 	      area->vm_mm->free_area_cache = area->vm_start;
 
-	remove_shared_vm_struct(area);
-
-	mpol_free(vma_policy(area));
-	if (area->vm_ops && area->vm_ops->close)
-		area->vm_ops->close(area);
-	if (area->vm_file)
-		fput(area->vm_file);
-	kmem_cache_free(vm_area_cachep, area);
+	remove_vm_struct(area);
 }
 
 /*
@@ -1500,15 +1499,7 @@ void exit_mmap(struct mm_struct *mm)
 	 */
 	while (vma) {
 		struct vm_area_struct *next = vma->vm_next;
-		remove_shared_vm_struct(vma);
-		if (vma->vm_ops) {
-			if (vma->vm_ops->close)
-				vma->vm_ops->close(vma);
-		}
-		if (vma->vm_file)
-			fput(vma->vm_file);
-		mpol_free(vma_policy(vma));
-		kmem_cache_free(vm_area_cachep, vma);
+		remove_vm_struct(vma);
 		vma = next;
 	}
 }

[PATCH] rmap 29 VM_RESERVED safety

[Hugh Dickins]

From: Andrea Arcangeli <andrea@suse.de>

Set VM_RESERVED in videobuf_mmap_mapper, to warn do_no_page and swapout
not to worry about its pages.  Set VM_RESERVED in ia64_elf32_init, it
too provides an unusual nopage which might surprise higher level checks.
Future safety: they don't actually pose a problem in this current tree.

 arch/ia64/ia32/binfmt_elf32.c   |    2 +-
 drivers/media/video/video-buf.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--- rmap28/arch/ia64/ia32/binfmt_elf32.c	2004-05-08 20:55:05.446550456 +0100
+++ rmap29/arch/ia64/ia32/binfmt_elf32.c	2004-05-08 20:55:27.517195208 +0100
@@ -78,7 +78,7 @@ ia64_elf32_init (struct pt_regs *regs)
 		vma->vm_start = IA32_GDT_OFFSET;
 		vma->vm_end = vma->vm_start + PAGE_SIZE;
 		vma->vm_page_prot = PAGE_SHARED;
-		vma->vm_flags = VM_READ|VM_MAYREAD;
+		vma->vm_flags = VM_READ|VM_MAYREAD|VM_RESERVED;
 		vma->vm_ops = &ia32_shared_page_vm_ops;
 		down_write(&current->mm->mmap_sem);
 		{
--- rmap28/drivers/media/video/video-buf.c	2004-04-04 03:38:43.000000000 +0100
+++ rmap29/drivers/media/video/video-buf.c	2004-05-08 20:55:27.519194904 +0100
@@ -1176,7 +1176,7 @@ int videobuf_mmap_mapper(struct vm_area_
 	map->end      = vma->vm_end;
 	map->q        = q;
 	vma->vm_ops   = &videobuf_vm_ops;
-	vma->vm_flags |= VM_DONTEXPAND;
+	vma->vm_flags |= VM_DONTEXPAND | VM_RESERVED;
 	vma->vm_flags &= ~VM_IO; /* using shared anonymous pages */
 	vma->vm_private_data = map;
 	dprintk(1,"mmap %p: %08lx-%08lx pgoff %08lx bufs %d-%d\n",

[PATCH] rmap 30 fix bad mapcount

[Hugh Dickins]

From: Andrea Arcangeli <andrea@suse.de>

page_alloc.c's bad_page routine should reset a bad mapcount; and it's
more revealing to show the bad mapcount than just the boolean mapped.

 page_alloc.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

--- rmap29/mm/page_alloc.c	2004-05-05 13:29:08.000000000 +0100
+++ rmap30/mm/page_alloc.c	2004-05-08 20:55:38.593511352 +0100
@@ -73,9 +73,9 @@ static void bad_page(const char *functio
 {
 	printk(KERN_EMERG "Bad page state at %s (in process '%s', page %p)\n",
 		function, current->comm, page);
-	printk(KERN_EMERG "flags:0x%08lx mapping:%p mapped:%d count:%d\n",
+	printk(KERN_EMERG "flags:0x%08lx mapping:%p mapcount:%d count:%d\n",
 		(unsigned long)page->flags, page->mapping,
-		page_mapped(page), page_count(page));
+		(int)page->mapcount, page_count(page));
 	printk(KERN_EMERG "Backtrace:\n");
 	dump_stack();
 	printk(KERN_EMERG "Trying to fix it up, but a reboot is needed\n");
@@ -90,6 +90,7 @@ static void bad_page(const char *functio
 			1 << PG_writeback);
 	set_page_count(page, 0);
 	page->mapping = NULL;
+	page->mapcount = 0;
 }
 
 #ifndef CONFIG_HUGETLB_PAGE

[PATCH] rmap 31 unlikely bad memory

[Hugh Dickins]

From: Andrea Arcangeli <andrea@suse.de>

Sprinkle unlikelys throughout mm/memory.c, wherever we see a
pgd_bad or a pmd_bad; likely or unlikely on pte_same or !pte_same.
Put the jump in the error return from do_no_page, not in the fast path.

 memory.c |   32 ++++++++++++++++----------------
 1 files changed, 16 insertions(+), 16 deletions(-)

--- rmap30/mm/memory.c	2004-05-05 13:29:09.000000000 +0100
+++ rmap31/mm/memory.c	2004-05-08 20:55:49.588839808 +0100
@@ -97,7 +97,7 @@ static inline void free_one_pmd(struct m
 
 	if (pmd_none(*dir))
 		return;
-	if (pmd_bad(*dir)) {
+	if (unlikely(pmd_bad(*dir))) {
 		pmd_ERROR(*dir);
 		pmd_clear(dir);
 		return;
@@ -115,7 +115,7 @@ static inline void free_one_pgd(struct m
 
 	if (pgd_none(*dir))
 		return;
-	if (pgd_bad(*dir)) {
+	if (unlikely(pgd_bad(*dir))) {
 		pgd_ERROR(*dir);
 		pgd_clear(dir);
 		return;
@@ -232,7 +232,7 @@ int copy_page_range(struct mm_struct *ds
 		
 		if (pgd_none(*src_pgd))
 			goto skip_copy_pmd_range;
-		if (pgd_bad(*src_pgd)) {
+		if (unlikely(pgd_bad(*src_pgd))) {
 			pgd_ERROR(*src_pgd);
 			pgd_clear(src_pgd);
 skip_copy_pmd_range:	address = (address + PGDIR_SIZE) & PGDIR_MASK;
@@ -253,7 +253,7 @@ skip_copy_pmd_range:	address = (address 
 		
 			if (pmd_none(*src_pmd))
 				goto skip_copy_pte_range;
-			if (pmd_bad(*src_pmd)) {
+			if (unlikely(pmd_bad(*src_pmd))) {
 				pmd_ERROR(*src_pmd);
 				pmd_clear(src_pmd);
 skip_copy_pte_range:
@@ -355,7 +355,7 @@ static void zap_pte_range(struct mmu_gat
 
 	if (pmd_none(*pmd))
 		return;
-	if (pmd_bad(*pmd)) {
+	if (unlikely(pmd_bad(*pmd))) {
 		pmd_ERROR(*pmd);
 		pmd_clear(pmd);
 		return;
@@ -436,7 +436,7 @@ static void zap_pmd_range(struct mmu_gat
 
 	if (pgd_none(*dir))
 		return;
-	if (pgd_bad(*dir)) {
+	if (unlikely(pgd_bad(*dir))) {
 		pgd_ERROR(*dir);
 		pgd_clear(dir);
 		return;
@@ -617,7 +617,7 @@ follow_page(struct mm_struct *mm, unsign
 		return page;
 
 	pgd = pgd_offset(mm, address);
-	if (pgd_none(*pgd) || pgd_bad(*pgd))
+	if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
 		goto out;
 
 	pmd = pmd_offset(pgd, address);
@@ -625,7 +625,7 @@ follow_page(struct mm_struct *mm, unsign
 		goto out;
 	if (pmd_huge(*pmd))
 		return follow_huge_pmd(mm, address, pmd, write);
-	if (pmd_bad(*pmd))
+	if (unlikely(pmd_bad(*pmd)))
 		goto out;
 
 	ptep = pte_offset_map(pmd, address);
@@ -682,12 +682,12 @@ untouched_anonymous_page(struct mm_struc
 
 	/* Check if page directory entry exists. */
 	pgd = pgd_offset(mm, address);
-	if (pgd_none(*pgd) || pgd_bad(*pgd))
+	if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
 		return 1;
 
 	/* Check if page middle directory entry exists. */
 	pmd = pmd_offset(pgd, address);
-	if (pmd_none(*pmd) || pmd_bad(*pmd))
+	if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
 		return 1;
 
 	/* There is a pte slot for 'address' in 'mm'. */
@@ -1092,7 +1092,7 @@ static int do_wp_page(struct mm_struct *
 	 */
 	spin_lock(&mm->page_table_lock);
 	page_table = pte_offset_map(pmd, address);
-	if (pte_same(*page_table, pte)) {
+	if (likely(pte_same(*page_table, pte))) {
 		if (PageReserved(old_page))
 			++mm->rss;
 		else
@@ -1323,7 +1323,7 @@ static int do_swap_page(struct mm_struct
 			 */
 			spin_lock(&mm->page_table_lock);
 			page_table = pte_offset_map(pmd, address);
-			if (pte_same(*page_table, orig_pte))
+			if (likely(pte_same(*page_table, orig_pte)))
 				ret = VM_FAULT_OOM;
 			else
 				ret = VM_FAULT_MINOR;
@@ -1346,7 +1346,7 @@ static int do_swap_page(struct mm_struct
 	 */
 	spin_lock(&mm->page_table_lock);
 	page_table = pte_offset_map(pmd, address);
-	if (!pte_same(*page_table, orig_pte)) {
+	if (unlikely(!pte_same(*page_table, orig_pte))) {
 		pte_unmap(page_table);
 		spin_unlock(&mm->page_table_lock);
 		unlock_page(page);
@@ -1552,12 +1552,12 @@ retry:
 	/* no need to invalidate: a not-present page shouldn't be cached */
 	update_mmu_cache(vma, address, entry);
 	spin_unlock(&mm->page_table_lock);
-	goto out;
+out:
+	return ret;
 oom:
 	page_cache_release(new_page);
 	ret = VM_FAULT_OOM;
-out:
-	return ret;
+	goto out;
 }
 
 /*

[PATCH] rmap 32 zap_pmd_range wrap

[Hugh Dickins]

From: Andrea Arcangeli <andrea@suse.de>

zap_pmd_range, alone of all those page_range loops, lacks the check for
whether address wrapped.  Hugh is in doubt as to whether this makes any
difference to any config on any arch, but eager to fix the odd one out.

 memory.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

--- rmap31/mm/memory.c	2004-05-08 20:55:49.588839808 +0100
+++ rmap32/mm/memory.c	2004-05-08 20:56:00.578169176 +0100
@@ -449,7 +449,7 @@ static void zap_pmd_range(struct mmu_gat
 		zap_pte_range(tlb, pmd, address, end - address, details);
 		address = (address + PMD_SIZE) & PMD_MASK; 
 		pmd++;
-	} while (address < end);
+	} while (address && (address < end));
 }
 
 static void unmap_page_range(struct mmu_gather *tlb,

Re: [PATCH] rmap 24 no rmap fastcalls

[Christoph Hellwig]

>  	/*
> -	 * The warning below may appear if page_referenced catches the
> -	 * page in between page_add_{anon,file}_rmap and its replacement
> +	 * The warning below may appear if page_referenced_anon catches
> +	 * the page in between page_add_anon_rmap and its replacement

is this backing out of my comment fixup intentional? :)

Re: [PATCH] rmap 24 pte_young first

[Christoph Hellwig]

On Sat, May 08, 2004 at 10:56:26PM +0100, Hugh Dickins wrote:
> From: Andrea Arcangeli <andrea@suse.de>
> 
> rmap test pte_young before doing the costlier ptep_test_and_clear_young.
> 
>  rmap.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> --- rmap24/mm/rmap.c	2004-05-08 20:54:32.421571016 +0100
> +++ rmap25/mm/rmap.c	2004-05-08 20:54:43.358908288 +0100
> @@ -198,7 +198,7 @@ static int page_referenced_one(struct pa
>  	if (page_to_pfn(page) != pte_pfn(*pte))
>  		goto out_unmap;
>  
> -	if (ptep_test_and_clear_young(pte))
> +	if (pte_young(*pte) && ptep_test_and_clear_young(pte))

stupid question - shouldn't the pte_young check simply move to
the beginning of ptep_test_and_clear_young?

Re: [PATCH] rmap 24 no rmap fastcalls

[Hugh Dickins]

On Sat, 8 May 2004, Christoph Hellwig wrote:
> >  	/*
> > -	 * The warning below may appear if page_referenced catches the
> > -	 * page in between page_add_{anon,file}_rmap and its replacement
> > +	 * The warning below may appear if page_referenced_anon catches
> > +	 * the page in between page_add_anon_rmap and its replacement
> 
> is this backing out of my comment fixup intentional? :)

I wouldn't dare!  No, look again, it doesn't back it out, it refines
it to refer solely to page_add_anon_rmap, page_add_file_rmap is not
relevant here - your fixup was that the original comment still said
page_add_rmap after that had been replaced by anon, file variants.

What was not intentional was having two rmap 24s:
this was the real 24, and the pte_young one should have said 25.

Hugh

Re: [PATCH] rmap 24 no rmap fastcalls

[Christoph Hellwig]

On Sat, May 08, 2004 at 11:32:26PM +0100, Hugh Dickins wrote:
> I wouldn't dare!  No, look again, it doesn't back it out, it refines
> it to refer solely to page_add_anon_rmap, page_add_file_rmap is not
> relevant here - your fixup was that the original comment still said
> page_add_rmap after that had been replaced by anon, file variants.

ah, thanks!

Re: [PATCH] rmap 24 pte_young first

[Hugh Dickins]

rmap 25 of course

On Sat, 8 May 2004, Christoph Hellwig wrote:
> On Sat, May 08, 2004 at 10:56:26PM +0100, Hugh Dickins wrote:
> >  
> > -	if (ptep_test_and_clear_young(pte))
> > +	if (pte_young(*pte) && ptep_test_and_clear_young(pte))
> 
> stupid question - shouldn't the pte_young check simply move to
> the beginning of ptep_test_and_clear_young?

I don't think that would be a good idea.  We're used to those
test_and_clear operations being atomic, putting an initial non-atomic
test inside would make it fundamentally non-atomic.  We know here that
it's not the end of the world if we miss a racing transition of the
young bit, but it wouldn't be good to hide and force that on others.

Hugh

Re: [PATCH] rmap 24 pte_young first

[Russell King]

On Sat, May 08, 2004 at 11:39:32PM +0100, Hugh Dickins wrote:
> rmap 25 of course
> 
> On Sat, 8 May 2004, Christoph Hellwig wrote:
> > On Sat, May 08, 2004 at 10:56:26PM +0100, Hugh Dickins wrote:
> > >  
> > > -	if (ptep_test_and_clear_young(pte))
> > > +	if (pte_young(*pte) && ptep_test_and_clear_young(pte))
> > 
> > stupid question - shouldn't the pte_young check simply move to
> > the beginning of ptep_test_and_clear_young?
> 
> I don't think that would be a good idea.  We're used to those
> test_and_clear operations being atomic, putting an initial non-atomic
> test inside would make it fundamentally non-atomic.  We know here that
> it's not the end of the world if we miss a racing transition of the
> young bit, but it wouldn't be good to hide and force that on others.

EAGAIN.

include/asm-generic/pgtable.h:

#ifndef __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG
static inline int ptep_test_and_clear_young(pte_t *ptep)
{
        pte_t pte = *ptep;
        if (!pte_young(pte))
                return 0;
        set_pte(ptep, pte_mkold(pte));
        return 1;
}
#endif


-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] rmap 24 pte_young first

[Hugh Dickins]

On Sat, 8 May 2004, Russell King wrote:
> On Sat, May 08, 2004 at 11:39:32PM +0100, Hugh Dickins wrote:
> > On Sat, 8 May 2004, Christoph Hellwig wrote:
> > > 
> > > stupid question - shouldn't the pte_young check simply move to
> > > the beginning of ptep_test_and_clear_young?
> > 
> > I don't think that would be a good idea.  We're used to those
> > test_and_clear operations being atomic, putting an initial non-atomic
> > test inside would make it fundamentally non-atomic.  We know here that
> > it's not the end of the world if we miss a racing transition of the
> > young bit, but it wouldn't be good to hide and force that on others.
> 
> EAGAIN.
> 
> include/asm-generic/pgtable.h:
> 
> #ifndef __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG
> static inline int ptep_test_and_clear_young(pte_t *ptep)
> {
>         pte_t pte = *ptep;
>         if (!pte_young(pte))
>                 return 0;
>         set_pte(ptep, pte_mkold(pte));
>         return 1;
> }
> #endif

Hah!  Delightful refutation of my little lecture.  Thanks a lot for
turning that up.  Hmm.  Well, I guess I need to research that one
further: a first guess would be that the generic variety is silly
to be doing an optimization which the specialist versions don't do:
but perhaps when I look I'll find some of them do.  Can scrub that
patch for now if you prefer, Andrew: world won't stop turning either way.

Hugh

Re: [PATCH] rmap 24 pte_young first

[Hugh Dickins]

On Sat, 8 May 2004, Hugh Dickins wrote:
> On Sat, 8 May 2004, Russell King wrote:
> > On Sat, May 08, 2004 at 11:39:32PM +0100, Hugh Dickins wrote:
> > > On Sat, 8 May 2004, Christoph Hellwig wrote:
> > > > 
> > > > stupid question - shouldn't the pte_young check simply move to
> > > > the beginning of ptep_test_and_clear_young?
> > > 
> > > I don't think that would be a good idea.
> > 
> > EAGAIN.

I think that should be -EEXIST.

> Hah!  Delightful refutation of my little lecture.  Thanks a lot for
> turning that up.  Hmm.  Well, I guess I need to research that one

Okay, the arch implementations of ptep_test_and_clear_young say
Christoph is right: the preliminary pte_young test should be buried
in there (in the SMP cases as in the UP cases), not out in rmap.c.

(Just in case anyone is in doubt, I'm absolutely not proposing a
preliminary pte_dirty test in the SMP ptep_test_and_clear_dirty.)

I'll leave it for now: it's much better dealt with when we get to
doing the (much more important) TLB flush after clearing young.

Andrew, please just ignore rmap 25 (billed as 24) "pte_young first".

Hugh