* [patch]readahead: fault retry breaks mmap file read random detection
@ 2012-08-22 3:40 Shaohua Li
2012-08-22 15:40 ` Rik van Riel
0 siblings, 1 reply; 4+ messages in thread
From: Shaohua Li @ 2012-08-22 3:40 UTC (permalink / raw)
To: linux-mm; +Cc: fengguang.wu, akpm, riel
.fault now can retry. The retry can break state machine of .fault. In
filemap_fault, if page is miss, ra->mmap_miss is increased. In the second try,
since the page is in page cache now, ra->mmap_miss is decreased. And these are
done in one fault, so we can't detect random mmap file access.
Add a new flag to indicate .fault is tried once. In the second try, skip
ra->mmap_miss decreasing. The filemap_fault state machine is ok with it.
I only tested x86, didn't test other archs, but looks the change for other
archs is obvious, but who knows :)
Signed-off-by: Shaohua Li <shaohua.li@fusionio.com>
---
arch/arm/mm/fault.c | 1 +
arch/avr32/mm/fault.c | 1 +
arch/cris/mm/fault.c | 1 +
arch/hexagon/mm/vm_fault.c | 1 +
arch/ia64/mm/fault.c | 1 +
arch/m68k/mm/fault.c | 1 +
arch/microblaze/mm/fault.c | 1 +
arch/mips/mm/fault.c | 1 +
arch/openrisc/mm/fault.c | 1 +
arch/powerpc/mm/fault.c | 1 +
arch/s390/mm/fault.c | 1 +
arch/sh/mm/fault.c | 1 +
arch/sparc/mm/fault_32.c | 1 +
arch/sparc/mm/fault_64.c | 1 +
arch/tile/mm/fault.c | 1 +
arch/um/kernel/trap.c | 1 +
arch/x86/mm/fault.c | 1 +
arch/xtensa/mm/fault.c | 1 +
include/linux/mm.h | 1 +
mm/filemap.c | 4 ++--
20 files changed, 21 insertions(+), 2 deletions(-)
Index: linux/arch/x86/mm/fault.c
===================================================================
--- linux.orig/arch/x86/mm/fault.c 2012-08-22 09:51:22.939527887 +0800
+++ linux/arch/x86/mm/fault.c 2012-08-22 09:52:22.818774975 +0800
@@ -1201,6 +1201,7 @@ good_area:
/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk
* of starvation. */
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
goto retry;
}
}
Index: linux/include/linux/mm.h
===================================================================
--- linux.orig/include/linux/mm.h 2012-08-22 09:51:23.087526029 +0800
+++ linux/include/linux/mm.h 2012-08-22 09:52:22.822775020 +0800
@@ -157,6 +157,7 @@ extern pgprot_t protection_map[16];
#define FAULT_FLAG_ALLOW_RETRY 0x08 /* Retry fault if blocking */
#define FAULT_FLAG_RETRY_NOWAIT 0x10 /* Don't drop mmap_sem and wait when retrying */
#define FAULT_FLAG_KILLABLE 0x20 /* The fault task is in SIGKILL killable region */
+#define FAULT_FLAG_TRIED 0x40 /* second try */
/*
* This interface is used by x86 PAT code to identify a pfn mapping that is
Index: linux/mm/filemap.c
===================================================================
--- linux.orig/mm/filemap.c 2012-08-22 09:51:23.079526129 +0800
+++ linux/mm/filemap.c 2012-08-22 09:52:22.822775020 +0800
@@ -1611,13 +1611,13 @@ int filemap_fault(struct vm_area_struct
* Do we have something in the page cache already?
*/
page = find_get_page(mapping, offset);
- if (likely(page)) {
+ if (likely(page) && !(vmf->flags & FAULT_FLAG_TRIED)) {
/*
* We found the page, so try async readahead before
* waiting for the lock.
*/
do_async_mmap_readahead(vma, ra, file, page, offset);
- } else {
+ } else if (!page) {
/* No page in the page cache at all */
do_sync_mmap_readahead(vma, ra, file, offset);
count_vm_event(PGMAJFAULT);
Index: linux/arch/arm/mm/fault.c
===================================================================
--- linux.orig/arch/arm/mm/fault.c 2012-08-22 09:51:22.899528391 +0800
+++ linux/arch/arm/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -336,6 +336,7 @@ retry:
/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk
* of starvation. */
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
goto retry;
}
}
Index: linux/arch/avr32/mm/fault.c
===================================================================
--- linux.orig/arch/avr32/mm/fault.c 2012-08-22 09:51:23.035526683 +0800
+++ linux/arch/avr32/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -152,6 +152,7 @@ good_area:
tsk->min_flt++;
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/*
* No need to up_read(&mm->mmap_sem) as we would have
Index: linux/arch/cris/mm/fault.c
===================================================================
--- linux.orig/arch/cris/mm/fault.c 2012-08-22 09:51:23.059526379 +0800
+++ linux/arch/cris/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -186,6 +186,7 @@ retry:
tsk->min_flt++;
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/*
* No need to up_read(&mm->mmap_sem) as we would
Index: linux/arch/hexagon/mm/vm_fault.c
===================================================================
--- linux.orig/arch/hexagon/mm/vm_fault.c 2012-08-22 09:51:22.915528191 +0800
+++ linux/arch/hexagon/mm/vm_fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -113,6 +113,7 @@ good_area:
current->min_flt++;
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
goto retry;
}
}
Index: linux/arch/ia64/mm/fault.c
===================================================================
--- linux.orig/arch/ia64/mm/fault.c 2012-08-22 09:51:22.967527537 +0800
+++ linux/arch/ia64/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -184,6 +184,7 @@ retry:
current->min_flt++;
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/* No need to up_read(&mm->mmap_sem) as we would
* have already released it in __lock_page_or_retry
Index: linux/arch/m68k/mm/fault.c
===================================================================
--- linux.orig/arch/m68k/mm/fault.c 2012-08-22 09:51:23.015526933 +0800
+++ linux/arch/m68k/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -170,6 +170,7 @@ good_area:
/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk
* of starvation. */
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/*
* No need to up_read(&mm->mmap_sem) as we would
Index: linux/arch/microblaze/mm/fault.c
===================================================================
--- linux.orig/arch/microblaze/mm/fault.c 2012-08-22 09:51:22.995527183 +0800
+++ linux/arch/microblaze/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -233,6 +233,7 @@ good_area:
current->min_flt++;
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/*
* No need to up_read(&mm->mmap_sem) as we would
Index: linux/arch/mips/mm/fault.c
===================================================================
--- linux.orig/arch/mips/mm/fault.c 2012-08-22 09:51:22.975527437 +0800
+++ linux/arch/mips/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -171,6 +171,7 @@ good_area:
}
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/*
* No need to up_read(&mm->mmap_sem) as we would
Index: linux/arch/openrisc/mm/fault.c
===================================================================
--- linux.orig/arch/openrisc/mm/fault.c 2012-08-22 09:51:23.027526783 +0800
+++ linux/arch/openrisc/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -183,6 +183,7 @@ good_area:
tsk->min_flt++;
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/* No need to up_read(&mm->mmap_sem) as we would
* have already released it in __lock_page_or_retry
Index: linux/arch/powerpc/mm/fault.c
===================================================================
--- linux.orig/arch/powerpc/mm/fault.c 2012-08-22 09:51:22.987527285 +0800
+++ linux/arch/powerpc/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -450,6 +450,7 @@ good_area:
/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk
* of starvation. */
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
goto retry;
}
}
Index: linux/arch/s390/mm/fault.c
===================================================================
--- linux.orig/arch/s390/mm/fault.c 2012-08-22 09:51:23.067526279 +0800
+++ linux/arch/s390/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -367,6 +367,7 @@ retry:
/* Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk
* of starvation. */
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
down_read(&mm->mmap_sem);
goto retry;
}
Index: linux/arch/sh/mm/fault.c
===================================================================
--- linux.orig/arch/sh/mm/fault.c 2012-08-22 09:51:22.907528291 +0800
+++ linux/arch/sh/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
@@ -504,6 +504,7 @@ good_area:
}
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/*
* No need to up_read(&mm->mmap_sem) as we would
Index: linux/arch/sparc/mm/fault_32.c
===================================================================
--- linux.orig/arch/sparc/mm/fault_32.c 2012-08-22 09:51:22.955527687 +0800
+++ linux/arch/sparc/mm/fault_32.c 2012-08-22 09:52:22.826775037 +0800
@@ -265,6 +265,7 @@ good_area:
}
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/* No need to up_read(&mm->mmap_sem) as we would
* have already released it in __lock_page_or_retry
Index: linux/arch/sparc/mm/fault_64.c
===================================================================
--- linux.orig/arch/sparc/mm/fault_64.c 2012-08-22 09:51:22.947527787 +0800
+++ linux/arch/sparc/mm/fault_64.c 2012-08-22 09:52:22.826775037 +0800
@@ -452,6 +452,7 @@ good_area:
}
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/* No need to up_read(&mm->mmap_sem) as we would
* have already released it in __lock_page_or_retry
Index: linux/arch/tile/mm/fault.c
===================================================================
--- linux.orig/arch/tile/mm/fault.c 2012-08-22 09:51:23.007527033 +0800
+++ linux/arch/tile/mm/fault.c 2012-08-22 09:52:22.826775037 +0800
@@ -454,6 +454,7 @@ good_area:
tsk->min_flt++;
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/*
* No need to up_read(&mm->mmap_sem) as we would
Index: linux/arch/um/kernel/trap.c
===================================================================
--- linux.orig/arch/um/kernel/trap.c 2012-08-22 09:51:23.047526530 +0800
+++ linux/arch/um/kernel/trap.c 2012-08-22 09:52:22.826775037 +0800
@@ -89,6 +89,7 @@ good_area:
current->min_flt++;
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
goto retry;
}
Index: linux/arch/xtensa/mm/fault.c
===================================================================
--- linux.orig/arch/xtensa/mm/fault.c 2012-08-22 09:51:22.927528040 +0800
+++ linux/arch/xtensa/mm/fault.c 2012-08-22 09:52:22.826775037 +0800
@@ -126,6 +126,7 @@ good_area:
current->min_flt++;
if (fault & VM_FAULT_RETRY) {
flags &= ~FAULT_FLAG_ALLOW_RETRY;
+ flags |= FAULT_FLAG_TRIED;
/* No need to up_read(&mm->mmap_sem) as we would
* have already released it in __lock_page_or_retry
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [patch]readahead: fault retry breaks mmap file read random detection
2012-08-22 3:40 [patch]readahead: fault retry breaks mmap file read random detection Shaohua Li
@ 2012-08-22 15:40 ` Rik van Riel
2012-08-23 1:10 ` Shaohua Li
0 siblings, 1 reply; 4+ messages in thread
From: Rik van Riel @ 2012-08-22 15:40 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-mm, fengguang.wu, akpm
On 08/21/2012 11:40 PM, Shaohua Li wrote:
> .fault now can retry. The retry can break state machine of .fault. In
> filemap_fault, if page is miss, ra->mmap_miss is increased. In the second try,
> since the page is in page cache now, ra->mmap_miss is decreased. And these are
> done in one fault, so we can't detect random mmap file access.
>
> Add a new flag to indicate .fault is tried once. In the second try, skip
> ra->mmap_miss decreasing. The filemap_fault state machine is ok with it.
> Index: linux/arch/avr32/mm/fault.c
> ===================================================================
> --- linux.orig/arch/avr32/mm/fault.c 2012-08-22 09:51:23.035526683 +0800
> +++ linux/arch/avr32/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
> @@ -152,6 +152,7 @@ good_area:
> tsk->min_flt++;
> if (fault & VM_FAULT_RETRY) {
> flags &= ~FAULT_FLAG_ALLOW_RETRY;
> + flags |= FAULT_FLAG_TRIED;
Is there any place where you set FAULT_FLAG_TRIED
where FAULT_FLAG_ALLOW_RETRY is not cleared?
In other words, could we use the absence of the
FAULT_FLAG_ALLOW_RETRY as the test, avoiding the
need for a new bit flag?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [patch]readahead: fault retry breaks mmap file read random detection
2012-08-22 15:40 ` Rik van Riel
@ 2012-08-23 1:10 ` Shaohua Li
2012-08-30 17:21 ` Minchan Kim
0 siblings, 1 reply; 4+ messages in thread
From: Shaohua Li @ 2012-08-23 1:10 UTC (permalink / raw)
To: Rik van Riel; +Cc: linux-mm, fengguang.wu, akpm
On Wed, Aug 22, 2012 at 11:40:33AM -0400, Rik van Riel wrote:
> On 08/21/2012 11:40 PM, Shaohua Li wrote:
> >.fault now can retry. The retry can break state machine of .fault. In
> >filemap_fault, if page is miss, ra->mmap_miss is increased. In the second try,
> >since the page is in page cache now, ra->mmap_miss is decreased. And these are
> >done in one fault, so we can't detect random mmap file access.
> >
> >Add a new flag to indicate .fault is tried once. In the second try, skip
> >ra->mmap_miss decreasing. The filemap_fault state machine is ok with it.
>
> >Index: linux/arch/avr32/mm/fault.c
> >===================================================================
> >--- linux.orig/arch/avr32/mm/fault.c 2012-08-22 09:51:23.035526683 +0800
> >+++ linux/arch/avr32/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
> >@@ -152,6 +152,7 @@ good_area:
> > tsk->min_flt++;
> > if (fault & VM_FAULT_RETRY) {
> > flags &= ~FAULT_FLAG_ALLOW_RETRY;
> >+ flags |= FAULT_FLAG_TRIED;
>
> Is there any place where you set FAULT_FLAG_TRIED
> where FAULT_FLAG_ALLOW_RETRY is not cleared?
>
> In other words, could we use the absence of the
> FAULT_FLAG_ALLOW_RETRY as the test, avoiding the
> need for a new bit flag?
There are still several archs (~7) don't enable fault retry yet. For such
archs, FAULT_FLAG_ALLOW_RETRY isn't set in the first try. If all archs support
fault retry, the new flag is unnecessary.
Thanks,
Shaohua
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [patch]readahead: fault retry breaks mmap file read random detection
2012-08-23 1:10 ` Shaohua Li
@ 2012-08-30 17:21 ` Minchan Kim
0 siblings, 0 replies; 4+ messages in thread
From: Minchan Kim @ 2012-08-30 17:21 UTC (permalink / raw)
To: Shaohua Li; +Cc: Rik van Riel, linux-mm, fengguang.wu, akpm
On Thu, Aug 23, 2012 at 09:10:03AM +0800, Shaohua Li wrote:
> On Wed, Aug 22, 2012 at 11:40:33AM -0400, Rik van Riel wrote:
> > On 08/21/2012 11:40 PM, Shaohua Li wrote:
> > >.fault now can retry. The retry can break state machine of .fault. In
> > >filemap_fault, if page is miss, ra->mmap_miss is increased. In the second try,
> > >since the page is in page cache now, ra->mmap_miss is decreased. And these are
> > >done in one fault, so we can't detect random mmap file access.
> > >
> > >Add a new flag to indicate .fault is tried once. In the second try, skip
> > >ra->mmap_miss decreasing. The filemap_fault state machine is ok with it.
> >
> > >Index: linux/arch/avr32/mm/fault.c
> > >===================================================================
> > >--- linux.orig/arch/avr32/mm/fault.c 2012-08-22 09:51:23.035526683 +0800
> > >+++ linux/arch/avr32/mm/fault.c 2012-08-22 09:52:22.822775020 +0800
> > >@@ -152,6 +152,7 @@ good_area:
> > > tsk->min_flt++;
> > > if (fault & VM_FAULT_RETRY) {
> > > flags &= ~FAULT_FLAG_ALLOW_RETRY;
> > >+ flags |= FAULT_FLAG_TRIED;
> >
> > Is there any place where you set FAULT_FLAG_TRIED
> > where FAULT_FLAG_ALLOW_RETRY is not cleared?
> >
> > In other words, could we use the absence of the
> > FAULT_FLAG_ALLOW_RETRY as the test, avoiding the
> > need for a new bit flag?
>
> There are still several archs (~7) don't enable fault retry yet. For such
> archs, FAULT_FLAG_ALLOW_RETRY isn't set in the first try. If all archs support
> fault retry, the new flag is unnecessary.
I'm not sure it's a good idea because archs support FAULT_FLAG_ALLOW_RETRY
use FAULT_FLAG_ALLOW_RETRY to avoid miscount major/minor fault accouting.
It's a similar to your goal so if you introduce new flag, major/minor fault
accounting should use your flag for the consistency, too. Otherwise,
you could be better to use FAULT_FLAG_ALLOW_RETRY but the problem is
all arch don't support it now as you mentioned. So ideal solution is that
firstly you can make all archs support FAULT_FLAG_ALLOW_RETRY(I'm not sure
it's easy or not), then use that bit flag instead of introducing new flag.
If you don't like it, I'm not strongly against with you but at least,
please write down TODO for tidy up in future.
TODO :
If all arch support FAULT_FLAG_ALLOW_RETRY in future, we can remove
FAULT_FLAG_TRIED and use FAULT_FLAG_ALLOW_RETRY to prevent misaccounting
major/minor fault and readahead mmap_miss.
>
> Thanks,
> Shaohua
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-08-30 17:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-22 3:40 [patch]readahead: fault retry breaks mmap file read random detection Shaohua Li
2012-08-22 15:40 ` Rik van Riel
2012-08-23 1:10 ` Shaohua Li
2012-08-30 17:21 ` Minchan Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).