lkml.org 
[lkml]   [2007]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] nfs: fix congestion control
    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 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: 2007-01-17 21:43    [W:0.031 / U:29.456 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site