On Tue, 16 Jan 2007, Peter Zijlstra wrote:
> The current NFS client congestion logic is severely broken, it marks the
> backing device congested during each nfs_writepages() call and implements
> its own waitqueue.
This is the magic bullet that Andrew is looking for to fix the NFS issues?
> Index: linux-2.6-git/include/linux/nfs_fs_sb.h
> ===================================================================
> --- linux-2.6-git.orig/include/linux/nfs_fs_sb.h 2007-01-12
> 08:03:47.000000000 +0100
> +++ linux-2.6-git/include/linux/nfs_fs_sb.h 2007-01-12 08:53:26.000000000
> +0100
> @@ -82,6 +82,8 @@ struct nfs_server {
> struct rpc_clnt * client_acl; /* ACL RPC client handle */
> struct nfs_iostats * io_stats; /* I/O statistics */
> struct backing_dev_info backing_dev_info;
> + atomic_t writeback; /* number of writeback pages */
> + atomic_t commit; /* number of commit pages */
> int flags; /* various flags */
I think writeback is frequently incremented? Would it be possible to avoid
a single global instance of an atomic_t here? In a busy NFS system
with lots of processors writing via NFS this may cause a hot cacheline
that limits write speed.
Would it be possible to use NR_WRITEBACK? If not then maybe add another
ZVC counter named NFS_NFS_WRITEBACK?
> Index: linux-2.6-git/mm/page-writeback.c
> ===================================================================
> --- linux-2.6-git.orig/mm/page-writeback.c 2007-01-12 08:03:47.000000000
> +0100
> +++ linux-2.6-git/mm/page-writeback.c 2007-01-12 08:53:26.000000000 +0100
> @@ -167,6 +167,12 @@ get_dirty_limits(long *pbackground, long
> *pdirty = dirty;
> }
>
> +int dirty_pages_exceeded(struct address_space *mapping)
> +{
> + return dirty_exceeded;
> +}
> +EXPORT_SYMBOL_GPL(dirty_pages_exceeded);
> +
Export the variable instead of adding a new function? Why does it take an
address space parameter that is not used?
> Index: linux-2.6-git/fs/inode.c
> ===================================================================
> --- linux-2.6-git.orig/fs/inode.c 2007-01-12 08:03:47.000000000 +0100
> +++ linux-2.6-git/fs/inode.c 2007-01-12 08:53:26.000000000 +0100
> @@ -81,6 +81,7 @@ static struct hlist_head *inode_hashtabl
> * the i_state of an inode while it is in use..
> */
> DEFINE_SPINLOCK(inode_lock);
> +EXPORT_SYMBOL_GPL(inode_lock);
Hmmm... Commits to all NFS servers will be globally serialized via the
inode_lock?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
