linux-kernel - Re: [PATCH] nfs: fix congestion control

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 17 Jan 2007 12:05:53 -0800 (PST)
From:	Christoph Lameter <clameter@....com>
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
cc:	Andrew Morton <akpm@...l.org>, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org, Trond Myklebust <trond.myklebust@....uio.no>
Subject: Re: [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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/