nfs: remove redundant tests on reading new pages
Subsystems (2)
PATCH 1/1 nfs: remove redundant tests on reading new pages
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().
--- 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.
fs/nfs/read.c +0 -6
@@ -533,12 +533,6 @@ readpage_async_filler(void *data, structunsigned 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);
Quoted 24 lines · Wu Fengguang
aops->readpages()and its NFS helperreadpage_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 clearPG_readahead, which is the reminder for asynchronous readahead.More background: the
PG_readaheadpage flag is shared withPG_reclaim, one for read path and the other for write path.clear_page_dirty_for_io()unconditionally clearsPG_readaheadto 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_ callsclear_page_dirty_for_io()in the read path, i.e. for readpages()/readpage().Cc Trond Myklebust <[email protected]>Signed-off-by Wu Fengguang <[email protected]>--- 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.
Quoted 8 lines
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) = 30The 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
Quoted 15 lines
fs/nfs/read.c +0
-6@@ -533,12 +533,6 @@ readpage_async_filler(void *data, structunsigned 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);