lkml.org 
[lkml]   [2008]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] nfs: remove redundant tests on reading new pages
On Mon, Dec 01, 2008 at 11:19:22AM +0800, Wu Fengguang wrote:
> aops->readpages() and its NFS helper readpage_async_filler() will only
> be called to do readahead I/O for newly allocated pages. So it's not
> necessary to test for the always 0 dirty/uptodate page flags.
>
> The removal of nfs_wb_page() call also fixes a readahead bug: the NFS
> readahead has been synchronous since 2.6.23, because that call will
> clear PG_readahead, which is the reminder for asynchronous readahead.
>
> More background: the PG_readahead page flag is shared with PG_reclaim,
> one for read path and the other for write path. clear_page_dirty_for_io()
> unconditionally clears PG_readahead to prevent possible readahead residuals,
> assuming itself to be always called in the write path. However, NFS is one
> and the only exception in that it _always_ calls clear_page_dirty_for_io()
> in the read path, i.e. for readpages()/readpage().
>
> Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
> Signed-off-by: Wu Fengguang <wfg@linux.intel.com>
> ---
> fs/nfs/read.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> This bug was found when playing with my readahead tracing module posted at
> http://lkml.org/lkml/2008/11/27/373 and confirmed to work.
>
> The NFS readahead traces are now
>
> [ 59.972526] readahead-initial0(pid=3646(dd), dev=00:0f(0:f), ino=293189(1G), req=0+25, ra=0+60-30, async=0) = 60
> [ 59.992734] readahead-subsequent(pid=3646(dd), dev=00:0f(0:f), ino=293189(1G), req=30+20, ra=60+30-30, async=1) = 30
> [ 60.030900] readahead-subsequent(pid=3646(dd), dev=00:0f(0:f), ino=293189(1G), req=60+15, ra=90+30-30, async=1) = 30
> [ 60.031687] readahead-subsequent(pid=3646(dd), dev=00:0f(0:f), ino=293189(1G), req=90+10, ra=120+30-30, async=1) = 30
>
> The async field was always 0 before this patch.

On the trace messages: they are generated by the command
'dd if=/tmp/sparse/1G of=/dev/null bs=100k count=9' with readahead_size=120k.

The printk for generating the messages is:

printk(KERN_DEBUG "readahead-%s(pid=%d(%s), dev=%02x:%02x(%s), "
"ino=%lu(%s), req=%lu+%lu, ra=%lu+%d-%d, async=%d) = %d\n",
ra_pattern_names[pattern],
current->pid, current->comm,
MAJOR(mapping->host->i_sb->s_dev),
MINOR(mapping->host->i_sb->s_dev),
mapping->host->i_sb->s_id,
mapping->host->i_ino,
filp->f_path.dentry->d_name.name,
offset, req_size,
ra->start, ra->size, ra->async_size,
async,
actual);

Thanks,
Fengguang

> --- linux-2.6.orig/fs/nfs/read.c
> +++ linux-2.6/fs/nfs/read.c
> @@ -533,12 +533,6 @@ readpage_async_filler(void *data, struct
> unsigned int len;
> int error;
>
> - error = nfs_wb_page(inode, page);
> - if (error)
> - goto out_unlock;
> - if (PageUptodate(page))
> - goto out_unlock;
> -
> len = nfs_page_length(page);
> if (len == 0)
> return nfs_return_empty_page(page);


\
 
 \ /
  Last update: 2008-12-01 04:33    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site