lkml.org 
[lkml]   [2005]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: NFS client latencies

* Trond Myklebust <trond.myklebust@fys.uio.no> wrote:

> > your patch works fine here - but there are still latencies in
> > nfs_scan_commit()/nfs_scan_list(): see the attached 3.7 msec latency
> > trace. It happened during a simple big-file writeout and is easily
> > reproducible. Could the nfsi->commit list searching be replaced with a
> > radix based approach too?
>
> That would be 100% pure overhead. The nfsi->commit list does not need
> to be sorted and with these patches applied, it no longer is. In fact
> one of the cleanups I still need to do is to get rid of those
> redundant checks on wb->index (start is now always set to 0, and end
> is always ~0UL).
>
> So the overhead you are currently seeing should just be that of
> iterating through the list, locking said requests and adding them to
> our private list.

ah - cool! This was a 100 MB writeout so having 3.7 msecs to process
20K+ pages is not unreasonable. To break the latency, can i just do a
simple lock-break, via the patch below?

Ingo

--- linux/fs/nfs/pagelist.c.orig
+++ linux/fs/nfs/pagelist.c
@@ -311,8 +311,9 @@ out:
* You must be holding the inode's req_lock when calling this function
*/
int
-nfs_scan_list(struct list_head *head, struct list_head *dst,
- unsigned long idx_start, unsigned int npages)
+nfs_scan_list(struct nfs_inode *nfsi, struct list_head *head,
+ struct list_head *dst, unsigned long idx_start,
+ unsigned int npages)
{
struct list_head *pos, *tmp;
struct nfs_page *req;
@@ -327,6 +328,8 @@ nfs_scan_list(struct list_head *head, st

list_for_each_safe(pos, tmp, head) {

+ cond_resched_lock(&nfsi->req_lock);
+
req = nfs_list_entry(pos);

if (req->wb_index < idx_start)
--- linux/fs/nfs/write.c.orig
+++ linux/fs/nfs/write.c
@@ -569,7 +569,7 @@ nfs_scan_commit(struct inode *inode, str
int res = 0;

if (nfsi->ncommit != 0) {
- res = nfs_scan_list(&nfsi->commit, dst, idx_start, npages);
+ res = nfs_scan_list(nfsi, &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");
--- linux/include/linux/nfs_page.h.orig
+++ linux/include/linux/nfs_page.h
@@ -63,8 +63,8 @@ extern void nfs_release_request(struct n

extern int nfs_scan_lock_dirty(struct nfs_inode *nfsi, struct list_head *dst,
unsigned long idx_start, unsigned int npages);
-extern int nfs_scan_list(struct list_head *, struct list_head *,
- unsigned long, unsigned int);
+extern int nfs_scan_list(struct nfs_inode *nfsi, struct list_head *,
+ struct list_head *, unsigned long, unsigned int);
extern int nfs_coalesce_requests(struct list_head *, struct list_head *,
unsigned int);
extern int nfs_wait_on_request(struct nfs_page *);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2005-04-06 13:31    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site