[PATCH 2.6.6rc1-mm1] NFS sysctlized - readahead tunable

[PATCH 2.6.6rc1-mm1] NFS sysctlized - readahead tunable

[Fabian Frederick]

[-- Attachment #1: Type: text/plain, Size: 289 bytes --]

Trond,

	Here is a patch to have nfs to sysctl although Maxreadahead is tunable
under nfs init only.Do you have an idea and do you think it's acceptable
to make it applicable directly i.e. would it be readahead reduction
tolerant ?

btw, is this inode.c an issue for V4 ?

Regards,
Fabian

[-- Attachment #2: nfsctl.diff --]
[-- Type: text/x-patch, Size: 3711 bytes --]

diff -Naur orig/fs/nfs/inode.c edited/fs/nfs/inode.c
--- orig/fs/nfs/inode.c	2004-04-19 20:27:30.000000000 +0200
+++ edited/fs/nfs/inode.c	2004-04-19 22:32:14.000000000 +0200
@@ -11,6 +11,10 @@
  *  Change to nfs_read_super() to permit NFS mounts to multi-homed hosts.
  *  J.S.Peatfield@damtp.cam.ac.uk
  *
+ *  04/04 (FabianF)
+ *	-sysctl support
+ *	-readahead through sysctl
+ *
  */
 
 #include <linux/config.h>
@@ -42,13 +46,8 @@
 #define NFSDBG_FACILITY		NFSDBG_VFS
 #define NFS_PARANOIA 1
 
-/* Maximum number of readahead requests
- * FIXME: this should really be a sysctl so that users may tune it to suit
- *        their needs. People that do NFS over a slow network, might for
- *        instance want to reduce it to something closer to 1 for improved
- *        interactive response.
- */
-#define NFS_MAX_READAHEAD	(RPC_DEF_SLOT_TABLE - 1)
+/*Local sysctl handling*/
+#include "sysctl.h"
 
 static void nfs_invalidate_inode(struct inode *);
 static int nfs_update_inode(struct inode *, struct nfs_fattr *, unsigned long);
@@ -326,7 +325,7 @@
 		server->acdirmin = server->acdirmax = 0;
 		sb->s_flags |= MS_SYNCHRONOUS;
 	}
-	server->backing_dev_info.ra_pages = server->rpages * NFS_MAX_READAHEAD;
+	server->backing_dev_info.ra_pages = server->rpages * nfs_maxreadahead;
 
 	sb->s_maxbytes = fsinfo.maxfilesize;
 	if (sb->s_maxbytes > MAX_LFS_FILESIZE) 
@@ -1814,6 +1813,12 @@
 #ifdef CONFIG_PROC_FS
 	rpc_proc_register(&nfs_rpcstat);
 #endif
+#ifdef CONFIG_SYSCTL
+	nfs_sysctl_table = register_sysctl_table(nfs_sysctl_root, 0);
+	if(!nfs_sysctl_table)
+		return -ENOMEM;
+#endif
+
         err = register_filesystem(&nfs_fs_type);
 	if (err)
 		goto out;
@@ -1844,6 +1849,10 @@
 #endif
 	unregister_filesystem(&nfs_fs_type);
 	unregister_nfs4fs();
+#ifdef CONFIG_SYSCTL 
+	unregister_sysctl_table(nfs_sysctl_table);
+#endif
+
 }
 
 /* Not quite true; I just maintain it */
diff -Naur orig/fs/nfs/sysctl.h edited/fs/nfs/sysctl.h
--- orig/fs/nfs/sysctl.h	1970-01-01 01:00:00.000000000 +0100
+++ edited/fs/nfs/sysctl.h	2004-04-19 22:23:21.000000000 +0200
@@ -0,0 +1,56 @@
+/*
+ * sysctl.h - NFS sysctl handling
+ * 
+ * Copyright (c) 2004 Fabian Frederick (fabian.frederick@gmx.fr)
+ *
+ */
+
+#ifndef _LINUX_NFS_SYSCTL_H
+#define _LINUX_NFS_SYSCTL_H
+
+#include <linux/sysctl.h>
+static unsigned int 		nfs_maxreadahead = RPC_DEF_SLOT_TABLE-1;
+
+static const unsigned int	nfs_maxreadahead_min = 0;
+static const unsigned int	nfs_maxreadahead_max = 256;
+
+#ifdef CONFIG_SYSCTL
+static struct ctl_table_header *nfs_sysctl_table;
+
+#define CTL_UNNUMBERED		-2
+
+static ctl_table nfs_sysctls[] = {
+        {
+                .ctl_name       = CTL_UNNUMBERED,
+                .procname       = "nfs_maxreadahead",
+                .data           = &nfs_maxreadahead,
+                .maxlen         = sizeof(int),
+                .mode           = 0644,
+                .proc_handler   = &proc_doulongvec_minmax,
+                .extra1         = (unsigned long *) &nfs_maxreadahead_min,
+                .extra2         = (unsigned long *) &nfs_maxreadahead_max,
+        }
+};
+
+static ctl_table nfs_sysctl_dir[] = {
+        {
+                .ctl_name       = CTL_UNNUMBERED,
+                .procname       = "nfs",
+                .mode           = 0555,
+                .child          = nfs_sysctls,
+        },
+        { .ctl_name = 0 }
+};
+
+static ctl_table nfs_sysctl_root[] = {
+        {
+                .ctl_name       = CTL_FS,
+                .procname       = "fs",
+                .mode           = 0555,
+                .child          = nfs_sysctl_dir,
+        },
+        { .ctl_name = 0 }
+};
+
+#endif /* CONFIG_SYSCTL */
+#endif /* _LINUX_NFS_SYSCTL_H */

Re: [PATCH 2.6.6rc1-mm1] NFS sysctlized - readahead tunable

[Trond Myklebust]

On Mon, 2004-04-19 at 16:49, Fabian Frederick wrote:
> Trond,
> 
> 	Here is a patch to have nfs to sysctl although Maxreadahead is tunable
> under nfs init only.Do you have an idea and do you think it's acceptable
> to make it applicable directly i.e. would it be readahead reduction
> tolerant ?
> 
> btw, is this inode.c an issue for V4 ?
> 

The lockd module has already registered the name /proc/sys/fs/nfs, so
your scheme will end up corrupting the sysctl list. Sorting out the
/proc namespace issue is the main reason why this hasn't been done
before.
Personally, I'd prefer renaming the lockd module into
/proc/sys/fs/lockd, but it will have to be up to Andrew to decide
whether he wants to allow that during a stable kernel cycle.

Also note that putting initializers into a ".h" file is horrible style.
".h" files should be for forward declarations only.

Cheers,
  Trond

Re: [PATCH 2.6.6rc1-mm1] NFS sysctlized - readahead tunable

[Fabian Frederick]

On Mon, 2004-04-19 at 23:08, Trond Myklebust wrote:
> On Mon, 2004-04-19 at 16:49, Fabian Frederick wrote:
> > Trond,
> > 
> > 	Here is a patch to have nfs to sysctl although Maxreadahead is tunable
> > under nfs init only.Do you have an idea and do you think it's acceptable
> > to make it applicable directly i.e. would it be readahead reduction
> > tolerant ?
> > 
> > btw, is this inode.c an issue for V4 ?
> > 
> 
> The lockd module has already registered the name /proc/sys/fs/nfs, so
> your scheme will end up corrupting the sysctl list. Sorting out the
> /proc namespace issue is the main reason why this hasn't been done
> before.
> Personally, I'd prefer renaming the lockd module into
> /proc/sys/fs/lockd, but it will have to be up to Andrew to decide
> whether he wants to allow that during a stable kernel cycle.
AFAICS one of the first sysctl concepts is to be redundancy tolerant.
If you take fs for instance, it's being declared here and there.
It means "if it doens't exist yet, we create it with mode (555) else
 go cycle to the sub-branch (fs-nfs....) btw, we have a register(fs) so
no problem for me.Lockd has to do with nfs so it should be preserved at
the same place IMHO.

> 
> Also note that putting initializers into a ".h" file is horrible style.
> ".h" files should be for forward declarations only.
I used both ntfs, coda fs scheme having in mind independant sysctl
registering in forthcoming releases.

But hey ! "I'm an absolute beginner" :) Maybe you and Andrew can tell me
what to do with this ugly patch ;) e.g. no sysctl.h -> include stuff in
inode.c ...

Regards,
FabianF

Re: [PATCH 2.6.6rc1-mm1] NFS sysctlized - readahead tunable

[Trond Myklebust]

On Mon, 2004-04-19 at 17:29, Fabian Frederick wrote:
> But hey ! "I'm an absolute beginner" :) Maybe you and Andrew can tell me
> what to do with this ugly patch ;) e.g. no sysctl.h -> include stuff in
> inode.c ...

Yes.

Re: [PATCH 2.6.6rc1-mm1] NFS sysctlized - readahead tunable

[FabianF]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

