Re: [PATCH 03/14] mm: remove FAULT_FLAG_RETRY dead code

From: Wu Fengguang
Date: Tue Apr 07 2009 - 19:27:30 EST


On Wed, Apr 08, 2009 at 04:03:36AM +0800, Ying Han wrote:
> On Tue, Apr 7, 2009 at 12:17 AM, Wu Fengguang <fengguang.wu@xxxxxxxxx> wrote:
> > Cc: Ying Han <yinghan@xxxxxxxxxx>
> > Signed-off-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
> > ---
> > mm/memory.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > --- mm.orig/mm/memory.c
> > +++ mm/mm/memory.c
> > @@ -2766,10 +2766,8 @@ static int do_linear_fault(struct mm_str
> > {
> > pgoff_t pgoff = (((address & PAGE_MASK)
> > - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
> > - int write = write_access & ~FAULT_FLAG_RETRY;
> > - unsigned int flags = (write ? FAULT_FLAG_WRITE : 0);
> > + unsigned int flags = (write_access ? FAULT_FLAG_WRITE : 0);
> >
> > - flags |= (write_access & FAULT_FLAG_RETRY);
> > pte_unmap(page_table);
> > return __do_fault(mm, vma, address, pmd, pgoff, flags, orig_pte);
> > }
> So, we got rid of FAULT_FLAG_RETRY flag?

Seems yes for the current mm tree, see the following two commits.

I did this patch on seeing 761fe7bc8193b7. But a closer look
indicates that the following two patches disable the filemap
VM_FAULT_RETRY part totally...

Anyway, if these two patches are to be reverted somehow(I guess yes),
this patch shall be _ignored_.

btw, do you have any test case and performance numbers for
FAULT_FLAG_RETRY? And possible overheads for (the worst case)
sparse random mmap reads on a sparse file? I cannot find any
in your changelogs..

Thanks,
Fengguang


commit 761fe7bc8193b7858b7dc7eb4a026dc66e49fe1f
Author: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Date: Mon Feb 9 21:08:50 2009 +0100

A shot in the dark :(

Cc: Mike Waychison <mikew@xxxxxxxxxx>
Cc: Ying Han <yinghan@xxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index bac7d7a..1c6736d 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1139,8 +1139,6 @@ good_area:
return;
}

- write |= retry_flag;
-
/*
* If for any reason at all we couldn't handle the fault,
* make sure we exit gracefully rather than endlessly redo


commit f01ca7a68c37680a4eee22a8722a713c5102b3bb
Author: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Date: Mon Feb 9 21:08:50 2009 +0100

Untangle the `write' boolean from the FAULT_FLAG_foo non-boolean field.

Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
Cc: David Rientjes <rientjes@xxxxxxxxxx>
Cc: Hugh Dickins <hugh@xxxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxx>
Cc: Lee Schermerhorn <lee.schermerhorn@xxxxxx>
Cc: Mike Waychison <mikew@xxxxxxxxxx>
Cc: Nick Piggin <npiggin@xxxxxxx>
Cc: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
Cc: Rohit Seth <rohitseth@xxxxxxxxxx>
Cc: T<F6>r<F6>k Edwin <edwintorok@xxxxxxxxx>
Cc: Valdis.Kletnieks@xxxxxx
Cc: Ying Han <yinghan@xxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index b2cc88f..bac7d7a 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -978,7 +978,7 @@ do_page_fault(struct pt_regs *regs, unsigned long error_code)
struct mm_struct *mm;
int write;
int fault;
- unsigned int retry_flag = FAULT_FLAG_RETRY;
+ int retry_flag = 1;

tsk = current;
mm = tsk->mm;
@@ -1140,6 +1140,7 @@ good_area:
}

write |= retry_flag;
+
/*
* If for any reason at all we couldn't handle the fault,
* make sure we exit gracefully rather than endlessly redo
@@ -1159,8 +1160,8 @@ good_area:
* be removed or changed after the retry.
*/
if (fault & VM_FAULT_RETRY) {
- if (write & FAULT_FLAG_RETRY) {
- retry_flag &= ~FAULT_FLAG_RETRY;
+ if (retry_flag) {
+ retry_flag = 0;
goto retry;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/