From: [email protected]
Subject: Add /proc/sys/fs/nfs sysctls to nfsd module
This patch adds the plumbing for adding nfs-specific sysctls to fs/nfs,
and makes the max size of a readdirplus reply tunable.
The reason for this is that we found that some clients do not seem
to grok readdirplus replies larger than 4K. Rather than reducing the
overall number this sysctl allows admins to work around these clients.
Signed-off-by: Olaf Kirch <[email protected]>
fs/nfsd/nfs3xdr.c | 5 ++++-
fs/nfsd/nfsctl.c | 28 ++++++++++++++++++++++++++++
include/linux/nfsd/xdr3.h | 4 ++++
3 files changed, 36 insertions(+), 1 deletion(-)
Index: linux-2.6.18/fs/nfsd/nfsctl.c
===================================================================
--- linux-2.6.18.orig/fs/nfsd/nfsctl.c
+++ linux-2.6.18/fs/nfsd/nfsctl.c
@@ -31,6 +31,7 @@
#include <linux/nfsd/nfsd.h>
#include <linux/nfsd/cache.h>
#include <linux/nfsd/xdr.h>
+#include <linux/nfsd/xdr3.h>
#include <linux/nfsd/syscall.h>
#include <linux/nfsd/interface.h>
@@ -507,8 +508,28 @@ static struct file_system_type nfsd_fs_t
.kill_sb = kill_litter_super,
};
+ /*
+ * NFS sysctls
+ */
+static struct ctl_table_header *nfsd_sysctl_table;
+
+static ctl_table nfsd_sysctls[] = {
+ {
+ .ctl_name = -2,
+ .procname = "nfsd_readdirplus_max",
+ .data = &nfsd_readdirplus_max,
+ .maxlen = sizeof(nfsd_readdirplus_max),
+ .mode = 0644,
+ .proc_handler = &proc_dointvec_minmax,
+ .extra1 = &nfsd_readdirplus_max_lb,
+ .extra2 = &nfsd_readdirplus_max_ub,
+ },
+ { .ctl_name = 0 }
+};
+
static int __init init_nfsd(void)
{
+ struct ctl_path ctl_path[] = { { CTL_FS, "fs", 0555 }, { -2, "nfs", 0555 }, { 0 } };
int retval;
printk(KERN_INFO "Installing knfsd (copyright (C) 1996 [email protected]).\n");
@@ -518,6 +539,7 @@ static int __init init_nfsd(void)
nfsd_lockd_init(); /* lockd->nfsd callbacks */
nfs4_state_init(); /* NFSv4 locking state */
nfsd_idmap_init(); /* Name to ID mapping */
+ nfsd_sysctl_table = register_sysctl_table_path(nfsd_sysctls, ctl_path);
if (proc_mkdir("fs/nfs", NULL)) {
struct proc_dir_entry *entry;
entry = create_proc_entry("fs/nfs/exports", 0, NULL);
@@ -532,6 +554,9 @@ static int __init init_nfsd(void)
remove_proc_entry("fs/nfs", NULL);
nfsd_stat_shutdown();
nfsd_lockd_shutdown();
+ if (nfsd_sysctl_table)
+ unregister_sysctl_table(nfsd_sysctl_table);
+ nfsd_sysctl_table = NULL;
}
return retval;
}
@@ -546,6 +571,9 @@ static void __exit exit_nfsd(void)
nfsd_lockd_shutdown();
nfsd_idmap_shutdown();
unregister_filesystem(&nfsd_fs_type);
+ if (nfsd_sysctl_table)
+ unregister_sysctl_table(nfsd_sysctl_table);
+ nfsd_sysctl_table = NULL;
}
MODULE_AUTHOR("Olaf Kirch <[email protected]>");
Index: linux-2.6.18/fs/nfsd/nfs3xdr.c
===================================================================
--- linux-2.6.18.orig/fs/nfsd/nfs3xdr.c
+++ linux-2.6.18/fs/nfsd/nfs3xdr.c
@@ -28,6 +28,9 @@
# define inline
#endif
+unsigned int nfsd_readdirplus_max = NFSSVC_MAXBLKSIZE;
+unsigned int nfsd_readdirplus_max_lb = 512;
+unsigned int nfsd_readdirplus_max_ub = NFSSVC_MAXBLKSIZE;
/*
* Mapping of S_IF* types to NFS file types
@@ -573,7 +576,7 @@ nfs3svc_decode_readdirplusargs(struct sv
args->dircount = ntohl(*p++);
args->count = ntohl(*p++);
- len = (args->count > NFSSVC_MAXBLKSIZE) ? NFSSVC_MAXBLKSIZE :
+ len = (args->count > nfsd_readdirplus_max) ? nfsd_readdirplus_max :
args->count;
args->count = len;
Index: linux-2.6.18/include/linux/nfsd/xdr3.h
===================================================================
--- linux-2.6.18.orig/include/linux/nfsd/xdr3.h
+++ linux-2.6.18/include/linux/nfsd/xdr3.h
@@ -268,6 +268,10 @@ union nfsd3_xdrstore {
#define NFS3_SVC_XDRSIZE sizeof(union nfsd3_xdrstore)
+extern unsigned int nfsd_readdirplus_max;
+extern unsigned int nfsd_readdirplus_max_ub;
+extern unsigned int nfsd_readdirplus_max_lb;
+
int nfs3svc_decode_fhandle(struct svc_rqst *, u32 *, struct nfsd_fhandle *);
int nfs3svc_decode_sattrargs(struct svc_rqst *, u32 *,
struct nfsd3_sattrargs *);
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Fri, 2006-08-04 at 18:43 +1000, Greg Banks wrote:
> On Fri, 2006-08-04 at 02:52, Chuck Lever wrote:
> > On 8/3/06, Olaf Kirch <[email protected]> wrote:
> > > From: [email protected]
> > > Subject: Add /proc/sys/fs/nfs sysctls to nfsd module
> > >
> > > This patch adds the plumbing for adding nfs-specific sysctls to fs/nfs,
> > > and makes the max size of a readdirplus reply tunable.
> > >
> > > The reason for this is that we found that some clients do not seem
> > > to grok readdirplus replies larger than 4K. Rather than reducing the
> > > overall number this sysctl allows admins to work around these clients.
>
> Neat.
Not really. It would be better to identify and fix the buggy clients
that aren't setting the READDIRPLUS maxcount parameter correctly.
Cheers,
Trond
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Fri, Aug 04, 2006 at 11:36:37AM -0400, Trond Myklebust wrote:
> Not really. It would be better to identify and fix the buggy clients
> that aren't setting the READDIRPLUS maxcount parameter correctly.
One of them was Solaris 9, IIRC. Don't recall what the other one was.
I contacted at least Sun, but that didn't go anywhere.
Olaf
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
[email protected] | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
> Note, however, that the entire sysctl should be unnecessary. Clients are
> supposed to tell the server how large a reply to return, by means of the
> 'maxcount' argument (see RFC1813). If the clients aren't doing that,
> then they really need to be fixed.
>From a distributor's point of view, waiting for the client vendor to
fix their bugs is often not the best way to go. People buying support
contracts expect a fix, no matter buy whom, they only want it NOW :-)
But I understand if you reject this for upstream.
Olaf
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
[email protected] | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Fri, 2006-08-04 at 19:52 +0200, Olaf Kirch wrote:
> On Fri, Aug 04, 2006 at 11:36:37AM -0400, Trond Myklebust wrote:
> > Not really. It would be better to identify and fix the buggy clients
> > that aren't setting the READDIRPLUS maxcount parameter correctly.
>
> One of them was Solaris 9, IIRC. Don't recall what the other one was.
> I contacted at least Sun, but that didn't go anywhere.
Yeah.... Solaris appears to have a warped design in their READDIR code.
They want to specify one size for the actual READDIR reply, and one size
for all the extra READDIRPLUS gunk (see pages 209 and 210 in Brent's
'NFS Illustrated').
In their case, you want to look at the 'dircount' parameter, and
truncate the readdirplus reply when you hit 'dircount' bytes of filename
+fileid information. After that, you have to look at the total XDR
information, and truncate again when you hit 'maxcount' bytes.
Sucks, sucks, sucks....
Trond
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
Trond Myklebust wrote:
> On Fri, 2006-08-04 at 19:52 +0200, Olaf Kirch wrote:
>
>> On Fri, Aug 04, 2006 at 11:36:37AM -0400, Trond Myklebust wrote:
>>
>>> Not really. It would be better to identify and fix the buggy clients
>>> that aren't setting the READDIRPLUS maxcount parameter correctly.
>>>
>> One of them was Solaris 9, IIRC. Don't recall what the other one was.
>> I contacted at least Sun, but that didn't go anywhere.
>>
>
> Yeah.... Solaris appears to have a warped design in their READDIR code.
> They want to specify one size for the actual READDIR reply, and one size
> for all the extra READDIRPLUS gunk (see pages 209 and 210 in Brent's
> 'NFS Illustrated').
> In their case, you want to look at the 'dircount' parameter, and
> truncate the readdirplus reply when you hit 'dircount' bytes of filename
> +fileid information. After that, you have to look at the total XDR
> information, and truncate again when you hit 'maxcount' bytes.
>
> Sucks, sucks, sucks....
Actually, the Solaris implementation matches the intended semantics as
described in RFC1813. The number of bytes of requested information
via readdir(3) is passed via dircount and then the maximum size of the
response is passed through maxcount.
The two fields were intended to be used to reduce the overhead on
the server by passing a hint for the amount of data that the client
was actually looking for and then a count for the maximum sized response
that it was prepared to handle.
Solaris probably uses something like 1048/32768 or 8192/32768, doesn't
it?
Thanx...
ps
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Fri, Aug 04, 2006 at 03:16:40PM -0400, Peter Staubach wrote:
> The two fields were intended to be used to reduce the overhead on
> the server by passing a hint for the amount of data that the client
> was actually looking for and then a count for the maximum sized response
> that it was prepared to handle.
>
> Solaris probably uses something like 1048/32768 or 8192/32768, doesn't
> it?
I don't remember the values, all I know was that it got stuck in
READDIRPLUS, repeating the same call over and over again. And it can't
have been that many directory entries (definitely not in the thousands),
and IIRC we didn't hit the maxcount limit.
Olaf
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
[email protected] | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Fri, 2006-08-04 at 15:16 -0400, Peter Staubach wrote:
> > Sucks, sucks, sucks....
>
> Actually, the Solaris implementation matches the intended semantics as
> described in RFC1813. The number of bytes of requested information
> via readdir(3) is passed via dircount and then the maximum size of the
> response is passed through maxcount.
>
> The two fields were intended to be used to reduce the overhead on
> the server by passing a hint for the amount of data that the client
> was actually looking for and then a count for the maximum sized response
> that it was prepared to handle.
Right. At first glance a good idea, but in practice it sucks because the
client has no idea of how much data it was looking for. In order to
know, it would need to know the length of the filenames in the
directory. Unless it is already caching the directory, that will be 100%
guesswork.
Cheers,
Trond
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On 8/3/06, Olaf Kirch <[email protected]> wrote:
> From: [email protected]
> Subject: Add /proc/sys/fs/nfs sysctls to nfsd module
>
> This patch adds the plumbing for adding nfs-specific sysctls to fs/nfs,
> and makes the max size of a readdirplus reply tunable.
>
> The reason for this is that we found that some clients do not seem
> to grok readdirplus replies larger than 4K. Rather than reducing the
> overall number this sysctl allows admins to work around these clients.
>
> Signed-off-by: Olaf Kirch <[email protected]>
>
> fs/nfsd/nfs3xdr.c | 5 ++++-
> fs/nfsd/nfsctl.c | 28 ++++++++++++++++++++++++++++
> include/linux/nfsd/xdr3.h | 4 ++++
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.18/fs/nfsd/nfsctl.c
> ===================================================================
> --- linux-2.6.18.orig/fs/nfsd/nfsctl.c
> +++ linux-2.6.18/fs/nfsd/nfsctl.c
> @@ -31,6 +31,7 @@
> #include <linux/nfsd/nfsd.h>
> #include <linux/nfsd/cache.h>
> #include <linux/nfsd/xdr.h>
> +#include <linux/nfsd/xdr3.h>
> #include <linux/nfsd/syscall.h>
> #include <linux/nfsd/interface.h>
>
> @@ -507,8 +508,28 @@ static struct file_system_type nfsd_fs_t
> .kill_sb = kill_litter_super,
> };
>
> + /*
> + * NFS sysctls
> + */
> +static struct ctl_table_header *nfsd_sysctl_table;
> +
> +static ctl_table nfsd_sysctls[] = {
> + {
> + .ctl_name = -2,
> + .procname = "nfsd_readdirplus_max",
> + .data = &nfsd_readdirplus_max,
> + .maxlen = sizeof(nfsd_readdirplus_max),
> + .mode = 0644,
> + .proc_handler = &proc_dointvec_minmax,
> + .extra1 = &nfsd_readdirplus_max_lb,
> + .extra2 = &nfsd_readdirplus_max_ub,
> + },
> + { .ctl_name = 0 }
> +};
> +
> static int __init init_nfsd(void)
> {
> + struct ctl_path ctl_path[] = { { CTL_FS, "fs", 0555 }, { -2, "nfs", 0555 }, { 0 } };
> int retval;
> printk(KERN_INFO "Installing knfsd (copyright (C) 1996 [email protected]).\n");
>
> @@ -518,6 +539,7 @@ static int __init init_nfsd(void)
> nfsd_lockd_init(); /* lockd->nfsd callbacks */
> nfs4_state_init(); /* NFSv4 locking state */
> nfsd_idmap_init(); /* Name to ID mapping */
> + nfsd_sysctl_table = register_sysctl_table_path(nfsd_sysctls, ctl_path);
> if (proc_mkdir("fs/nfs", NULL)) {
> struct proc_dir_entry *entry;
> entry = create_proc_entry("fs/nfs/exports", 0, NULL);
> @@ -532,6 +554,9 @@ static int __init init_nfsd(void)
> remove_proc_entry("fs/nfs", NULL);
> nfsd_stat_shutdown();
> nfsd_lockd_shutdown();
> + if (nfsd_sysctl_table)
> + unregister_sysctl_table(nfsd_sysctl_table);
> + nfsd_sysctl_table = NULL;
> }
> return retval;
> }
> @@ -546,6 +571,9 @@ static void __exit exit_nfsd(void)
> nfsd_lockd_shutdown();
> nfsd_idmap_shutdown();
> unregister_filesystem(&nfsd_fs_type);
> + if (nfsd_sysctl_table)
> + unregister_sysctl_table(nfsd_sysctl_table);
> + nfsd_sysctl_table = NULL;
> }
>
> MODULE_AUTHOR("Olaf Kirch <[email protected]>");
> Index: linux-2.6.18/fs/nfsd/nfs3xdr.c
> ===================================================================
> --- linux-2.6.18.orig/fs/nfsd/nfs3xdr.c
> +++ linux-2.6.18/fs/nfsd/nfs3xdr.c
> @@ -28,6 +28,9 @@
> # define inline
> #endif
>
> +unsigned int nfsd_readdirplus_max = NFSSVC_MAXBLKSIZE;
> +unsigned int nfsd_readdirplus_max_lb = 512;
> +unsigned int nfsd_readdirplus_max_ub = NFSSVC_MAXBLKSIZE;
>
> /*
> * Mapping of S_IF* types to NFS file types
> @@ -573,7 +576,7 @@ nfs3svc_decode_readdirplusargs(struct sv
> args->dircount = ntohl(*p++);
> args->count = ntohl(*p++);
>
> - len = (args->count > NFSSVC_MAXBLKSIZE) ? NFSSVC_MAXBLKSIZE :
> + len = (args->count > nfsd_readdirplus_max) ? nfsd_readdirplus_max :
> args->count;
> args->count = len;
>
> Index: linux-2.6.18/include/linux/nfsd/xdr3.h
> ===================================================================
> --- linux-2.6.18.orig/include/linux/nfsd/xdr3.h
> +++ linux-2.6.18/include/linux/nfsd/xdr3.h
> @@ -268,6 +268,10 @@ union nfsd3_xdrstore {
>
> #define NFS3_SVC_XDRSIZE sizeof(union nfsd3_xdrstore)
>
> +extern unsigned int nfsd_readdirplus_max;
> +extern unsigned int nfsd_readdirplus_max_ub;
> +extern unsigned int nfsd_readdirplus_max_lb;
> +
> int nfs3svc_decode_fhandle(struct svc_rqst *, u32 *, struct nfsd_fhandle *);
> int nfs3svc_decode_sattrargs(struct svc_rqst *, u32 *,
> struct nfsd3_sattrargs *);
I don't recall the history of /proc/sys/fs -- is there also a
/proc/sys/fs/nfsd directory, and wouldn't it be better to add this
particular sysctl in there?
Any thoughts about providing a mechanism for tuning or disabling
readdirplus on the client as well? If we provided such a mechanism,
would it not collide with the server tunable in /proc/sys/fs/nfs ?
--
"We who cut mere stones must always be envisioning cathedrals"
-- Quarry worker's creed
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Thu, 2006-08-03 at 09:52 -0700, Chuck Lever wrote:
> I don't recall the history of /proc/sys/fs -- is there also a
> /proc/sys/fs/nfsd directory, and wouldn't it be better to add this
> particular sysctl in there?
I too would prefer to reserve /proc/sys/fs/nfs for client and/or common
stuff (like the lockd listening ports).
There is an nfsd filesystem (usually mounted on /proc/fs/nfsd), for
which the above is more appropriate.
Note, however, that the entire sysctl should be unnecessary. Clients are
supposed to tell the server how large a reply to return, by means of the
'maxcount' argument (see RFC1813). If the clients aren't doing that,
then they really need to be fixed.
Linux clients should, AFAIK, always be setting maxcount to
PAGE_CACHE_SIZE == 4k.
Cheers,
Trond
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Fri, 2006-08-04 at 02:52, Chuck Lever wrote:
> On 8/3/06, Olaf Kirch <[email protected]> wrote:
> > From: [email protected]
> > Subject: Add /proc/sys/fs/nfs sysctls to nfsd module
> >
> > This patch adds the plumbing for adding nfs-specific sysctls to fs/nfs,
> > and makes the max size of a readdirplus reply tunable.
> >
> > The reason for this is that we found that some clients do not seem
> > to grok readdirplus replies larger than 4K. Rather than reducing the
> > overall number this sysctl allows admins to work around these clients.
Neat.
> > --- linux-2.6.18.orig/fs/nfsd/nfs3xdr.c
> > +++ linux-2.6.18/fs/nfsd/nfs3xdr.c
> > @@ -573,7 +576,7 @@ nfs3svc_decode_readdirplusargs(struct sv
> > args->dircount = ntohl(*p++);
> > args->count = ntohl(*p++);
> >
> > - len = (args->count > NFSSVC_MAXBLKSIZE) ? NFSSVC_MAXBLKSIZE :
> > + len = (args->count > nfsd_readdirplus_max) ? nfsd_readdirplus_max :
> > args->count;
> > args->count = len;
> >
Neil, a headsup: this hunk clashes with one in the server large
wsize patch, and will require minor and obvious manual adjustment.
> I don't recall the history of /proc/sys/fs -- is there also a
> /proc/sys/fs/nfsd directory,
No.
> and wouldn't it be better to add this
> particular sysctl in there?
I'd be happy to see such a thing. One of my pending patches adds
serverside sysctls; it feels wrong putting them in /proc/sys/fs/nfs/.
OTOH that directory already has four serverside NLM entries that
people do use.
Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs