* [PATCH 0/3] Readahead fixes @ 2011-07-09 19:41 ` Raghavendra D Prabhu 0 siblings, 0 replies; 21+ messages in thread From: Raghavendra D Prabhu @ 2011-07-09 19:41 UTC (permalink / raw) To: fengguang.wu; +Cc: linux-mm, linux-kernel, akpm, Raghavendra D Prabhu Minor readahead fixes, details in respective patch mails Raghavendra D Prabhu (3): Change the check for PageReadahead into an else-if Remove file_ra_state from arguments of count_history_pages. Move the check for ra_pages after VM_SequentialReadHint() mm/filemap.c | 8 ++++---- mm/readahead.c | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) -- 1.7.6 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 0/3] Readahead fixes @ 2011-07-09 19:41 ` Raghavendra D Prabhu 0 siblings, 0 replies; 21+ messages in thread From: Raghavendra D Prabhu @ 2011-07-09 19:41 UTC (permalink / raw) To: fengguang.wu; +Cc: linux-mm, linux-kernel, akpm, Raghavendra D Prabhu Minor readahead fixes, details in respective patch mails Raghavendra D Prabhu (3): Change the check for PageReadahead into an else-if Remove file_ra_state from arguments of count_history_pages. Move the check for ra_pages after VM_SequentialReadHint() mm/filemap.c | 8 ++++---- mm/readahead.c | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) -- 1.7.6 -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/3] mm/readahead: Change the check for PageReadahead into an else-if 2011-07-09 19:41 ` Raghavendra D Prabhu @ 2011-07-09 19:41 ` Raghavendra D Prabhu -1 siblings, 0 replies; 21+ messages in thread From: Raghavendra D Prabhu @ 2011-07-09 19:41 UTC (permalink / raw) To: fengguang.wu; +Cc: linux-mm, linux-kernel, akpm, Raghavendra D Prabhu >From 51daa88ebd8e0d437289f589af29d4b39379ea76, page_sync_readahead coalesces async readahead into its readahead window, so another checking for that again is not required. Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> --- mm/filemap.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index a8251a8..074c23d 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1115,8 +1115,7 @@ find_page: page = find_get_page(mapping, index); if (unlikely(page == NULL)) goto no_cached_page; - } - if (PageReadahead(page)) { + } else if (PageReadahead(page)) { page_cache_async_readahead(mapping, ra, filp, page, index, last_index - index); -- 1.7.6 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 1/3] mm/readahead: Change the check for PageReadahead into an else-if @ 2011-07-09 19:41 ` Raghavendra D Prabhu 0 siblings, 0 replies; 21+ messages in thread From: Raghavendra D Prabhu @ 2011-07-09 19:41 UTC (permalink / raw) To: fengguang.wu; +Cc: linux-mm, linux-kernel, akpm, Raghavendra D Prabhu >From 51daa88ebd8e0d437289f589af29d4b39379ea76, page_sync_readahead coalesces async readahead into its readahead window, so another checking for that again is not required. Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> --- mm/filemap.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index a8251a8..074c23d 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1115,8 +1115,7 @@ find_page: page = find_get_page(mapping, index); if (unlikely(page == NULL)) goto no_cached_page; - } - if (PageReadahead(page)) { + } else if (PageReadahead(page)) { page_cache_async_readahead(mapping, ra, filp, page, index, last_index - index); -- 1.7.6 -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] mm/readahead: Change the check for PageReadahead into an else-if 2011-07-09 19:41 ` Raghavendra D Prabhu @ 2011-07-09 20:47 ` Wu Fengguang -1 siblings, 0 replies; 21+ messages in thread From: Wu Fengguang @ 2011-07-09 20:47 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, Raghavendra D Prabhu On Sun, Jul 10, 2011 at 03:41:18AM +0800, Raghavendra D Prabhu wrote: > >From 51daa88ebd8e0d437289f589af29d4b39379ea76, page_sync_readahead coalesces > async readahead into its readahead window, so another checking for that again is > not required. > > Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> Acked-by: Wu Fengguang <fengguang.wu@intel.com> Thank you! ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] mm/readahead: Change the check for PageReadahead into an else-if @ 2011-07-09 20:47 ` Wu Fengguang 0 siblings, 0 replies; 21+ messages in thread From: Wu Fengguang @ 2011-07-09 20:47 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, Raghavendra D Prabhu On Sun, Jul 10, 2011 at 03:41:18AM +0800, Raghavendra D Prabhu wrote: > >From 51daa88ebd8e0d437289f589af29d4b39379ea76, page_sync_readahead coalesces > async readahead into its readahead window, so another checking for that again is > not required. > > Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> Acked-by: Wu Fengguang <fengguang.wu@intel.com> Thank you! -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/3] mm/readahead: Remove file_ra_state from arguments of count_history_pages. 2011-07-09 19:41 ` Raghavendra D Prabhu @ 2011-07-09 19:41 ` Raghavendra D Prabhu -1 siblings, 0 replies; 21+ messages in thread From: Raghavendra D Prabhu @ 2011-07-09 19:41 UTC (permalink / raw) To: fengguang.wu; +Cc: linux-mm, linux-kernel, akpm, Raghavendra D Prabhu count_history_pages doesn't require readahead state to calculate the offset from history. Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> --- mm/readahead.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/mm/readahead.c b/mm/readahead.c index 867f9dd..925d3b3 100644 --- a/mm/readahead.c +++ b/mm/readahead.c @@ -343,7 +343,6 @@ static unsigned long get_next_ra_size(struct file_ra_state *ra, * - thrashing threshold in memory tight systems */ static pgoff_t count_history_pages(struct address_space *mapping, - struct file_ra_state *ra, pgoff_t offset, unsigned long max) { pgoff_t head; @@ -366,7 +365,7 @@ static int try_context_readahead(struct address_space *mapping, { pgoff_t size; - size = count_history_pages(mapping, ra, offset, max); + size = count_history_pages(mapping, offset, max); /* * no history pages: -- 1.7.6 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/3] mm/readahead: Remove file_ra_state from arguments of count_history_pages. @ 2011-07-09 19:41 ` Raghavendra D Prabhu 0 siblings, 0 replies; 21+ messages in thread From: Raghavendra D Prabhu @ 2011-07-09 19:41 UTC (permalink / raw) To: fengguang.wu; +Cc: linux-mm, linux-kernel, akpm, Raghavendra D Prabhu count_history_pages doesn't require readahead state to calculate the offset from history. Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> --- mm/readahead.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/mm/readahead.c b/mm/readahead.c index 867f9dd..925d3b3 100644 --- a/mm/readahead.c +++ b/mm/readahead.c @@ -343,7 +343,6 @@ static unsigned long get_next_ra_size(struct file_ra_state *ra, * - thrashing threshold in memory tight systems */ static pgoff_t count_history_pages(struct address_space *mapping, - struct file_ra_state *ra, pgoff_t offset, unsigned long max) { pgoff_t head; @@ -366,7 +365,7 @@ static int try_context_readahead(struct address_space *mapping, { pgoff_t size; - size = count_history_pages(mapping, ra, offset, max); + size = count_history_pages(mapping, offset, max); /* * no history pages: -- 1.7.6 -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] mm/readahead: Remove file_ra_state from arguments of count_history_pages. 2011-07-09 19:41 ` Raghavendra D Prabhu @ 2011-07-09 20:47 ` Wu Fengguang -1 siblings, 0 replies; 21+ messages in thread From: Wu Fengguang @ 2011-07-09 20:47 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, Raghavendra D Prabhu On Sun, Jul 10, 2011 at 03:41:19AM +0800, Raghavendra D Prabhu wrote: > count_history_pages doesn't require readahead state to calculate the offset from history. > > Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> Acked-by: Wu Fengguang <fengguang.wu@intel.com> Thank you! ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] mm/readahead: Remove file_ra_state from arguments of count_history_pages. @ 2011-07-09 20:47 ` Wu Fengguang 0 siblings, 0 replies; 21+ messages in thread From: Wu Fengguang @ 2011-07-09 20:47 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, Raghavendra D Prabhu On Sun, Jul 10, 2011 at 03:41:19AM +0800, Raghavendra D Prabhu wrote: > count_history_pages doesn't require readahead state to calculate the offset from history. > > Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> Acked-by: Wu Fengguang <fengguang.wu@intel.com> Thank you! -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] mm/readahead: Move the check for ra_pages after VM_SequentialReadHint() 2011-07-09 19:41 ` Raghavendra D Prabhu @ 2011-07-09 19:41 ` Raghavendra D Prabhu -1 siblings, 0 replies; 21+ messages in thread From: Raghavendra D Prabhu @ 2011-07-09 19:41 UTC (permalink / raw) To: fengguang.wu; +Cc: linux-mm, linux-kernel, akpm, Raghavendra D Prabhu page_cache_sync_readahead checks for ra->ra_pages again, so moving the check after VM_SequentialReadHint. Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> --- mm/filemap.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index 074c23d..748f720 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1566,8 +1566,6 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, /* If we don't want any read-ahead, don't bother */ if (VM_RandomReadHint(vma)) return; - if (!ra->ra_pages) - return; if (VM_SequentialReadHint(vma)) { page_cache_sync_readahead(mapping, ra, file, offset, @@ -1575,6 +1573,9 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, return; } + if (!ra->ra_pages) + return; + /* Avoid banging the cache line if not needed */ if (ra->mmap_miss < MMAP_LOTSAMISS * 10) ra->mmap_miss++; -- 1.7.6 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/3] mm/readahead: Move the check for ra_pages after VM_SequentialReadHint() @ 2011-07-09 19:41 ` Raghavendra D Prabhu 0 siblings, 0 replies; 21+ messages in thread From: Raghavendra D Prabhu @ 2011-07-09 19:41 UTC (permalink / raw) To: fengguang.wu; +Cc: linux-mm, linux-kernel, akpm, Raghavendra D Prabhu page_cache_sync_readahead checks for ra->ra_pages again, so moving the check after VM_SequentialReadHint. Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> --- mm/filemap.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index 074c23d..748f720 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1566,8 +1566,6 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, /* If we don't want any read-ahead, don't bother */ if (VM_RandomReadHint(vma)) return; - if (!ra->ra_pages) - return; if (VM_SequentialReadHint(vma)) { page_cache_sync_readahead(mapping, ra, file, offset, @@ -1575,6 +1573,9 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, return; } + if (!ra->ra_pages) + return; + /* Avoid banging the cache line if not needed */ if (ra->mmap_miss < MMAP_LOTSAMISS * 10) ra->mmap_miss++; -- 1.7.6 -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] mm/readahead: Move the check for ra_pages after VM_SequentialReadHint() 2011-07-09 19:41 ` Raghavendra D Prabhu @ 2011-07-09 20:53 ` Wu Fengguang -1 siblings, 0 replies; 21+ messages in thread From: Wu Fengguang @ 2011-07-09 20:53 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, Raghavendra D Prabhu On Sun, Jul 10, 2011 at 03:41:20AM +0800, Raghavendra D Prabhu wrote: > page_cache_sync_readahead checks for ra->ra_pages again, so moving the check after VM_SequentialReadHint. NAK. This patch adds nothing but overheads. > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1566,8 +1566,6 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, > /* If we don't want any read-ahead, don't bother */ > if (VM_RandomReadHint(vma)) > return; > - if (!ra->ra_pages) > - return; > > if (VM_SequentialReadHint(vma)) { > page_cache_sync_readahead(mapping, ra, file, offset, > @@ -1575,6 +1573,9 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, > return; > } > > + if (!ra->ra_pages) > + return; > + page_cache_sync_readahead() has the same if (!ra->ra_pages) return; So the patch adds the call into page_cache_sync_readahead() just to return.. Thanks, Fengguang ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] mm/readahead: Move the check for ra_pages after VM_SequentialReadHint() @ 2011-07-09 20:53 ` Wu Fengguang 0 siblings, 0 replies; 21+ messages in thread From: Wu Fengguang @ 2011-07-09 20:53 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, Raghavendra D Prabhu On Sun, Jul 10, 2011 at 03:41:20AM +0800, Raghavendra D Prabhu wrote: > page_cache_sync_readahead checks for ra->ra_pages again, so moving the check after VM_SequentialReadHint. NAK. This patch adds nothing but overheads. > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1566,8 +1566,6 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, > /* If we don't want any read-ahead, don't bother */ > if (VM_RandomReadHint(vma)) > return; > - if (!ra->ra_pages) > - return; > > if (VM_SequentialReadHint(vma)) { > page_cache_sync_readahead(mapping, ra, file, offset, > @@ -1575,6 +1573,9 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, > return; > } > > + if (!ra->ra_pages) > + return; > + page_cache_sync_readahead() has the same if (!ra->ra_pages) return; So the patch adds the call into page_cache_sync_readahead() just to return.. Thanks, Fengguang -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [TOME] Re: [PATCH 3/3] mm/readahead: Move the check for ra_pages after VM_SequentialReadHint() 2011-07-09 20:53 ` Wu Fengguang (?) @ 2011-07-10 12:59 ` Raghavendra D Prabhu 2011-07-10 15:59 ` Wu Fengguang -1 siblings, 1 reply; 21+ messages in thread From: Raghavendra D Prabhu @ 2011-07-10 12:59 UTC (permalink / raw) To: Wu Fengguang Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org [-- Attachment #1: Type: text/plain, Size: 1650 bytes --] * On Sat, Jul 09, 2011 at 01:53:08PM -0700, Wu Fengguang <fengguang.wu@intel.com> wrote: >On Sun, Jul 10, 2011 at 03:41:20AM +0800, Raghavendra D Prabhu wrote: >> page_cache_sync_readahead checks for ra->ra_pages again, so moving the check after VM_SequentialReadHint. > >NAK. This patch adds nothing but overheads. > >> --- a/mm/filemap.c >> +++ b/mm/filemap.c >> @@ -1566,8 +1566,6 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, >> /* If we don't want any read-ahead, don't bother */ >> if (VM_RandomReadHint(vma)) >> return; >> - if (!ra->ra_pages) >> - return; >> if (VM_SequentialReadHint(vma)) { >> page_cache_sync_readahead(mapping, ra, file, offset, >> @@ -1575,6 +1573,9 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, >> return; >> } >> + if (!ra->ra_pages) >> + return; >> + > >page_cache_sync_readahead() has the same > > if (!ra->ra_pages) > return; 1. Yes, I saw that and that is why I moved it after the condition, so that duplicate checks are not needed -- ie., if VM_SequentialReadHint is true, then (!ra->ra_pages) is checked twice otherwise. 2. Also, another thought, is the check needed at its original place (if not it can be removed), reasons being -- filesystems like tmpfs which have ra_pages set to 0 don't use filemap_fault in their VMA ops and also do_sync_mmap_readahead is called in a major page fault context. > >So the patch adds the call into page_cache_sync_readahead() just to return.. > >Thanks, >Fengguang > -------------------------- Raghavendra Prabhu GPG Id : 0xD72BE977 Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 www: wnohang.net [-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [TOME] Re: [PATCH 3/3] mm/readahead: Move the check for ra_pages after VM_SequentialReadHint() 2011-07-10 12:59 ` [TOME] " Raghavendra D Prabhu @ 2011-07-10 15:59 ` Wu Fengguang 0 siblings, 0 replies; 21+ messages in thread From: Wu Fengguang @ 2011-07-10 15:59 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org On Sun, Jul 10, 2011 at 08:59:09PM +0800, Raghavendra D Prabhu wrote: > * On Sat, Jul 09, 2011 at 01:53:08PM -0700, Wu Fengguang <fengguang.wu@intel.com> wrote: > >On Sun, Jul 10, 2011 at 03:41:20AM +0800, Raghavendra D Prabhu wrote: > >>page_cache_sync_readahead checks for ra->ra_pages again, so moving the check after VM_SequentialReadHint. > > > >NAK. This patch adds nothing but overheads. > > > >>--- a/mm/filemap.c > >>+++ b/mm/filemap.c > >>@@ -1566,8 +1566,6 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, > >> /* If we don't want any read-ahead, don't bother */ > >> if (VM_RandomReadHint(vma)) > >> return; > >>- if (!ra->ra_pages) > >>- return; > > >> if (VM_SequentialReadHint(vma)) { > >> page_cache_sync_readahead(mapping, ra, file, offset, > >>@@ -1575,6 +1573,9 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, > >> return; > >> } > > >>+ if (!ra->ra_pages) > >>+ return; > >>+ > > > >page_cache_sync_readahead() has the same > > > > if (!ra->ra_pages) > > return; > 1. Yes, I saw that and that is why I moved it after the condition, so that duplicate checks are > not needed -- ie., if VM_SequentialReadHint is true, then > (!ra->ra_pages) is checked twice otherwise. Ok, I see. > 2. Also, another thought, is the check needed at its original place (if > not it can be removed), reasons being -- filesystems like tmpfs which > have ra_pages set to 0 don't use filemap_fault in their VMA ops and also Good point. tmpfs is using shmem_fault().. Can you remove the test? > do_sync_mmap_readahead is called in a major page fault context. Right. This is irrelevant however, because if pa_pages==0, the page faults will normally be major ones. Thanks, Fengguang > >So the patch adds the call into page_cache_sync_readahead() just to return.. > > > >Thanks, > >Fengguang > > > -------------------------- > Raghavendra Prabhu > GPG Id : 0xD72BE977 > Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 > www: wnohang.net ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [TOME] Re: [PATCH 3/3] mm/readahead: Move the check for ra_pages after VM_SequentialReadHint() @ 2011-07-10 15:59 ` Wu Fengguang 0 siblings, 0 replies; 21+ messages in thread From: Wu Fengguang @ 2011-07-10 15:59 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org On Sun, Jul 10, 2011 at 08:59:09PM +0800, Raghavendra D Prabhu wrote: > * On Sat, Jul 09, 2011 at 01:53:08PM -0700, Wu Fengguang <fengguang.wu@intel.com> wrote: > >On Sun, Jul 10, 2011 at 03:41:20AM +0800, Raghavendra D Prabhu wrote: > >>page_cache_sync_readahead checks for ra->ra_pages again, so moving the check after VM_SequentialReadHint. > > > >NAK. This patch adds nothing but overheads. > > > >>--- a/mm/filemap.c > >>+++ b/mm/filemap.c > >>@@ -1566,8 +1566,6 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, > >> /* If we don't want any read-ahead, don't bother */ > >> if (VM_RandomReadHint(vma)) > >> return; > >>- if (!ra->ra_pages) > >>- return; > > >> if (VM_SequentialReadHint(vma)) { > >> page_cache_sync_readahead(mapping, ra, file, offset, > >>@@ -1575,6 +1573,9 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, > >> return; > >> } > > >>+ if (!ra->ra_pages) > >>+ return; > >>+ > > > >page_cache_sync_readahead() has the same > > > > if (!ra->ra_pages) > > return; > 1. Yes, I saw that and that is why I moved it after the condition, so that duplicate checks are > not needed -- ie., if VM_SequentialReadHint is true, then > (!ra->ra_pages) is checked twice otherwise. Ok, I see. > 2. Also, another thought, is the check needed at its original place (if > not it can be removed), reasons being -- filesystems like tmpfs which > have ra_pages set to 0 don't use filemap_fault in their VMA ops and also Good point. tmpfs is using shmem_fault().. Can you remove the test? > do_sync_mmap_readahead is called in a major page fault context. Right. This is irrelevant however, because if pa_pages==0, the page faults will normally be major ones. Thanks, Fengguang > >So the patch adds the call into page_cache_sync_readahead() just to return.. > > > >Thanks, > >Fengguang > > > -------------------------- > Raghavendra Prabhu > GPG Id : 0xD72BE977 > Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 > www: wnohang.net -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re. Revised [PATCH 3/3] mm/readahead: Remove the check for ra->ra_pages 2011-07-10 15:59 ` Wu Fengguang @ 2011-07-11 23:02 ` Raghavendra D Prabhu -1 siblings, 0 replies; 21+ messages in thread From: Raghavendra D Prabhu @ 2011-07-11 23:02 UTC (permalink / raw) To: Wu Fengguang Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org * On Sun, Jul 10, 2011 at 08:59:06AM -0700, Wu Fengguang <fengguang.wu@intel.com> wrote: >On Sun, Jul 10, 2011 at 08:59:09PM +0800, Raghavendra D Prabhu wrote: >> * On Sat, Jul 09, 2011 at 01:53:08PM -0700, Wu Fengguang <fengguang.wu@intel.com> wrote: >> >On Sun, Jul 10, 2011 at 03:41:20AM +0800, Raghavendra D Prabhu wrote: >> >>page_cache_sync_readahead checks for ra->ra_pages again, so moving the check after VM_SequentialReadHint. >> >NAK. This patch adds nothing but overheads. >> >>--- a/mm/filemap.c >> >>+++ b/mm/filemap.c >> >>@@ -1566,8 +1566,6 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, >> >> /* If we don't want any read-ahead, don't bother */ >> >> if (VM_RandomReadHint(vma)) >> >> return; >> >>- if (!ra->ra_pages) >> >>- return; >> >> if (VM_SequentialReadHint(vma)) { >> >> page_cache_sync_readahead(mapping, ra, file, offset, >> >>@@ -1575,6 +1573,9 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, >> >> return; >> >> } >> >>+ if (!ra->ra_pages) >> >>+ return; >> >>+ >> >page_cache_sync_readahead() has the same >> > if (!ra->ra_pages) >> > return; >> 1. Yes, I saw that and that is why I moved it after the condition, so that duplicate checks are >> not needed -- ie., if VM_SequentialReadHint is true, then >> (!ra->ra_pages) is checked twice otherwise. > >Ok, I see. > >> 2. Also, another thought, is the check needed at its original place (if >> not it can be removed), reasons being -- filesystems like tmpfs which >> have ra_pages set to 0 don't use filemap_fault in their VMA ops and also > >Good point. tmpfs is using shmem_fault().. Can you remove the test? I have removed that test. Patch attached. > >> do_sync_mmap_readahead is called in a major page fault context. > >Right. This is irrelevant however, because if pa_pages==0, the >page faults will normally be major ones. > >Thanks, >Fengguang > >> >So the patch adds the call into page_cache_sync_readahead() just to return.. >> >Thanks, >> >Fengguang >> -------------------------- >> Raghavendra Prabhu >> GPG Id : 0xD72BE977 >> Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 >> www: wnohang.net > > ====================================================================== The check for ra->ra_pages is not required since fs like tmpfs which have ra_pages set to 0 don't use filemap_fault as part of their VMA ops (it uses shmem_fault). Also, page_cache_sync_readahead does its own check for ra_pages. Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> --- mm/filemap.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index 074c23d..0bcd276 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1566,8 +1566,6 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, /* If we don't want any read-ahead, don't bother */ if (VM_RandomReadHint(vma)) return; - if (!ra->ra_pages) - return; if (VM_SequentialReadHint(vma)) { page_cache_sync_readahead(mapping, ra, file, offset, -- 1.7.6 -------------------------- Raghavendra Prabhu GPG Id : 0xD72BE977 Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 www: wnohang.net ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re. Revised [PATCH 3/3] mm/readahead: Remove the check for ra->ra_pages @ 2011-07-11 23:02 ` Raghavendra D Prabhu 0 siblings, 0 replies; 21+ messages in thread From: Raghavendra D Prabhu @ 2011-07-11 23:02 UTC (permalink / raw) To: Wu Fengguang Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org * On Sun, Jul 10, 2011 at 08:59:06AM -0700, Wu Fengguang <fengguang.wu@intel.com> wrote: >On Sun, Jul 10, 2011 at 08:59:09PM +0800, Raghavendra D Prabhu wrote: >> * On Sat, Jul 09, 2011 at 01:53:08PM -0700, Wu Fengguang <fengguang.wu@intel.com> wrote: >> >On Sun, Jul 10, 2011 at 03:41:20AM +0800, Raghavendra D Prabhu wrote: >> >>page_cache_sync_readahead checks for ra->ra_pages again, so moving the check after VM_SequentialReadHint. >> >NAK. This patch adds nothing but overheads. >> >>--- a/mm/filemap.c >> >>+++ b/mm/filemap.c >> >>@@ -1566,8 +1566,6 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, >> >> /* If we don't want any read-ahead, don't bother */ >> >> if (VM_RandomReadHint(vma)) >> >> return; >> >>- if (!ra->ra_pages) >> >>- return; >> >> if (VM_SequentialReadHint(vma)) { >> >> page_cache_sync_readahead(mapping, ra, file, offset, >> >>@@ -1575,6 +1573,9 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, >> >> return; >> >> } >> >>+ if (!ra->ra_pages) >> >>+ return; >> >>+ >> >page_cache_sync_readahead() has the same >> > if (!ra->ra_pages) >> > return; >> 1. Yes, I saw that and that is why I moved it after the condition, so that duplicate checks are >> not needed -- ie., if VM_SequentialReadHint is true, then >> (!ra->ra_pages) is checked twice otherwise. > >Ok, I see. > >> 2. Also, another thought, is the check needed at its original place (if >> not it can be removed), reasons being -- filesystems like tmpfs which >> have ra_pages set to 0 don't use filemap_fault in their VMA ops and also > >Good point. tmpfs is using shmem_fault().. Can you remove the test? I have removed that test. Patch attached. > >> do_sync_mmap_readahead is called in a major page fault context. > >Right. This is irrelevant however, because if pa_pages==0, the >page faults will normally be major ones. > >Thanks, >Fengguang > >> >So the patch adds the call into page_cache_sync_readahead() just to return.. >> >Thanks, >> >Fengguang >> -------------------------- >> Raghavendra Prabhu >> GPG Id : 0xD72BE977 >> Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 >> www: wnohang.net > > ====================================================================== The check for ra->ra_pages is not required since fs like tmpfs which have ra_pages set to 0 don't use filemap_fault as part of their VMA ops (it uses shmem_fault). Also, page_cache_sync_readahead does its own check for ra_pages. Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> --- mm/filemap.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/mm/filemap.c b/mm/filemap.c index 074c23d..0bcd276 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1566,8 +1566,6 @@ static void do_sync_mmap_readahead(struct vm_area_struct *vma, /* If we don't want any read-ahead, don't bother */ if (VM_RandomReadHint(vma)) return; - if (!ra->ra_pages) - return; if (VM_SequentialReadHint(vma)) { page_cache_sync_readahead(mapping, ra, file, offset, -- 1.7.6 -------------------------- Raghavendra Prabhu GPG Id : 0xD72BE977 Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 www: wnohang.net -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Re. Revised [PATCH 3/3] mm/readahead: Remove the check for ra->ra_pages 2011-07-11 23:02 ` Raghavendra D Prabhu @ 2011-07-12 18:09 ` Wu Fengguang -1 siblings, 0 replies; 21+ messages in thread From: Wu Fengguang @ 2011-07-12 18:09 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org > The check for ra->ra_pages is not required since fs like tmpfs which have > ra_pages set to 0 don't use filemap_fault as part of their VMA ops (it uses > shmem_fault). Also, page_cache_sync_readahead does its own check for ra_pages. > > Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> Acked-by: Wu Fengguang <fengguang.wu@intel.com> Thank you! ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Re. Revised [PATCH 3/3] mm/readahead: Remove the check for ra->ra_pages @ 2011-07-12 18:09 ` Wu Fengguang 0 siblings, 0 replies; 21+ messages in thread From: Wu Fengguang @ 2011-07-12 18:09 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org > The check for ra->ra_pages is not required since fs like tmpfs which have > ra_pages set to 0 don't use filemap_fault as part of their VMA ops (it uses > shmem_fault). Also, page_cache_sync_readahead does its own check for ra_pages. > > Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> Acked-by: Wu Fengguang <fengguang.wu@intel.com> Thank you! -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2011-07-12 18:10 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-07-09 19:41 [PATCH 0/3] Readahead fixes Raghavendra D Prabhu 2011-07-09 19:41 ` Raghavendra D Prabhu 2011-07-09 19:41 ` [PATCH 1/3] mm/readahead: Change the check for PageReadahead into an else-if Raghavendra D Prabhu 2011-07-09 19:41 ` Raghavendra D Prabhu 2011-07-09 20:47 ` Wu Fengguang 2011-07-09 20:47 ` Wu Fengguang 2011-07-09 19:41 ` [PATCH 2/3] mm/readahead: Remove file_ra_state from arguments of count_history_pages Raghavendra D Prabhu 2011-07-09 19:41 ` Raghavendra D Prabhu 2011-07-09 20:47 ` Wu Fengguang 2011-07-09 20:47 ` Wu Fengguang 2011-07-09 19:41 ` [PATCH 3/3] mm/readahead: Move the check for ra_pages after VM_SequentialReadHint() Raghavendra D Prabhu 2011-07-09 19:41 ` Raghavendra D Prabhu 2011-07-09 20:53 ` Wu Fengguang 2011-07-09 20:53 ` Wu Fengguang 2011-07-10 12:59 ` [TOME] " Raghavendra D Prabhu 2011-07-10 15:59 ` Wu Fengguang 2011-07-10 15:59 ` Wu Fengguang 2011-07-11 23:02 ` Re. Revised [PATCH 3/3] mm/readahead: Remove the check for ra->ra_pages Raghavendra D Prabhu 2011-07-11 23:02 ` Raghavendra D Prabhu 2011-07-12 18:09 ` Wu Fengguang 2011-07-12 18:09 ` Wu Fengguang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.