On Tue, 2004-04-20 at 02:01, Trond Myklebust wrote:
> On Mon, 2004-04-19 at 17:29, Fabian Frederick wrote:
> > But hey ! "I'm an absolute beginner" :) Maybe you and Andrew can tell me
> > what to do with this ugly patch ;) e.g. no sysctl.h -> include stuff in
> > inode.c ...
> 
> Yes.
This 'superbeast in time' patch against same patchset:
	-Type fixes
	-All in inode.c

Regards,
Fabian
> 

[-- Attachment #2: nfsctl2.diff --]
[-- Type: text/x-patch, Size: 3238 bytes --]

diff -Naur orig/fs/nfs/inode.c edited/fs/nfs/inode.c
--- orig/fs/nfs/inode.c	2004-04-19 20:27:30.000000000 +0200
+++ edited/fs/nfs/inode.c	2004-04-20 08:05:48.000000000 +0200
@@ -11,6 +11,10 @@
  *  Change to nfs_read_super() to permit NFS mounts to multi-homed hosts.
  *  J.S.Peatfield@damtp.cam.ac.uk
  *
+ *  April 2004 : Fabian Frederick
+ *		 -Add sysctl
+ *		 -maxreadahead to sysctl
+ *
  */
 
 #include <linux/config.h>
@@ -42,13 +46,52 @@
 #define NFSDBG_FACILITY		NFSDBG_VFS
 #define NFS_PARANOIA 1
 
-/* Maximum number of readahead requests
- * FIXME: this should really be a sysctl so that users may tune it to suit
- *        their needs. People that do NFS over a slow network, might for
- *        instance want to reduce it to something closer to 1 for improved
- *        interactive response.
- */
-#define NFS_MAX_READAHEAD	(RPC_DEF_SLOT_TABLE - 1)
+#include <linux/sysctl.h>
+
+static unsigned int 		nfs_maxreadahead = RPC_DEF_SLOT_TABLE-1;
+
+static const unsigned int	nfs_maxreadahead_min = 0;
+static const unsigned int	nfs_maxreadahead_max = 256;
+
+#ifdef CONFIG_SYSCTL
+static struct ctl_table_header *nfs_sysctl_table;
+
+#define CTL_UNNUMBERED		-2
+
+static ctl_table nfs_sysctls[] = {
+        {
+                .ctl_name       = CTL_UNNUMBERED,
+                .procname       = "nfs_maxreadahead",
+                .data           = &nfs_maxreadahead,
+                .maxlen         = sizeof(int),
+                .mode           = 0644,
+                .proc_handler   = &proc_dointvec_minmax,
+                .extra1         = (unsigned int *) &nfs_maxreadahead_min,
+                .extra2         = (unsigned int *) &nfs_maxreadahead_max,
+        }
+};
+
+static ctl_table nfs_sysctl_dir[] = {
+        {
+                .ctl_name       = CTL_UNNUMBERED,
+                .procname       = "nfs",
+                .mode           = 0555,
+                .child          = nfs_sysctls,
+        },
+        { .ctl_name = 0 }
+};
+
+static ctl_table nfs_sysctl_root[] = {
+        {
+                .ctl_name       = CTL_FS,
+                .procname       = "fs",
+                .mode           = 0555,
+                .child          = nfs_sysctl_dir,
+        },
+        { .ctl_name = 0 }
+};
+
+#endif
 
 static void nfs_invalidate_inode(struct inode *);
 static int nfs_update_inode(struct inode *, struct nfs_fattr *, unsigned long);
@@ -326,7 +369,7 @@
 		server->acdirmin = server->acdirmax = 0;
 		sb->s_flags |= MS_SYNCHRONOUS;
 	}
-	server->backing_dev_info.ra_pages = server->rpages * NFS_MAX_READAHEAD;
+	server->backing_dev_info.ra_pages = server->rpages * nfs_maxreadahead;
 
 	sb->s_maxbytes = fsinfo.maxfilesize;
 	if (sb->s_maxbytes > MAX_LFS_FILESIZE) 
@@ -1814,6 +1857,11 @@
 #ifdef CONFIG_PROC_FS
 	rpc_proc_register(&nfs_rpcstat);
 #endif
+#ifdef CONFIG_SYSCTL
+	nfs_sysctl_table = register_sysctl_table(nfs_sysctl_root, 0);
+	if(!nfs_sysctl_table)
+		return -ENOMEM;
+#endif
         err = register_filesystem(&nfs_fs_type);
 	if (err)
 		goto out;
@@ -1844,6 +1892,9 @@
 #endif
 	unregister_filesystem(&nfs_fs_type);
 	unregister_nfs4fs();
+#ifdef CONFIG_SYSCTL 
+	unregister_sysctl_table(nfs_sysctl_table);
+#endif
 }
 
 /* Not quite true; I just maintain it */