* [PATCH 001 of 8] knfsd: lockd: nfsd4: use same grace period for lockd and nfsd4
2007-06-21 4:30 [PATCH 000 of 8] knfsd: Assorted nfsv4 server patches NeilBrown
@ 2007-06-21 4:30 ` NeilBrown
2007-06-21 4:30 ` [PATCH 002 of 8] knfsd: nfsd4: fix NFSv4 filehandle size units confusion NeilBrown
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: NeilBrown @ 2007-06-21 4:30 UTC (permalink / raw)
To: Andrew Morton
Cc: nfs, linux-kernel, J . Bruce Fields, J. Bruce Fields, Marc Eshel,
Neil Brown
From: "J. Bruce Fields" <bfields@fieldses.org>
Both lockd and (in the nfsv4 case) nfsd enforce a "grace period" after
reboot, during which clients may reclaim locks from the previous server
instance, but may not acquire new locks.
Currently the lockd and nfsd enforce grace periods of different lengths.
This may cause problems when we reboot a server with both v2/v3 and v4
clients. For example, if the lockd grace period is shorter (as is likely
the case), then a v3 client might acquire a new lock that conflicts with a
lock already held (but not yet reclaimed) by a v4 client.
This patch calculates a lease time that lockd and nfsd can both use.
Signed-off-by: Marc Eshel <eshel@almaden.ibm.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/lockd/svc.c | 27 ++++++++++++++++++++-------
./fs/nfsd/lockd.c | 1 +
./fs/nfsd/nfs4state.c | 16 ++++++++++++----
./include/linux/lockd/bind.h | 9 +++++++++
4 files changed, 42 insertions(+), 11 deletions(-)
diff .prev/fs/lockd/svc.c ./fs/lockd/svc.c
--- .prev/fs/lockd/svc.c 2007-06-21 14:08:51.000000000 +1000
+++ ./fs/lockd/svc.c 2007-06-21 13:52:05.000000000 +1000
@@ -76,18 +76,31 @@ static const int nlm_port_min = 0, nlm_
static struct ctl_table_header * nlm_sysctl_table;
-static unsigned long set_grace_period(void)
+static unsigned long get_lockd_grace_period(void)
{
- unsigned long grace_period;
-
/* Note: nlm_timeout should always be nonzero */
if (nlm_grace_period)
- grace_period = ((nlm_grace_period + nlm_timeout - 1)
- / nlm_timeout) * nlm_timeout * HZ;
+ return roundup(nlm_grace_period, nlm_timeout) * HZ;
else
- grace_period = nlm_timeout * 5 * HZ;
+ return nlm_timeout * 5 * HZ;
+}
+
+unsigned long get_nfs_grace_period(void)
+{
+ unsigned long lockdgrace = get_lockd_grace_period();
+ unsigned long nfsdgrace = 0;
+
+ if (nlmsvc_ops)
+ nfsdgrace = nlmsvc_ops->get_grace_period();
+
+ return max(lockdgrace, nfsdgrace);
+}
+EXPORT_SYMBOL(get_nfs_grace_period);
+
+static unsigned long set_grace_period(void)
+{
nlmsvc_grace_period = 1;
- return grace_period + jiffies;
+ return get_nfs_grace_period() + jiffies;
}
static inline void clear_grace_period(void)
diff .prev/fs/nfsd/lockd.c ./fs/nfsd/lockd.c
--- .prev/fs/nfsd/lockd.c 2007-06-21 14:08:51.000000000 +1000
+++ ./fs/nfsd/lockd.c 2007-06-21 13:46:56.000000000 +1000
@@ -65,6 +65,7 @@ nlm_fclose(struct file *filp)
static struct nlmsvc_binding nfsd_nlm_ops = {
.fopen = nlm_fopen, /* open file for locking */
.fclose = nlm_fclose, /* close file */
+ .get_grace_period = get_nfs4_grace_period,
};
void
diff .prev/fs/nfsd/nfs4state.c ./fs/nfsd/nfs4state.c
--- .prev/fs/nfsd/nfs4state.c 2007-06-21 14:08:51.000000000 +1000
+++ ./fs/nfsd/nfs4state.c 2007-06-21 14:09:23.000000000 +1000
@@ -51,6 +51,7 @@
#include <linux/namei.h>
#include <linux/mutex.h>
#include <linux/lockd/bind.h>
+#include <linux/module.h>
#define NFSDDBG_FACILITY NFSDDBG_PROC
@@ -3191,20 +3192,27 @@ nfsd4_load_reboot_recovery_data(void)
printk("NFSD: Failure reading reboot recovery data\n");
}
+unsigned long
+get_nfs4_grace_period(void)
+{
+ return max(user_lease_time, lease_time) * HZ;
+}
+
/* initialization to perform when the nfsd service is started: */
static void
__nfs4_state_start(void)
{
- time_t grace_time;
+ unsigned long grace_time;
boot_time = get_seconds();
- grace_time = max(user_lease_time, lease_time);
+ grace_time = get_nfs_grace_period();
lease_time = user_lease_time;
in_grace = 1;
- printk("NFSD: starting %ld-second grace period\n", grace_time);
+ printk(KERN_INFO "NFSD: starting %ld-second grace period\n",
+ grace_time/HZ);
laundry_wq = create_singlethread_workqueue("nfsd4");
- queue_delayed_work(laundry_wq, &laundromat_work, grace_time*HZ);
+ queue_delayed_work(laundry_wq, &laundromat_work, grace_time);
}
int
diff .prev/include/linux/lockd/bind.h ./include/linux/lockd/bind.h
--- .prev/include/linux/lockd/bind.h 2007-06-21 14:08:51.000000000 +1000
+++ ./include/linux/lockd/bind.h 2007-06-21 13:46:56.000000000 +1000
@@ -27,6 +27,7 @@ struct nlmsvc_binding {
struct nfs_fh *,
struct file **);
void (*fclose)(struct file *);
+ unsigned long (*get_grace_period)(void);
};
extern struct nlmsvc_binding * nlmsvc_ops;
@@ -38,4 +39,12 @@ extern int nlmclnt_proc(struct inode *,
extern int lockd_up(int proto);
extern void lockd_down(void);
+unsigned long get_nfs_grace_period(void);
+
+#ifdef CONFIG_NFSD_V4
+unsigned long get_nfs4_grace_period(void);
+#else
+static inline unsigned long get_nfs4_grace_period(void) {return 0;}
+#endif
+
#endif /* LINUX_LOCKD_BIND_H */
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 002 of 8] knfsd: nfsd4: fix NFSv4 filehandle size units confusion
2007-06-21 4:30 [PATCH 000 of 8] knfsd: Assorted nfsv4 server patches NeilBrown
2007-06-21 4:30 ` [PATCH 001 of 8] knfsd: lockd: nfsd4: use same grace period for lockd and nfsd4 NeilBrown
@ 2007-06-21 4:30 ` NeilBrown
2007-06-21 4:30 ` [PATCH 003 of 8] knfsd: nfsd4: silence a compiler warning in ACL code NeilBrown
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: NeilBrown @ 2007-06-21 4:30 UTC (permalink / raw)
To: Andrew Morton
Cc: nfs, linux-kernel, J. Bruce Fields, J. Bruce Fields, Neil Brown
From: "J. Bruce Fields" <bfields@fieldses.org>
NFS4_FHSIZE is measured in bytes, not 4-byte words, so much more space
than necessary is being allocated for struct nfs4_cb_recall.
I should have wondered why this structure was so much larger than it
needed to be!
Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./include/linux/nfsd/state.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff .prev/include/linux/nfsd/state.h ./include/linux/nfsd/state.h
--- .prev/include/linux/nfsd/state.h 2007-06-21 13:46:39.000000000 +1000
+++ ./include/linux/nfsd/state.h 2007-06-21 14:09:31.000000000 +1000
@@ -67,7 +67,7 @@ struct nfs4_cb_recall {
int cbr_trunc;
stateid_t cbr_stateid;
u32 cbr_fhlen;
- u32 cbr_fhval[NFS4_FHSIZE];
+ char cbr_fhval[NFS4_FHSIZE];
struct nfs4_delegation *cbr_dp;
};
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 003 of 8] knfsd: nfsd4: silence a compiler warning in ACL code
2007-06-21 4:30 [PATCH 000 of 8] knfsd: Assorted nfsv4 server patches NeilBrown
2007-06-21 4:30 ` [PATCH 001 of 8] knfsd: lockd: nfsd4: use same grace period for lockd and nfsd4 NeilBrown
2007-06-21 4:30 ` [PATCH 002 of 8] knfsd: nfsd4: fix NFSv4 filehandle size units confusion NeilBrown
@ 2007-06-21 4:30 ` NeilBrown
2007-06-21 4:30 ` [PATCH 004 of 8] knfsd: nfsd4: fix enc_stateid_sz for nfsd callbacks NeilBrown
` (4 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: NeilBrown @ 2007-06-21 4:30 UTC (permalink / raw)
To: Andrew Morton
Cc: nfs, linux-kernel, J. Bruce Fields, J. Bruce Fields, Neil Brown
From: "J. Bruce Fields" <bfields@fieldses.org>
Silence a compiler warning in the ACL code, and add a comment making
clear the initialization serves no other purpose.
Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/nfsd/nfs4acl.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff .prev/fs/nfsd/nfs4acl.c ./fs/nfsd/nfs4acl.c
--- .prev/fs/nfsd/nfs4acl.c 2007-06-21 13:46:39.000000000 +1000
+++ ./fs/nfsd/nfs4acl.c 2007-06-21 14:10:06.000000000 +1000
@@ -183,8 +183,13 @@ static void
summarize_posix_acl(struct posix_acl *acl, struct posix_acl_summary *pas)
{
struct posix_acl_entry *pa, *pe;
- pas->users = 0;
- pas->groups = 0;
+
+ /*
+ * Only pas.users and pas.groups need initialization; previous
+ * posix_acl_valid() calls ensure that the other fields will be
+ * initialized in the following loop. But, just to placate gcc:
+ */
+ memset(pas, 0, sizeof(*pas));
pas->mask = 07;
pe = acl->a_entries + acl->a_count;
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 004 of 8] knfsd: nfsd4: fix enc_stateid_sz for nfsd callbacks
2007-06-21 4:30 [PATCH 000 of 8] knfsd: Assorted nfsv4 server patches NeilBrown
` (2 preceding siblings ...)
2007-06-21 4:30 ` [PATCH 003 of 8] knfsd: nfsd4: silence a compiler warning in ACL code NeilBrown
@ 2007-06-21 4:30 ` NeilBrown
2007-06-21 4:30 ` [PATCH 005 of 8] knfsd: nfsd4: fix handling of acl errrors NeilBrown
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: NeilBrown @ 2007-06-21 4:30 UTC (permalink / raw)
To: Andrew Morton
Cc: nfs, linux-kernel, Benny Halevy, J. Bruce Fields, J. Bruce Fields,
Neil Brown
From: "J. Bruce Fields" <bfields@fieldses.org>
enc_stateid_sz should be given in u32 words units, not bytes, so we were
overestimating the buffer space needed here.
Signed-off-by: Benny Halevy <bhalevy@panasas.com>
Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/nfsd/nfs4callback.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff .prev/fs/nfsd/nfs4callback.c ./fs/nfsd/nfs4callback.c
--- .prev/fs/nfsd/nfs4callback.c 2007-06-21 13:46:38.000000000 +1000
+++ ./fs/nfsd/nfs4callback.c 2007-06-21 14:10:31.000000000 +1000
@@ -75,7 +75,7 @@ enum nfs_cb_opnum4 {
#define op_enc_sz 1
#define op_dec_sz 2
#define enc_nfs4_fh_sz (1 + (NFS4_FHSIZE >> 2))
-#define enc_stateid_sz 16
+#define enc_stateid_sz (NFS4_STATEID_SIZE >> 2)
#define NFS4_enc_cb_recall_sz (cb_compound_enc_hdr_sz + \
1 + enc_stateid_sz + \
enc_nfs4_fh_sz)
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 005 of 8] knfsd: nfsd4: fix handling of acl errrors
2007-06-21 4:30 [PATCH 000 of 8] knfsd: Assorted nfsv4 server patches NeilBrown
` (3 preceding siblings ...)
2007-06-21 4:30 ` [PATCH 004 of 8] knfsd: nfsd4: fix enc_stateid_sz for nfsd callbacks NeilBrown
@ 2007-06-21 4:30 ` NeilBrown
2007-06-21 4:31 ` [PATCH 006 of 8] knfsd: nfsd: remove unused header interface.h NeilBrown
` (2 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: NeilBrown @ 2007-06-21 4:30 UTC (permalink / raw)
To: Andrew Morton
Cc: nfs, linux-kernel, Alexander Soule, J. Bruce Fields,
J. Bruce Fields, Neil Brown
From: "J. Bruce Fields" <bfields@fieldses.org>
nfs4_acl_nfsv4_to_posix() returns an error and returns any posix acls
calculated in two caller-provided pointers. It was setting these
pointers to -errno in some error cases, resulting in
nfsd4_set_nfs4_acl() calling posix_acl_release() with a -errno as an
argument.
Fix both the caller and the callee, by modifying nfsd4_set_nfs4_acl() to
stop relying on the passed-in-pointers being left as NULL in the error
case, and by modifying nfs4_acl_nfsv4_to_posix() to stop returning
garbage in those pointers.
Thanks to Alex Soule for reporting the bug.
Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
Cc: Alexander Soule <soule@umich.edu>
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/nfsd/nfs4acl.c | 3 +++
./fs/nfsd/vfs.c | 22 +++++++---------------
2 files changed, 10 insertions(+), 15 deletions(-)
diff .prev/fs/nfsd/nfs4acl.c ./fs/nfsd/nfs4acl.c
--- .prev/fs/nfsd/nfs4acl.c 2007-06-21 14:10:06.000000000 +1000
+++ ./fs/nfsd/nfs4acl.c 2007-06-21 14:11:02.000000000 +1000
@@ -737,13 +737,16 @@ int nfs4_acl_nfsv4_to_posix(struct nfs4_
*pacl = posix_state_to_acl(&effective_acl_state, flags);
if (IS_ERR(*pacl)) {
ret = PTR_ERR(*pacl);
+ *pacl = NULL;
goto out_dstate;
}
*dpacl = posix_state_to_acl(&default_acl_state,
flags | NFS4_ACL_TYPE_DEFAULT);
if (IS_ERR(*dpacl)) {
ret = PTR_ERR(*dpacl);
+ *dpacl = NULL;
posix_acl_release(*pacl);
+ *pacl = NULL;
goto out_dstate;
}
sort_pacl(*pacl);
diff .prev/fs/nfsd/vfs.c ./fs/nfsd/vfs.c
--- .prev/fs/nfsd/vfs.c 2007-06-21 13:46:38.000000000 +1000
+++ ./fs/nfsd/vfs.c 2007-06-21 14:11:02.000000000 +1000
@@ -435,7 +435,7 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
/* Get inode */
error = fh_verify(rqstp, fhp, 0 /* S_IFREG */, MAY_SATTR);
if (error)
- goto out;
+ return error;
dentry = fhp->fh_dentry;
inode = dentry->d_inode;
@@ -444,33 +444,25 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
host_error = nfs4_acl_nfsv4_to_posix(acl, &pacl, &dpacl, flags);
if (host_error == -EINVAL) {
- error = nfserr_attrnotsupp;
- goto out;
+ return nfserr_attrnotsupp;
} else if (host_error < 0)
goto out_nfserr;
host_error = set_nfsv4_acl_one(dentry, pacl, POSIX_ACL_XATTR_ACCESS);
if (host_error < 0)
- goto out_nfserr;
+ goto out_release;
- if (S_ISDIR(inode->i_mode)) {
+ if (S_ISDIR(inode->i_mode))
host_error = set_nfsv4_acl_one(dentry, dpacl, POSIX_ACL_XATTR_DEFAULT);
- if (host_error < 0)
- goto out_nfserr;
- }
-
- error = nfs_ok;
-out:
+out_release:
posix_acl_release(pacl);
posix_acl_release(dpacl);
- return (error);
out_nfserr:
if (host_error == -EOPNOTSUPP)
- error = nfserr_attrnotsupp;
+ return nfserr_attrnotsupp;
else
- error = nfserrno(host_error);
- goto out;
+ return nfserrno(host_error);
}
static struct posix_acl *
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 006 of 8] knfsd: nfsd: remove unused header interface.h
2007-06-21 4:30 [PATCH 000 of 8] knfsd: Assorted nfsv4 server patches NeilBrown
` (4 preceding siblings ...)
2007-06-21 4:30 ` [PATCH 005 of 8] knfsd: nfsd4: fix handling of acl errrors NeilBrown
@ 2007-06-21 4:31 ` NeilBrown
2007-06-21 4:31 ` [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size NeilBrown
2007-06-21 4:31 ` [PATCH 008 of 8] knfsd: nfsd4: don't delegate files that have had conflicts NeilBrown
7 siblings, 0 replies; 17+ messages in thread
From: NeilBrown @ 2007-06-21 4:31 UTC (permalink / raw)
To: Andrew Morton
Cc: nfs, linux-kernel, J. Bruce Fields, J. Bruce Fields, Neil Brown
From: "J. Bruce Fields" <bfields@fieldses.org>
It looks like Al Viro gutted this header file five years ago and it
hasn't been touched since.
Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/nfsd/nfsctl.c | 1 -
./include/linux/nfsd/interface.h | 13 -------------
./include/linux/nfsd/nfsd.h | 1 -
3 files changed, 15 deletions(-)
diff .prev/fs/nfsd/nfsctl.c ./fs/nfsd/nfsctl.c
--- .prev/fs/nfsd/nfsctl.c 2007-06-21 13:46:36.000000000 +1000
+++ ./fs/nfsd/nfsctl.c 2007-06-21 14:13:57.000000000 +1000
@@ -35,7 +35,6 @@
#include <linux/nfsd/cache.h>
#include <linux/nfsd/xdr.h>
#include <linux/nfsd/syscall.h>
-#include <linux/nfsd/interface.h>
#include <asm/uaccess.h>
diff .prev/include/linux/nfsd/interface.h ./include/linux/nfsd/interface.h
--- .prev/include/linux/nfsd/interface.h 2007-06-21 13:46:36.000000000 +1000
+++ ./include/linux/nfsd/interface.h 1970-01-01 10:00:00.000000000 +1000
@@ -1,13 +0,0 @@
-/*
- * include/linux/nfsd/interface.h
- *
- * defines interface between nfsd and other bits of
- * the kernel. Particularly filesystems (eventually).
- *
- * Copyright (C) 2000 Neil Brown <neilb@cse.unsw.edu.au>
- */
-
-#ifndef LINUX_NFSD_INTERFACE_H
-#define LINUX_NFSD_INTERFACE_H
-
-#endif /* LINUX_NFSD_INTERFACE_H */
diff .prev/include/linux/nfsd/nfsd.h ./include/linux/nfsd/nfsd.h
--- .prev/include/linux/nfsd/nfsd.h 2007-06-21 13:46:36.000000000 +1000
+++ ./include/linux/nfsd/nfsd.h 2007-06-21 14:13:57.000000000 +1000
@@ -22,7 +22,6 @@
#include <linux/nfsd/export.h>
#include <linux/nfsd/auth.h>
#include <linux/nfsd/stats.h>
-#include <linux/nfsd/interface.h>
/*
* nfsd version
*/
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size
2007-06-21 4:30 [PATCH 000 of 8] knfsd: Assorted nfsv4 server patches NeilBrown
` (5 preceding siblings ...)
2007-06-21 4:31 ` [PATCH 006 of 8] knfsd: nfsd: remove unused header interface.h NeilBrown
@ 2007-06-21 4:31 ` NeilBrown
2007-06-21 16:15 ` J. Bruce Fields
2007-06-26 3:52 ` Andrew Morton
2007-06-21 4:31 ` [PATCH 008 of 8] knfsd: nfsd4: don't delegate files that have had conflicts NeilBrown
7 siblings, 2 replies; 17+ messages in thread
From: NeilBrown @ 2007-06-21 4:31 UTC (permalink / raw)
To: Andrew Morton
Cc: nfs, linux-kernel, J. Bruce Fields, J. Bruce Fields, Neil Brown
From: "J. Bruce Fields" <bfields@fieldses.org>
Our original NFSv4 delegation policy was to give out a read delegation
on any open when it was possible to.
Since the lifetime of a delegation isn't limited to that of an open, a
client may quite reasonably hang on to a delegation as long as it has
the inode cached. This becomes an obvious problem the first time a
client's inode cache approaches the size of the server's total memory.
Our first quick solution was to add a hard-coded limit. This patch
makes a mild incremental improvement by varying that limit according to
the server's total memory size, allowing at most 4 delegations per
megabyte of RAM.
My quick back-of-the-envelope calculation finds that in the worst case
(where every delegation is for a different inode), a delegation could
take about 1.5K, which would make the worst case usage about 6% of
memory. The new limit works out to be about the same as the old on a
1-gig server.
Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/nfsd/nfs4state.c | 15 ++++++++++++++-
./include/linux/nfsd/nfsd.h | 1 +
2 files changed, 15 insertions(+), 1 deletion(-)
diff .prev/fs/nfsd/nfs4state.c ./fs/nfsd/nfs4state.c
--- .prev/fs/nfsd/nfs4state.c 2007-06-21 14:09:23.000000000 +1000
+++ ./fs/nfsd/nfs4state.c 2007-06-21 14:14:47.000000000 +1000
@@ -150,6 +150,7 @@ get_nfs4_file(struct nfs4_file *fi)
}
static int num_delegations;
+unsigned int max_delegations = 0;
/*
* Open owner state (share locks)
@@ -193,7 +194,7 @@ alloc_init_deleg(struct nfs4_client *clp
struct nfs4_callback *cb = &stp->st_stateowner->so_client->cl_callback;
dprintk("NFSD alloc_init_deleg\n");
- if (num_delegations > STATEID_HASH_SIZE * 4)
+ if (num_delegations > max_delegations)
return NULL;
dp = kmem_cache_alloc(deleg_slab, GFP_KERNEL);
if (dp == NULL)
@@ -3198,6 +3199,17 @@ get_nfs4_grace_period(void)
return max(user_lease_time, lease_time) * HZ;
}
+static void
+set_max_delegations()
+{
+ struct sysinfo sys;
+
+ si_meminfo(&sys);
+ sys.totalram *= sys.mem_unit;
+ sys.totalram >>= (18 - PAGE_SHIFT);
+ max_delegations = (unsigned int) sys.totalram;
+}
+
/* initialization to perform when the nfsd service is started: */
static void
@@ -3213,6 +3225,7 @@ __nfs4_state_start(void)
grace_time/HZ);
laundry_wq = create_singlethread_workqueue("nfsd4");
queue_delayed_work(laundry_wq, &laundromat_work, grace_time);
+ set_max_delegations();
}
int
diff .prev/include/linux/nfsd/nfsd.h ./include/linux/nfsd/nfsd.h
--- .prev/include/linux/nfsd/nfsd.h 2007-06-21 14:13:57.000000000 +1000
+++ ./include/linux/nfsd/nfsd.h 2007-06-21 14:14:47.000000000 +1000
@@ -148,6 +148,7 @@ extern int nfsd_max_blksize;
* NFSv4 State
*/
#ifdef CONFIG_NFSD_V4
+extern unsigned int max_delegations;
void nfs4_state_init(void);
int nfs4_state_start(void);
void nfs4_state_shutdown(void);
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size
2007-06-21 4:31 ` [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size NeilBrown
@ 2007-06-21 16:15 ` J. Bruce Fields
2007-06-26 3:52 ` Andrew Morton
1 sibling, 0 replies; 17+ messages in thread
From: J. Bruce Fields @ 2007-06-21 16:15 UTC (permalink / raw)
To: NeilBrown; +Cc: Andrew Morton, nfs, linux-kernel
I missed a compile warning here, apologies.
--b.
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c79f742..fbbbcab 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3204,7 +3204,7 @@ get_nfs4_grace_period(void)
}
static void
-set_max_delegations()
+set_max_delegations(void)
{
struct sysinfo sys;
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size
2007-06-21 4:31 ` [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size NeilBrown
2007-06-21 16:15 ` J. Bruce Fields
@ 2007-06-26 3:52 ` Andrew Morton
2007-06-28 2:15 ` J. Bruce Fields
1 sibling, 1 reply; 17+ messages in thread
From: Andrew Morton @ 2007-06-26 3:52 UTC (permalink / raw)
To: NeilBrown; +Cc: nfs, linux-kernel, J. Bruce Fields, J. Bruce Fields
On Thu, 21 Jun 2007 14:31:12 +1000 NeilBrown <neilb@suse.de> wrote:
>
> From: "J. Bruce Fields" <bfields@fieldses.org>
>
>
> Our original NFSv4 delegation policy was to give out a read delegation
> on any open when it was possible to.
>
> Since the lifetime of a delegation isn't limited to that of an open, a
> client may quite reasonably hang on to a delegation as long as it has
> the inode cached. This becomes an obvious problem the first time a
> client's inode cache approaches the size of the server's total memory.
>
> Our first quick solution was to add a hard-coded limit. This patch
> makes a mild incremental improvement by varying that limit according to
> the server's total memory size, allowing at most 4 delegations per
> megabyte of RAM.
>
> My quick back-of-the-envelope calculation finds that in the worst case
> (where every delegation is for a different inode), a delegation could
> take about 1.5K, which would make the worst case usage about 6% of
> memory. The new limit works out to be about the same as the old on a
> 1-gig server.
>
> ...
>
> +static void
> +set_max_delegations()
> +{
> + struct sysinfo sys;
> +
> + si_meminfo(&sys);
> + sys.totalram *= sys.mem_unit;
> + sys.totalram >>= (18 - PAGE_SHIFT);
> + max_delegations = (unsigned int) sys.totalram;
> +}
Please put yourself in the position of the reader who happens across this
code and wonders why it is that way.
They could of course go hunt it out of the git repo but I do think it's
quite a bit more reader-friendly to explain the thinking in code comments.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size
2007-06-26 3:52 ` Andrew Morton
@ 2007-06-28 2:15 ` J. Bruce Fields
2007-06-28 2:36 ` Andrew Morton
0 siblings, 1 reply; 17+ messages in thread
From: J. Bruce Fields @ 2007-06-28 2:15 UTC (permalink / raw)
To: Andrew Morton; +Cc: NeilBrown, nfs, linux-kernel
This code would benefit from a little more explanation.
Also, I screwed up the previous patch and forgot to delete a line, so the
calculation here is off; fix it.
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
fs/nfsd/nfs4state.c | 16 +++++++++++++++-
1 files changed, 15 insertions(+), 1 deletions(-)
On Mon, Jun 25, 2007 at 08:52:44PM -0700, Andrew Morton wrote:
>
> Please put yourself in the position of the reader who happens across this
> code and wonders why it is that way.
>
> They could of course go hunt it out of the git repo but I do think it's
> quite a bit more reader-friendly to explain the thinking in code comments.
>
Yup, sorry about that. Could you apply this?
--b.
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index fbbbcab..1b2657f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3203,13 +3203,27 @@ get_nfs4_grace_period(void)
return max(user_lease_time, lease_time) * HZ;
}
+/*
+ * Since the lifetime of a delegation isn't limited to that of an open, a
+ * client may quite reasonably hang on to a delegation as long as it has
+ * the inode cached. This becomes an obvious problem the first time a
+ * client's inode cache approaches the size of the server's total memory.
+ *
+ * For now we avoid this problem by imposing a hard limit on the number
+ * of delegations, which varies according to the server's memory size.
+ */
static void
set_max_delegations(void)
{
struct sysinfo sys;
si_meminfo(&sys);
- sys.totalram *= sys.mem_unit;
+ /*
+ * Allow at most 4 delegations per megabyte of RAM. Quick
+ * estimates suggest that in the worst case (where every delegation
+ * is for a different inode), a delegation could take about 1.5K,
+ * giving a worst case usage of about 6% of memory.
+ */
sys.totalram >>= (18 - PAGE_SHIFT);
max_delegations = (unsigned int) sys.totalram;
}
--
1.5.2.58.g98ee
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size
2007-06-28 2:15 ` J. Bruce Fields
@ 2007-06-28 2:36 ` Andrew Morton
2007-06-28 2:57 ` J. Bruce Fields
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Morton @ 2007-06-28 2:36 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: NeilBrown, nfs, linux-kernel
On Wed, 27 Jun 2007 22:15:52 -0400 "J. Bruce Fields" <bfields@fieldses.org> wrote:
> + /*
> + * Allow at most 4 delegations per megabyte of RAM. Quick
> + * estimates suggest that in the worst case (where every delegation
> + * is for a different inode), a delegation could take about 1.5K,
> + * giving a worst case usage of about 6% of memory.
> + */
> sys.totalram >>= (18 - PAGE_SHIFT);
6% of 32GB is 2GB is too much for a large highmem machine. We should base
this on the amount of kernel-addressable memory (aka lowmem).
How's this?
static void
set_max_delegations(void)
{
/*
* Allow at most 4 delegations per megabyte of RAM. Quick
* estimates suggest that in the worst case (where every delegation
* is for a different inode), a delegation could take about 1.5K,
* giving a worst case usage of about 6% of memory.
*/
max_delegations = nr_free_buffer_pages() >> (20 - 2 - PAGE_SHIFT);
}
fs/nfsd/nfs4state.c | 7 ++-----
mm/page_alloc.c | 1 +
diff -puN fs/nfsd/nfs4state.c~knfsd-nfsd4-vary-maximum-delegation-limit-based-on-ram-size-fix-fix-fix-fix fs/nfsd/nfs4state.c
--- a/fs/nfsd/nfs4state.c~knfsd-nfsd4-vary-maximum-delegation-limit-based-on-ram-size-fix-fix-fix-fix
+++ a/fs/nfsd/nfs4state.c
@@ -49,6 +49,7 @@
#include <linux/nfsd/state.h>
#include <linux/nfsd/xdr4.h>
#include <linux/namei.h>
+#include <linux/swap.h>
#include <linux/mutex.h>
#include <linux/lockd/bind.h>
#include <linux/module.h>
@@ -3210,17 +3211,13 @@ get_nfs4_grace_period(void)
static void
set_max_delegations(void)
{
- struct sysinfo sys;
-
- si_meminfo(&sys);
/*
* Allow at most 4 delegations per megabyte of RAM. Quick
* estimates suggest that in the worst case (where every delegation
* is for a different inode), a delegation could take about 1.5K,
* giving a worst case usage of about 6% of memory.
*/
- sys.totalram >>= (18 - PAGE_SHIFT);
- max_delegations = (unsigned int) sys.totalram;
+ max_delegations = nr_free_buffer_pages() >> (20 - 2 - PAGE_SHIFT);
}
/* initialization to perform when the nfsd service is started: */
diff -puN mm/page_alloc.c~knfsd-nfsd4-vary-maximum-delegation-limit-based-on-ram-size-fix-fix-fix-fix mm/page_alloc.c
--- a/mm/page_alloc.c~knfsd-nfsd4-vary-maximum-delegation-limit-based-on-ram-size-fix-fix-fix-fix
+++ a/mm/page_alloc.c
@@ -1729,6 +1729,7 @@ unsigned int nr_free_buffer_pages(void)
{
return nr_free_zone_pages(gfp_zone(GFP_USER));
}
+EXPORT_SYMBOL_GPL(nr_free_buffer_pages);
/*
* Amount of free RAM allocatable within all zones
_
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size
2007-06-28 2:36 ` Andrew Morton
@ 2007-06-28 2:57 ` J. Bruce Fields
2007-06-28 3:10 ` Andrew Morton
0 siblings, 1 reply; 17+ messages in thread
From: J. Bruce Fields @ 2007-06-28 2:57 UTC (permalink / raw)
To: Andrew Morton; +Cc: NeilBrown, nfs, linux-kernel
On Wed, Jun 27, 2007 at 07:36:13PM -0700, Andrew Morton wrote:
> How's this?
Makes sense to me, thanks. I guess fs/nsfd/nfssvc:nfsd_create_serv()
should be similarly modified? It calculates the size of per-nfsd-thread
buffers used to hold requests, which will eventually be allocated with
an alloc_page(GFP_KERNEL), and it bases the calculation on the same
.totalram field from struct sysinfo.
--b.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size
2007-06-28 2:57 ` J. Bruce Fields
@ 2007-06-28 3:10 ` Andrew Morton
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Morton @ 2007-06-28 3:10 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: NeilBrown, nfs, linux-kernel
On Wed, 27 Jun 2007 22:57:39 -0400 "J. Bruce Fields" <bfields@fieldses.org> wrote:
> On Wed, Jun 27, 2007 at 07:36:13PM -0700, Andrew Morton wrote:
> > How's this?
>
> Makes sense to me, thanks. I guess fs/nsfd/nfssvc:nfsd_create_serv()
> should be similarly modified? It calculates the size of per-nfsd-thread
> buffers used to hold requests, which will eventually be allocated with
> an alloc_page(GFP_KERNEL), and it bases the calculation on the same
> .totalram field from struct sysinfo.
Yup, it'd be better to use nr_free_buffer_pages() there too.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 008 of 8] knfsd: nfsd4: don't delegate files that have had conflicts
2007-06-21 4:30 [PATCH 000 of 8] knfsd: Assorted nfsv4 server patches NeilBrown
` (6 preceding siblings ...)
2007-06-21 4:31 ` [PATCH 007 of 8] knfsd: nfsd4: vary maximum delegation limit based on RAM size NeilBrown
@ 2007-06-21 4:31 ` NeilBrown
2007-06-21 16:28 ` J. Bruce Fields
7 siblings, 1 reply; 17+ messages in thread
From: NeilBrown @ 2007-06-21 4:31 UTC (permalink / raw)
To: Andrew Morton
Cc: nfs, linux-kernel, J. Bruce Fields, J. Bruce Fields, Neil Brown
From: "J. Bruce Fields" <bfields@fieldses.org>
One more incremental delegation policy improvement: don't give out a
delegation on a file if conflicting access has previously required that
a delegation be revoked on that file. (In practice we'll forget about
the conflict when the struct nfs4_file is removed on close, so this is
of limited use for now, though it should at least solve a temporary
problem with self-conflicts on write opens from the same client.)
Signed-off-by: "J. Bruce Fields" <bfields@citi.umich.edu>
Signed-off-by: Neil Brown <neilb@suse.de>
### Diffstat output
./fs/nfsd/nfs4state.c | 4 ++++
./include/linux/nfsd/state.h | 1 +
2 files changed, 5 insertions(+)
diff .prev/fs/nfsd/nfs4state.c ./fs/nfsd/nfs4state.c
--- .prev/fs/nfsd/nfs4state.c 2007-06-21 14:14:47.000000000 +1000
+++ ./fs/nfsd/nfs4state.c 2007-06-21 14:16:21.000000000 +1000
@@ -194,6 +194,8 @@ alloc_init_deleg(struct nfs4_client *clp
struct nfs4_callback *cb = &stp->st_stateowner->so_client->cl_callback;
dprintk("NFSD alloc_init_deleg\n");
+ if (fp->fi_had_conflict)
+ return NULL;
if (num_delegations > max_delegations)
return NULL;
dp = kmem_cache_alloc(deleg_slab, GFP_KERNEL);
@@ -1002,6 +1004,7 @@ alloc_init_file(struct inode *ino)
list_add(&fp->fi_hash, &file_hashtbl[hashval]);
fp->fi_inode = igrab(ino);
fp->fi_id = current_fileid++;
+ fp->fi_had_conflict = false;
return fp;
}
return NULL;
@@ -1328,6 +1331,7 @@ do_recall(void *__dp)
{
struct nfs4_delegation *dp = __dp;
+ dp->dl_file->fi_had_conflict = true;
nfsd4_cb_recall(dp);
return 0;
}
diff .prev/include/linux/nfsd/state.h ./include/linux/nfsd/state.h
--- .prev/include/linux/nfsd/state.h 2007-06-21 14:09:31.000000000 +1000
+++ ./include/linux/nfsd/state.h 2007-06-21 14:16:21.000000000 +1000
@@ -224,6 +224,7 @@ struct nfs4_file {
struct inode *fi_inode;
u32 fi_id; /* used with stateowner->so_id
* for stateid_hashtbl hash */
+ bool fi_had_conflict;
};
/*
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 008 of 8] knfsd: nfsd4: don't delegate files that have had conflicts
2007-06-21 4:31 ` [PATCH 008 of 8] knfsd: nfsd4: don't delegate files that have had conflicts NeilBrown
@ 2007-06-21 16:28 ` J. Bruce Fields
2007-06-21 16:33 ` J. Bruce Fields
0 siblings, 1 reply; 17+ messages in thread
From: J. Bruce Fields @ 2007-06-21 16:28 UTC (permalink / raw)
To: NeilBrown; +Cc: Andrew Morton, nfs, linux-kernel, meelap
The authorship information got lost on these last two patches--both were
originally by Meelap Shah <meelap@umich.edu>.
(The problem may have been that I had a From: line for him but not a
Signed-off-by: line?)
--b.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 008 of 8] knfsd: nfsd4: don't delegate files that have had conflicts
2007-06-21 16:28 ` J. Bruce Fields
@ 2007-06-21 16:33 ` J. Bruce Fields
0 siblings, 0 replies; 17+ messages in thread
From: J. Bruce Fields @ 2007-06-21 16:33 UTC (permalink / raw)
To: NeilBrown; +Cc: Andrew Morton, nfs, linux-kernel, meelap
On Thu, Jun 21, 2007 at 12:28:01PM -0400, bfields wrote:
> The authorship information got lost on these last two patches--both were
> originally by Meelap Shah <meelap@umich.edu>.
>
> (The problem may have been that I had a From: line for him but not a
> Signed-off-by: line?)
Hm, no, actually I think the same happened to patch 1 (from Marc Eshel)
and 4 (from Benny Halevy), neither which had missing signed-off-by's, so
probably your scripts have always ignored From: lines in the body of an
email, and I only just now noticed?
--b.
^ permalink raw reply [flat|nested] 17+ messages in thread