to den 31.03.2005 Klokka 09:30 (+0200) skreiv Ingo Molnar:
> > And presumably that list-based code rarely hits the worst-case, else
> > it would have been changed by now.
>
> that was my other point in a previous mail: most write benchmarks do
> continuous append writes, and CPU overhead easily gets lost in
> network
> latency.
Well. Most _good_ write benchmarks go through a variety of scenarios.
The problem with the radix-tree is that in most cases you are swapping a
linear scan through only the dirty pages (most of which you want to try
to add to the list anyway) for a linear scan through the entire list of
pages.
Damn! I've forgotten an extra _obvious_ optimization that might help
here... See the attached patch.
> Also, considering that a good portion of the NFS client's code is still
> running under the BKL one would assume if the BKL hurts performance it
> would have been changed by now? ;-)
Funny you should mention that... Chuck happens to have spent the last
fortnight working on removing our BKL-addiction. ;-)
Cheers,
Trond
--
Trond Myklebust <[EMAIL PROTECTED]>
fs/nfs/write.c | 28 +++++++++++++++++-----------
1 files changed, 17 insertions(+), 11 deletions(-)
Index: linux-2.6.12-rc1/fs/nfs/write.c
===================================================================
--- linux-2.6.12-rc1.orig/fs/nfs/write.c
+++ linux-2.6.12-rc1/fs/nfs/write.c
@@ -539,12 +539,15 @@ static int
nfs_scan_dirty(struct inode *inode, struct list_head *dst, unsigned long
idx_start, unsigned int npages)
{
struct nfs_inode *nfsi = NFS_I(inode);
- int res;
- res = nfs_scan_lock_dirty(nfsi, dst, idx_start, npages);
- nfsi->ndirty -= res;
- sub_page_state(nr_dirty,res);
- if ((nfsi->ndirty == 0) != list_empty(&nfsi->dirty))
- printk(KERN_ERR "NFS: desynchronized value of nfs_i.ndirty.\n");
+ int res = 0;
+
+ if (nfsi->ndirty != 0) {
+ res = nfs_scan_lock_dirty(nfsi, dst, idx_start, npages);
+ nfsi->ndirty -= res;
+ sub_page_state(nr_dirty,res);
+ if ((nfsi->ndirty == 0) != list_empty(&nfsi->dirty))
+ printk(KERN_ERR "NFS: desynchronized value of
nfs_i.ndirty.\n");
+ }
return res;
}
@@ -563,11 +566,14 @@ static int
nfs_scan_commit(struct inode *inode, struct list_head *dst, unsigned long
idx_start, unsigned int npages)
{
struct nfs_inode *nfsi = NFS_I(inode);
- int res;
- res = nfs_scan_list(&nfsi->commit, dst, idx_start, npages);
- nfsi->ncommit -= res;
- if ((nfsi->ncommit == 0) != list_empty(&nfsi->commit))
- printk(KERN_ERR "NFS: desynchronized value of
nfs_i.ncommit.\n");
+ int res;
+
+ if (nfsi->ncommit != 0) {
+ res = nfs_scan_list(&nfsi->commit, dst, idx_start, npages);
+ nfsi->ncommit -= res;
+ if ((nfsi->ncommit == 0) != list_empty(&nfsi->commit))
+ printk(KERN_ERR "NFS: desynchronized value of
nfs_i.ncommit.\n");
+ }
return res;
}
#endif
