lkml.org 
[lkml]   [2011]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [TOME] Re: [PATCH 3/3] mm/readahead: Move the check for ra_pages after VM_SequentialReadHint()
    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




    \
     
     \ /
      Last update: 2011-07-10 18:01    [W:0.049 / U:153.444 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site