On Tue, Apr 7, 2009 at 4:27 PM, Wu Fengguang <fengguang.wu@xxxxxxxxx> wrote:
> 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..
here is the benchmark i posted on [V1] but somehow missed in [V2] describtion
Benchmarks:
case 1. one application has a high count of threads each faulting in
different pages of a hugefile. Benchmark indicate that this double data
structure walking in case of major fault results in << 1% performance hit.
case 2. add another thread in the above application which in a tight loop of
mmap()/munmap(). Here we measure loop count in the new thread while other
threads doing the same amount of work as case one. we got << 3% performance
hit on the Complete Time(benchmark value for case one) and 10% performance
improvement on the mmap()/munmap() counter.
This patch helps a lot in cases we have writer which is waitting behind all
readers, so it could execute much faster.
--Ying
>
> 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-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
