This patchset is the remainder of the client_mutex removal patches,
which should be merged after the stateid and stateowner refcounting
overhaul. The main focus of this part is to ensure that the nfs4_client
refcounting is up to snuff and then to remove the client_mutex.
There's also a series in here that basically rewrites the fault
injection code. That rewrite has some significant holes in it (note the
WARN_ONs when the client refcount goes too high), but it should keep it
limping along for now.
FWIW, I think we should probably go ahead and mark the fault injection
code deprecated in the upcoming merge window and then plan to remove
it in 2-3 releases.
Jeff Layton (19):
nfsd: Protect session creation and client confirm using client_lock
nfsd: protect the close_lru list and oo_last_closed_stid with
client_lock
nfsd: move unhash_client_locked call into mark_client_expired_locked
nfsd: don't destroy client if mark_client_expired_locked fails
nfsd: don't destroy clients that are busy
nfsd: protect clid and verifier generation with client_lock
nfsd: abstract out the get and set routines into the fault injection
ops
nfsd: add a forget_clients "get" routine with proper locking
nfsd: add a forget_client set_clnt routine
nfsd: add nfsd_inject_forget_clients
nfsd: add a list_head arg to nfsd_foreach_client_lock
nfsd: add more granular locking to forget_locks fault injector
nfsd: add more granular locking to forget_openowners fault injector
nfsd: add more granular locking to *_delegations fault injectors
nfsd: remove old fault injection infrastructure
nfsd: remove nfs4_lock_state: nfs4_laundromat
nfsd: remove nfs4_lock_state: nfs4_state_shutdown_net
nfsd: remove the client_mutex and the nfs4_lock/unlock_state wrappers
nfsd: add some comments to the nfsd4 object definitions
Trond Myklebust (18):
nfsd: Ensure struct nfs4_client is unhashed before we try to destroy
it
nfsd: Ensure that the laundromat unhashes the client before releasing
locks
nfsd: Don't require client_lock in free_client
nfsd: Move create_client() call outside the lock
nfsd: Protect unconfirmed client creation using client_lock
nfsd: Protect nfsd4_destroy_clientid using client_lock
nfsd: Ensure lookup_clientid() takes client_lock
nfsd: Add lockdep assertions to document the nfs4_client/session
locking
nfsd: Remove nfs4_lock_state(): nfs4_preprocess_stateid_op()
nfsd: Remove nfs4_lock_state(): nfsd4_test_stateid/nfsd4_free_stateid
nfsd: Remove nfs4_lock_state(): nfsd4_release_lockowner
nfsd: Remove nfs4_lock_state(): nfsd4_lock/locku/lockt()
nfsd: Remove nfs4_lock_state(): nfsd4_open_downgrade + nfsd4_close
nfsd: Remove nfs4_lock_state(): nfsd4_delegreturn()
nfsd: Remove nfs4_lock_state(): nfsd4_open and nfsd4_open_confirm
nfsd: Remove nfs4_lock_state(): exchange_id, create/destroy_session()
nfsd: Remove nfs4_lock_state(): setclientid, setclientid_confirm,
renew
nfsd: Remove nfs4_lock_state(): reclaim_complete()
fs/nfsd/fault_inject.c | 130 +++----
fs/nfsd/netns.h | 14 +-
fs/nfsd/nfs4proc.c | 3 -
fs/nfsd/nfs4state.c | 922 +++++++++++++++++++++++++++++++++++--------------
fs/nfsd/state.h | 122 +++++--
5 files changed, 836 insertions(+), 355 deletions(-)
--
1.9.3
From: Trond Myklebust <[email protected]>
The struct nfs_client is supposed to be invisible and unreferenced
before it gets here.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a374592e7dcf..256e9032f49c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1416,9 +1416,6 @@ static void __free_session(struct nfsd4_session *ses)
static void free_session(struct nfsd4_session *ses)
{
- struct nfsd_net *nn = net_generic(ses->se_client->net, nfsd_net_id);
-
- lockdep_assert_held(&nn->client_lock);
nfsd4_del_conns(ses);
nfsd4_put_drc_mem(&ses->se_fchannel);
__free_session(ses);
@@ -1568,9 +1565,6 @@ err_no_name:
static void
free_client(struct nfs4_client *clp)
{
- struct nfsd_net __maybe_unused *nn = net_generic(clp->net, nfsd_net_id);
-
- lockdep_assert_held(&nn->client_lock);
while (!list_empty(&clp->cl_sessions)) {
struct nfsd4_session *ses;
ses = list_entry(clp->cl_sessions.next, struct nfsd4_session,
@@ -1627,7 +1621,6 @@ __destroy_client(struct nfs4_client *clp)
struct nfs4_openowner *oo;
struct nfs4_delegation *dp;
struct list_head reaplist;
- struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
INIT_LIST_HEAD(&reaplist);
spin_lock(&state_lock);
@@ -1655,10 +1648,7 @@ __destroy_client(struct nfs4_client *clp)
nfsd4_shutdown_callback(clp);
if (clp->cl_cb_conn.cb_xprt)
svc_xprt_put(clp->cl_cb_conn.cb_xprt);
- spin_lock(&nn->client_lock);
- WARN_ON_ONCE(atomic_read(&clp->cl_refcount));
free_client(clp);
- spin_unlock(&nn->client_lock);
}
static void
@@ -1862,7 +1852,6 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
struct sockaddr *sa = svc_addr(rqstp);
int ret;
struct net *net = SVC_NET(rqstp);
- struct nfsd_net *nn = net_generic(net, nfsd_net_id);
clp = alloc_client(name);
if (clp == NULL)
@@ -1870,9 +1859,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
ret = copy_cred(&clp->cl_cred, &rqstp->rq_cred);
if (ret) {
- spin_lock(&nn->client_lock);
free_client(clp);
- spin_unlock(&nn->client_lock);
return NULL;
}
INIT_WORK(&clp->cl_cb_null.cb_work, nfsd4_run_cb_null);
--
1.9.3
Remove the old nfsd_for_n_state function and move nfsd_find_client
higher up into the file to get rid of forward declaration. Remove
the struct nfsd_fault_inject_op arguments from the operations as
they are no longer needed by any of them.
Finally, remove the old "standard" get and set routines, which
also eliminates the client_mutex from this code.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/fault_inject.c | 51 ++++-------------------------
fs/nfsd/nfs4state.c | 87 +++++++++++++++++++-------------------------------
fs/nfsd/state.h | 45 +++++++++++---------------
3 files changed, 57 insertions(+), 126 deletions(-)
diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
index 2479dba71c3c..c16bf5af6831 100644
--- a/fs/nfsd/fault_inject.c
+++ b/fs/nfsd/fault_inject.c
@@ -17,52 +17,13 @@
struct nfsd_fault_inject_op {
char *file;
- u64 (*get)(struct nfsd_fault_inject_op *);
- u64 (*set_val)(struct nfsd_fault_inject_op *, u64);
- u64 (*set_clnt)(struct nfsd_fault_inject_op *,
- struct sockaddr_storage *, size_t);
- u64 (*forget)(struct nfs4_client *, u64);
- u64 (*print)(struct nfs4_client *, u64);
+ u64 (*get)(void);
+ u64 (*set_val)(u64);
+ u64 (*set_clnt)(struct sockaddr_storage *, size_t);
};
static struct dentry *debug_dir;
-static u64 nfsd_inject_set(struct nfsd_fault_inject_op *op, u64 val)
-{
- u64 count;
-
- nfs4_lock_state();
- count = nfsd_for_n_state(val, op->forget);
- nfs4_unlock_state();
- return count;
-}
-
-static u64 nfsd_inject_set_client(struct nfsd_fault_inject_op *op,
- struct sockaddr_storage *addr,
- size_t addr_size)
-{
- struct nfs4_client *clp;
- u64 count = 0;
-
- nfs4_lock_state();
- clp = nfsd_find_client(addr, addr_size);
- if (clp)
- count = op->forget(clp, 0);
- nfs4_unlock_state();
- return count;
-}
-
-static u64 nfsd_inject_get(struct nfsd_fault_inject_op *op)
-{
- u64 count;
-
- nfs4_lock_state();
- count = nfsd_for_n_state(0, op->print);
- nfs4_unlock_state();
-
- return count;
-}
-
static ssize_t fault_inject_read(struct file *file, char __user *buf,
size_t len, loff_t *ppos)
{
@@ -73,7 +34,7 @@ static ssize_t fault_inject_read(struct file *file, char __user *buf,
struct nfsd_fault_inject_op *op = file_inode(file)->i_private;
if (!pos)
- val = op->get(op);
+ val = op->get();
size = scnprintf(read_buf, sizeof(read_buf), "%llu\n", val);
return simple_read_from_buffer(buf, len, ppos, read_buf, size);
@@ -103,7 +64,7 @@ static ssize_t fault_inject_write(struct file *file, const char __user *buf,
size = rpc_pton(net, write_buf, size, (struct sockaddr *)&sa, sizeof(sa));
if (size > 0) {
- val = op->set_clnt(op, &sa, size);
+ val = op->set_clnt(&sa, size);
if (val)
pr_info("NFSD [%s]: Client %s had %llu state object(s)\n",
op->file, write_buf, val);
@@ -114,7 +75,7 @@ static ssize_t fault_inject_write(struct file *file, const char __user *buf,
else
pr_info("NFSD Fault Injection: %s (n = %llu)",
op->file, val);
- val = op->set_val(op, val);
+ val = op->set_val(val);
pr_info("NFSD: %s: found %llu", op->file, val);
}
return len; /* on success, claim we got the whole input */
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f558b12af267..baf83cf96b26 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5725,8 +5725,25 @@ put_client(struct nfs4_client *clp)
atomic_dec(&clp->cl_refcount);
}
+static struct nfs4_client *
+nfsd_find_client(struct sockaddr_storage *addr, size_t addr_size)
+{
+ struct nfs4_client *clp;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+
+ if (!nfsd_netns_ready(nn))
+ return NULL;
+
+ list_for_each_entry(clp, &nn->client_lru, cl_lru) {
+ if (memcmp(&clp->cl_addr, addr, addr_size) == 0)
+ return clp;
+ }
+ return NULL;
+}
+
u64
-nfsd_inject_print_clients(struct nfsd_fault_inject_op *op)
+nfsd_inject_print_clients(void)
{
struct nfs4_client *clp;
u64 count = 0;
@@ -5749,8 +5766,7 @@ nfsd_inject_print_clients(struct nfsd_fault_inject_op *op)
}
u64
-nfsd_inject_forget_client(struct nfsd_fault_inject_op *op,
- struct sockaddr_storage *addr, size_t addr_size)
+nfsd_inject_forget_client(struct sockaddr_storage *addr, size_t addr_size)
{
u64 count = 0;
struct nfs4_client *clp;
@@ -5777,7 +5793,7 @@ nfsd_inject_forget_client(struct nfsd_fault_inject_op *op,
}
u64
-nfsd_inject_forget_clients(struct nfsd_fault_inject_op *op, u64 max)
+nfsd_inject_forget_clients(u64 max)
{
u64 count = 0;
struct nfs4_client *clp, *next;
@@ -5884,7 +5900,7 @@ nfsd_print_client_locks(struct nfs4_client *clp)
}
u64
-nfsd_inject_print_locks(struct nfsd_fault_inject_op *op)
+nfsd_inject_print_locks(void)
{
struct nfs4_client *clp;
u64 count = 0;
@@ -5917,8 +5933,7 @@ nfsd_reap_locks(struct list_head *reaplist)
}
u64
-nfsd_inject_forget_client_locks(struct nfsd_fault_inject_op *op,
- struct sockaddr_storage *addr, size_t addr_size)
+nfsd_inject_forget_client_locks(struct sockaddr_storage *addr, size_t addr_size)
{
unsigned int count = 0;
struct nfs4_client *clp;
@@ -5939,7 +5954,7 @@ nfsd_inject_forget_client_locks(struct nfsd_fault_inject_op *op,
}
u64
-nfsd_inject_forget_locks(struct nfsd_fault_inject_op *op, u64 max)
+nfsd_inject_forget_locks(u64 max)
{
u64 count = 0;
struct nfs4_client *clp;
@@ -6016,7 +6031,7 @@ nfsd_collect_client_openowners(struct nfs4_client *clp,
}
u64
-nfsd_inject_print_openowners(struct nfsd_fault_inject_op *op)
+nfsd_inject_print_openowners(void)
{
struct nfs4_client *clp;
u64 count = 0;
@@ -6049,8 +6064,8 @@ nfsd_reap_openowners(struct list_head *reaplist)
}
u64
-nfsd_inject_forget_client_openowners(struct nfsd_fault_inject_op *op,
- struct sockaddr_storage *addr, size_t addr_size)
+nfsd_inject_forget_client_openowners(struct sockaddr_storage *addr,
+ size_t addr_size)
{
unsigned int count = 0;
struct nfs4_client *clp;
@@ -6071,7 +6086,7 @@ nfsd_inject_forget_client_openowners(struct nfsd_fault_inject_op *op,
}
u64
-nfsd_inject_forget_openowners(struct nfsd_fault_inject_op *op, u64 max)
+nfsd_inject_forget_openowners(u64 max)
{
u64 count = 0;
struct nfs4_client *clp;
@@ -6145,7 +6160,7 @@ nfsd_print_client_delegations(struct nfs4_client *clp)
}
u64
-nfsd_inject_print_delegations(struct nfsd_fault_inject_op *op)
+nfsd_inject_print_delegations(void)
{
struct nfs4_client *clp;
u64 count = 0;
@@ -6178,8 +6193,8 @@ nfsd_forget_delegations(struct list_head *reaplist)
}
u64
-nfsd_inject_forget_client_delegations(struct nfsd_fault_inject_op *op,
- struct sockaddr_storage *addr, size_t addr_size)
+nfsd_inject_forget_client_delegations(struct sockaddr_storage *addr,
+ size_t addr_size)
{
u64 count = 0;
struct nfs4_client *clp;
@@ -6201,7 +6216,7 @@ nfsd_inject_forget_client_delegations(struct nfsd_fault_inject_op *op,
}
u64
-nfsd_inject_forget_delegations(struct nfsd_fault_inject_op *op, u64 max)
+nfsd_inject_forget_delegations(u64 max)
{
u64 count = 0;
struct nfs4_client *clp;
@@ -6247,8 +6262,7 @@ nfsd_recall_delegations(struct list_head *reaplist)
}
u64
-nfsd_inject_recall_client_delegations(struct nfsd_fault_inject_op *op,
- struct sockaddr_storage *addr,
+nfsd_inject_recall_client_delegations(struct sockaddr_storage *addr,
size_t addr_size)
{
u64 count = 0;
@@ -6271,7 +6285,7 @@ nfsd_inject_recall_client_delegations(struct nfsd_fault_inject_op *op,
}
u64
-nfsd_inject_recall_delegations(struct nfsd_fault_inject_op *op, u64 max)
+nfsd_inject_recall_delegations(u64 max)
{
u64 count = 0;
struct nfs4_client *clp, *next;
@@ -6292,41 +6306,6 @@ nfsd_inject_recall_delegations(struct nfsd_fault_inject_op *op, u64 max)
nfsd_recall_delegations(&reaplist);
return count;
}
-
-u64 nfsd_for_n_state(u64 max, u64 (*func)(struct nfs4_client *, u64))
-{
- struct nfs4_client *clp, *next;
- u64 count = 0;
- struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
- nfsd_net_id);
-
- if (!nfsd_netns_ready(nn))
- return 0;
-
- list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) {
- count += func(clp, max - count);
- if ((max != 0) && (count >= max))
- break;
- }
-
- return count;
-}
-
-struct nfs4_client *nfsd_find_client(struct sockaddr_storage *addr, size_t addr_size)
-{
- struct nfs4_client *clp;
- struct nfsd_net *nn = net_generic(current->nsproxy->net_ns, nfsd_net_id);
-
- if (!nfsd_netns_ready(nn))
- return NULL;
-
- list_for_each_entry(clp, &nn->client_lru, cl_lru) {
- if (memcmp(&clp->cl_addr, addr, addr_size) == 0)
- return clp;
- }
- return NULL;
-}
-
#endif /* CONFIG_NFSD_FAULT_INJECTION */
/*
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 1a9ba857cbab..a363f6bb621e 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -471,35 +471,26 @@ extern void nfsd4_record_grace_done(struct nfsd_net *nn, time_t boot_time);
/* nfs fault injection functions */
#ifdef CONFIG_NFSD_FAULT_INJECTION
-struct nfsd_fault_inject_op;
-
int nfsd_fault_inject_init(void);
void nfsd_fault_inject_cleanup(void);
-u64 nfsd_for_n_state(u64, u64 (*)(struct nfs4_client *, u64));
-struct nfs4_client *nfsd_find_client(struct sockaddr_storage *, size_t);
-
-u64 nfsd_inject_print_clients(struct nfsd_fault_inject_op *op);
-u64 nfsd_inject_forget_client(struct nfsd_fault_inject_op *,
- struct sockaddr_storage *, size_t);
-u64 nfsd_inject_forget_clients(struct nfsd_fault_inject_op *, u64);
-
-u64 nfsd_inject_print_locks(struct nfsd_fault_inject_op *);
-u64 nfsd_inject_forget_client_locks(struct nfsd_fault_inject_op *,
- struct sockaddr_storage *, size_t);
-u64 nfsd_inject_forget_locks(struct nfsd_fault_inject_op *, u64);
-
-u64 nfsd_inject_print_openowners(struct nfsd_fault_inject_op *);
-u64 nfsd_inject_forget_client_openowners(struct nfsd_fault_inject_op *,
- struct sockaddr_storage *, size_t);
-u64 nfsd_inject_forget_openowners(struct nfsd_fault_inject_op *, u64);
-
-u64 nfsd_inject_print_delegations(struct nfsd_fault_inject_op *);
-u64 nfsd_inject_forget_client_delegations(struct nfsd_fault_inject_op *,
- struct sockaddr_storage *, size_t);
-u64 nfsd_inject_forget_delegations(struct nfsd_fault_inject_op *, u64);
-u64 nfsd_inject_recall_client_delegations(struct nfsd_fault_inject_op *,
- struct sockaddr_storage *, size_t);
-u64 nfsd_inject_recall_delegations(struct nfsd_fault_inject_op *, u64);
+
+u64 nfsd_inject_print_clients(void);
+u64 nfsd_inject_forget_client(struct sockaddr_storage *, size_t);
+u64 nfsd_inject_forget_clients(u64);
+
+u64 nfsd_inject_print_locks(void);
+u64 nfsd_inject_forget_client_locks(struct sockaddr_storage *, size_t);
+u64 nfsd_inject_forget_locks(u64);
+
+u64 nfsd_inject_print_openowners(void);
+u64 nfsd_inject_forget_client_openowners(struct sockaddr_storage *, size_t);
+u64 nfsd_inject_forget_openowners(u64);
+
+u64 nfsd_inject_print_delegations(void);
+u64 nfsd_inject_forget_client_delegations(struct sockaddr_storage *, size_t);
+u64 nfsd_inject_forget_delegations(u64);
+u64 nfsd_inject_recall_client_delegations(struct sockaddr_storage *, size_t);
+u64 nfsd_inject_recall_delegations(u64);
#else /* CONFIG_NFSD_FAULT_INJECTION */
static inline int nfsd_fault_inject_init(void) { return 0; }
static inline void nfsd_fault_inject_cleanup(void) {}
--
1.9.3
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a33ce55925fb..de811dc39061 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -75,9 +75,6 @@ static void nfs4_free_ol_stateid(struct nfs4_stid *stid);
/* Locking: */
-/* Currently used for almost all code touching nfsv4 state: */
-static DEFINE_MUTEX(client_mutex);
-
/*
* Currently used for the del_recall_lru and file hash table. In an
* effort to decrease the scope of the client_mutex, this spinlock may
@@ -97,12 +94,6 @@ static struct kmem_cache *file_slab;
static struct kmem_cache *stateid_slab;
static struct kmem_cache *deleg_slab;
-void
-nfs4_lock_state(void)
-{
- mutex_lock(&client_mutex);
-}
-
static void free_session(struct nfsd4_session *);
static bool is_session_dead(struct nfsd4_session *ses)
@@ -118,12 +109,6 @@ static __be32 mark_session_dead_locked(struct nfsd4_session *ses, int ref_held_b
return nfs_ok;
}
-void
-nfs4_unlock_state(void)
-{
- mutex_unlock(&client_mutex);
-}
-
static bool is_client_expired(struct nfs4_client *clp)
{
return clp->cl_time == 0;
--
1.9.3
...which uses the client_lock for protection instead of client_mutex.
Also remove nfsd_forget_client as there are no more callers.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/fault_inject.c | 3 +--
fs/nfsd/nfs4state.c | 42 ++++++++++++++++++++++++++++--------------
fs/nfsd/state.h | 2 +-
3 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
index 5f3ead0c72fb..76ecdff37ea2 100644
--- a/fs/nfsd/fault_inject.c
+++ b/fs/nfsd/fault_inject.c
@@ -135,9 +135,8 @@ static struct nfsd_fault_inject_op inject_ops[] = {
{
.file = "forget_clients",
.get = nfsd_inject_print_clients,
- .set_val = nfsd_inject_set,
+ .set_val = nfsd_inject_forget_clients,
.set_clnt = nfsd_inject_forget_client,
- .forget = nfsd_forget_client,
},
{
.file = "forget_locks",
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2dd9819e205b..8cc3a8c1c2a9 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5742,20 +5742,6 @@ nfsd_inject_print_clients(struct nfsd_fault_inject_op *op)
return count;
}
-u64 nfsd_forget_client(struct nfs4_client *clp, u64 max)
-{
- __be32 ret;
- struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
-
- spin_lock(&nn->client_lock);
- ret = mark_client_expired_locked(clp);
- spin_unlock(&nn->client_lock);
- if (ret != nfs_ok)
- return 0;
- expire_client(clp);
- return 1;
-}
-
u64
nfsd_inject_forget_client(struct nfsd_fault_inject_op *op,
struct sockaddr_storage *addr, size_t addr_size)
@@ -5784,6 +5770,34 @@ nfsd_inject_forget_client(struct nfsd_fault_inject_op *op,
return count;
}
+u64
+nfsd_inject_forget_clients(struct nfsd_fault_inject_op *op, u64 max)
+{
+ u64 count = 0;
+ struct nfs4_client *clp, *next;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+ LIST_HEAD(reaplist);
+
+ if (!nfsd_netns_ready(nn))
+ return count;
+
+ spin_lock(&nn->client_lock);
+ list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) {
+ if (mark_client_expired_locked(clp) == nfs_ok) {
+ list_add(&clp->cl_lru, &reaplist);
+ if (max != 0 && ++count >= max)
+ break;
+ }
+ }
+ spin_unlock(&nn->client_lock);
+
+ list_for_each_entry_safe(clp, next, &reaplist, cl_lru)
+ expire_client(clp);
+
+ return count;
+}
+
static void nfsd_print_count(struct nfs4_client *clp, unsigned int count,
const char *type)
{
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index e37520e1f767..480fce8d5cc8 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -479,9 +479,9 @@ u64 nfsd_for_n_state(u64, u64 (*)(struct nfs4_client *, u64));
struct nfs4_client *nfsd_find_client(struct sockaddr_storage *, size_t);
u64 nfsd_inject_print_clients(struct nfsd_fault_inject_op *op);
-u64 nfsd_forget_client(struct nfs4_client *, u64);
u64 nfsd_inject_forget_client(struct nfsd_fault_inject_op *,
struct sockaddr_storage *, size_t);
+u64 nfsd_inject_forget_clients(struct nfsd_fault_inject_op *, u64);
u64 nfsd_forget_client_locks(struct nfs4_client*, u64);
u64 nfsd_forget_client_openowners(struct nfs4_client *, u64);
--
1.9.3
From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index eba119a46add..ae261d0e62e1 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4851,7 +4851,6 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
return status;
- nfs4_lock_state();
status = nfsd4_lookup_stateid(cstate, stateid, NFS4_DELEG_STID, &s, nn);
if (status)
goto out;
@@ -4864,8 +4863,6 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
put_stateid:
nfs4_put_stid(&dp->dl_stid);
out:
- nfs4_unlock_state();
-
return status;
}
--
1.9.3
From: Trond Myklebust <[email protected]>
...instead of relying on the client_mutex.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 52a4677f6f35..68383b09c7dc 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2826,22 +2826,23 @@ nfsd4_sequence_done(struct nfsd4_compoundres *resp)
__be32
nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfsd4_destroy_clientid *dc)
{
- struct nfs4_client *conf, *unconf, *clp;
+ struct nfs4_client *conf, *unconf;
+ struct nfs4_client *clp = NULL;
__be32 status = 0;
struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
nfs4_lock_state();
+ spin_lock(&nn->client_lock);
unconf = find_unconfirmed_client(&dc->clientid, true, nn);
conf = find_confirmed_client(&dc->clientid, true, nn);
WARN_ON_ONCE(conf && unconf);
if (conf) {
- clp = conf;
-
if (client_has_state(conf)) {
status = nfserr_clientid_busy;
goto out;
}
+ clp = conf;
} else if (unconf)
clp = unconf;
else {
@@ -2849,12 +2850,16 @@ nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
goto out;
}
if (!mach_creds_match(clp, rqstp)) {
+ clp = NULL;
status = nfserr_wrong_cred;
goto out;
}
- expire_client(clp);
+ unhash_client_locked(clp);
out:
+ spin_unlock(&nn->client_lock);
nfs4_unlock_state();
+ if (clp)
+ expire_client(clp);
return status;
}
--
1.9.3
From: Trond Myklebust <[email protected]>
For efficiency reasons, and because we want to use spin locks instead
of relying on the client_mutex.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 35 +++++++++++++++++++----------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 256e9032f49c..4b42cb95e315 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2181,6 +2181,10 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
return nfserr_encr_alg_unsupp;
}
+ new = create_client(exid->clname, rqstp, &verf);
+ if (new == NULL)
+ return nfserr_jukebox;
+
/* Cases below refer to rfc 5661 section 18.35.4: */
nfs4_lock_state();
conf = find_confirmed_client_by_name(&exid->clname, nn);
@@ -2207,7 +2211,6 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
}
/* case 6 */
exid->flags |= EXCHGID4_FLAG_CONFIRMED_R;
- new = conf;
goto out_copy;
}
if (!creds_match) { /* case 3 */
@@ -2220,7 +2223,6 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
}
if (verfs_match) { /* case 2 */
conf->cl_exchange_flags |= EXCHGID4_FLAG_CONFIRMED_R;
- new = conf;
goto out_copy;
}
/* case 5, client reboot */
@@ -2238,29 +2240,28 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
/* case 1 (normal case) */
out_new:
- new = create_client(exid->clname, rqstp, &verf);
- if (new == NULL) {
- status = nfserr_jukebox;
- goto out;
- }
new->cl_minorversion = cstate->minorversion;
new->cl_mach_cred = (exid->spa_how == SP4_MACH_CRED);
gen_clid(new, nn);
add_to_unconfirmed(new);
+ conf = new;
+ new = NULL;
out_copy:
- exid->clientid.cl_boot = new->cl_clientid.cl_boot;
- exid->clientid.cl_id = new->cl_clientid.cl_id;
+ exid->clientid.cl_boot = conf->cl_clientid.cl_boot;
+ exid->clientid.cl_id = conf->cl_clientid.cl_id;
- exid->seqid = new->cl_cs_slot.sl_seqid + 1;
- nfsd4_set_ex_flags(new, exid);
+ exid->seqid = conf->cl_cs_slot.sl_seqid + 1;
+ nfsd4_set_ex_flags(conf, exid);
dprintk("nfsd4_exchange_id seqid %d flags %x\n",
- new->cl_cs_slot.sl_seqid, new->cl_exchange_flags);
+ conf->cl_cs_slot.sl_seqid, conf->cl_exchange_flags);
status = nfs_ok;
out:
nfs4_unlock_state();
+ if (new)
+ free_client(new);
return status;
}
@@ -2903,6 +2904,9 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
__be32 status;
struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+ new = create_client(clname, rqstp, &clverifier);
+ if (new == NULL)
+ return nfserr_jukebox;
/* Cases below refer to rfc 3530 section 14.2.33: */
nfs4_lock_state();
conf = find_confirmed_client_by_name(&clname, nn);
@@ -2923,10 +2927,6 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
unconf = find_unconfirmed_client_by_name(&clname, nn);
if (unconf)
expire_client(unconf);
- status = nfserr_jukebox;
- new = create_client(clname, rqstp, &clverifier);
- if (new == NULL)
- goto out;
if (conf && same_verf(&conf->cl_verifier, &clverifier))
/* case 1: probable callback update */
copy_clid(new, conf);
@@ -2938,9 +2938,12 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
setclid->se_clientid.cl_boot = new->cl_clientid.cl_boot;
setclid->se_clientid.cl_id = new->cl_clientid.cl_id;
memcpy(setclid->se_confirm.data, new->cl_confirm.data, sizeof(setclid->se_confirm.data));
+ new = NULL;
status = nfs_ok;
out:
nfs4_unlock_state();
+ if (new)
+ free_client(new);
return status;
}
--
1.9.3
...instead of relying on the client_mutex.
Also, fix up the printk output that is generated when the file is read.
It currently says that it's reporting the number of open files, but
it's actually reporting the number of openowners.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/fault_inject.c | 8 ++--
fs/nfsd/nfs4state.c | 122 +++++++++++++++++++++++++++++++++++++++++++++----
fs/nfsd/state.h | 7 ++-
3 files changed, 122 insertions(+), 15 deletions(-)
diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
index a444d821d2a5..d4472cd19807 100644
--- a/fs/nfsd/fault_inject.c
+++ b/fs/nfsd/fault_inject.c
@@ -146,11 +146,9 @@ static struct nfsd_fault_inject_op inject_ops[] = {
},
{
.file = "forget_openowners",
- .get = nfsd_inject_get,
- .set_val = nfsd_inject_set,
- .set_clnt = nfsd_inject_set_client,
- .forget = nfsd_forget_client_openowners,
- .print = nfsd_print_client_openowners,
+ .get = nfsd_inject_print_openowners,
+ .set_val = nfsd_inject_forget_openowners,
+ .set_clnt = nfsd_inject_forget_client_openowners,
},
{
.file = "forget_delegations",
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 594aa01806d1..996287e1f0be 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5961,30 +5961,136 @@ nfsd_inject_forget_locks(struct nfsd_fault_inject_op *op, u64 max)
return count;
}
-static u64 nfsd_foreach_client_open(struct nfs4_client *clp, u64 max, void (*func)(struct nfs4_openowner *))
+static u64
+nfsd_foreach_client_openowner(struct nfs4_client *clp, u64 max,
+ struct list_head *collect,
+ void (*func)(struct nfs4_openowner *))
{
struct nfs4_openowner *oop, *next;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
u64 count = 0;
+ lockdep_assert_held(&nn->client_lock);
+
+ spin_lock(&clp->cl_lock);
list_for_each_entry_safe(oop, next, &clp->cl_openowners, oo_perclient) {
- if (func)
+ if (func) {
func(oop);
- if (++count == max)
+ if (collect) {
+ atomic_inc(&clp->cl_refcount);
+ list_add(&oop->oo_perclient, collect);
+ }
+ }
+ ++count;
+ /*
+ * Despite the fact that these functions deal with
+ * 64-bit integers for "count", we must ensure that
+ * it doesn't blow up the clp->cl_refcount. Throw a
+ * warning if we start to approach INT_MAX here.
+ */
+ WARN_ON_ONCE(count == (INT_MAX / 2));
+ if (count == max)
break;
}
+ spin_unlock(&clp->cl_lock);
+
+ return count;
+}
+static u64
+nfsd_print_client_openowners(struct nfs4_client *clp)
+{
+ u64 count = nfsd_foreach_client_openowner(clp, 0, NULL, NULL);
+
+ nfsd_print_count(clp, count, "openowners");
return count;
}
-u64 nfsd_forget_client_openowners(struct nfs4_client *clp, u64 max)
+static u64
+nfsd_collect_client_openowners(struct nfs4_client *clp,
+ struct list_head *collect, u64 max)
{
- return nfsd_foreach_client_open(clp, max, release_openowner);
+ return nfsd_foreach_client_openowner(clp, max, collect,
+ unhash_openowner_locked);
}
-u64 nfsd_print_client_openowners(struct nfs4_client *clp, u64 max)
+u64
+nfsd_inject_print_openowners(struct nfsd_fault_inject_op *op)
{
- u64 count = nfsd_foreach_client_open(clp, max, NULL);
- nfsd_print_count(clp, count, "open files");
+ struct nfs4_client *clp;
+ u64 count = 0;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+
+ if (!nfsd_netns_ready(nn))
+ return 0;
+
+ spin_lock(&nn->client_lock);
+ list_for_each_entry(clp, &nn->client_lru, cl_lru)
+ count += nfsd_print_client_openowners(clp);
+ spin_unlock(&nn->client_lock);
+
+ return count;
+}
+
+static void
+nfsd_reap_openowners(struct list_head *reaplist)
+{
+ struct nfs4_client *clp;
+ struct nfs4_openowner *oop, *next;
+
+ list_for_each_entry_safe(oop, next, reaplist, oo_perclient) {
+ list_del_init(&oop->oo_perclient);
+ clp = oop->oo_owner.so_client;
+ release_openowner(oop);
+ put_client(clp);
+ }
+}
+
+u64
+nfsd_inject_forget_client_openowners(struct nfsd_fault_inject_op *op,
+ struct sockaddr_storage *addr, size_t addr_size)
+{
+ unsigned int count = 0;
+ struct nfs4_client *clp;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+ LIST_HEAD(reaplist);
+
+ if (!nfsd_netns_ready(nn))
+ return count;
+
+ spin_lock(&nn->client_lock);
+ clp = nfsd_find_client(addr, addr_size);
+ if (clp)
+ count = nfsd_collect_client_openowners(clp, &reaplist, 0);
+ spin_unlock(&nn->client_lock);
+ nfsd_reap_openowners(&reaplist);
+ return count;
+}
+
+u64
+nfsd_inject_forget_openowners(struct nfsd_fault_inject_op *op, u64 max)
+{
+ u64 count = 0;
+ struct nfs4_client *clp;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+ LIST_HEAD(reaplist);
+
+ if (!nfsd_netns_ready(nn))
+ return count;
+
+ spin_lock(&nn->client_lock);
+ list_for_each_entry(clp, &nn->client_lru, cl_lru) {
+ count += nfsd_collect_client_openowners(clp, &reaplist,
+ max - count);
+ if (max != 0 && count >= max)
+ break;
+ }
+ spin_unlock(&nn->client_lock);
+ nfsd_reap_openowners(&reaplist);
return count;
}
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index cfd6277b966d..881ea7d5c69e 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -488,11 +488,14 @@ u64 nfsd_inject_forget_client_locks(struct nfsd_fault_inject_op *,
struct sockaddr_storage *, size_t);
u64 nfsd_inject_forget_locks(struct nfsd_fault_inject_op *, u64);
-u64 nfsd_forget_client_openowners(struct nfs4_client *, u64);
+u64 nfsd_inject_print_openowners(struct nfsd_fault_inject_op *);
+u64 nfsd_inject_forget_client_openowners(struct nfsd_fault_inject_op *,
+ struct sockaddr_storage *, size_t);
+u64 nfsd_inject_forget_openowners(struct nfsd_fault_inject_op *, u64);
+
u64 nfsd_forget_client_delegations(struct nfs4_client *, u64);
u64 nfsd_recall_client_delegations(struct nfs4_client *, u64);
-u64 nfsd_print_client_openowners(struct nfs4_client *, u64);
u64 nfsd_print_client_delegations(struct nfs4_client *, u64);
#else /* CONFIG_NFSD_FAULT_INJECTION */
static inline int nfsd_fault_inject_init(void) { return 0; }
--
1.9.3
In a later patch, we'll want to collect the locks onto a list for later
destruction. If "func" is defined and "collect" is defined, then we'll
add the lock stateid to the list.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8cc3a8c1c2a9..b6020e4a2102 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5807,6 +5807,7 @@ static void nfsd_print_count(struct nfs4_client *clp, unsigned int count,
}
static u64 nfsd_foreach_client_lock(struct nfs4_client *clp, u64 max,
+ struct list_head *collect,
void (*func)(struct nfs4_ol_stateid *))
{
struct nfs4_openowner *oop;
@@ -5819,8 +5820,12 @@ static u64 nfsd_foreach_client_lock(struct nfs4_client *clp, u64 max,
&oop->oo_owner.so_stateids, st_perstateowner) {
list_for_each_entry_safe(lst, lst_next,
&stp->st_locks, st_locks) {
- if (func)
+ if (func) {
func(lst);
+ if (collect)
+ list_add(&lst->st_locks,
+ collect);
+ }
if (++count == max)
return count;
}
@@ -5832,12 +5837,12 @@ static u64 nfsd_foreach_client_lock(struct nfs4_client *clp, u64 max,
u64 nfsd_forget_client_locks(struct nfs4_client *clp, u64 max)
{
- return nfsd_foreach_client_lock(clp, max, release_lock_stateid);
+ return nfsd_foreach_client_lock(clp, max, NULL, release_lock_stateid);
}
u64 nfsd_print_client_locks(struct nfs4_client *clp, u64 max)
{
- u64 count = nfsd_foreach_client_lock(clp, max, NULL);
+ u64 count = nfsd_foreach_client_lock(clp, max, NULL, NULL);
nfsd_print_count(clp, count, "locked files");
return count;
}
--
1.9.3
All the callers except for the fault injection code call it directly
afterward, and in the fault injection case it won't hurt to do so
anyway.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 818480035453..56999cbe84a7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -129,14 +129,6 @@ static bool is_client_expired(struct nfs4_client *clp)
return clp->cl_time == 0;
}
-static __be32 mark_client_expired_locked(struct nfs4_client *clp)
-{
- if (atomic_read(&clp->cl_refcount))
- return nfserr_jukebox;
- clp->cl_time = 0;
- return nfs_ok;
-}
-
static __be32 get_client_locked(struct nfs4_client *clp)
{
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
@@ -1628,6 +1620,14 @@ unhash_client(struct nfs4_client *clp)
spin_unlock(&nn->client_lock);
}
+static __be32 mark_client_expired_locked(struct nfs4_client *clp)
+{
+ if (atomic_read(&clp->cl_refcount))
+ return nfserr_jukebox;
+ unhash_client_locked(clp);
+ return nfs_ok;
+}
+
static void
__destroy_client(struct nfs4_client *clp)
{
@@ -2498,7 +2498,6 @@ nfsd4_create_session(struct svc_rqst *rqstp,
status = mark_client_expired_locked(old);
if (status)
goto out_free_conn;
- unhash_client_locked(old);
}
move_to_confirmed(unconf);
conf = unconf;
@@ -3044,7 +3043,6 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
status = mark_client_expired_locked(old);
if (status)
goto out;
- unhash_client_locked(old);
}
move_to_confirmed(unconf);
conf = unconf;
@@ -4183,7 +4181,6 @@ nfs4_laundromat(struct nfsd_net *nn)
clp->cl_clientid.cl_id);
continue;
}
- unhash_client_locked(clp);
list_add(&clp->cl_lru, &reaplist);
}
spin_unlock(&nn->client_lock);
--
1.9.3
From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index baf83cf96b26..9013a30874e3 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4461,13 +4461,11 @@ nfs4_preprocess_stateid_op(struct net *net, struct nfsd4_compound_state *cstate,
if (ZERO_STATEID(stateid) || ONE_STATEID(stateid))
return check_special_stateids(net, current_fh, stateid, flags);
- nfs4_lock_state();
-
status = nfsd4_lookup_stateid(cstate, stateid,
NFS4_DELEG_STID|NFS4_OPEN_STID|NFS4_LOCK_STID,
&s, nn);
if (status)
- goto unlock_state;
+ return status;
status = check_stateid_generation(stateid, &s->sc_stateid, nfsd4_has_session(cstate));
if (status)
goto out;
@@ -4517,8 +4515,6 @@ nfs4_preprocess_stateid_op(struct net *net, struct nfsd4_compound_state *cstate,
*filpp = file;
out:
nfs4_put_stid(s);
-unlock_state:
- nfs4_unlock_state();
return status;
}
--
1.9.3
From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4proc.c | 3 ---
fs/nfsd/nfs4state.c | 6 ------
2 files changed, 9 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 29cf395b694e..5e0dc528a0e8 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -385,8 +385,6 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (nfsd4_has_session(cstate))
copy_clientid(&open->op_clientid, cstate->session);
- nfs4_lock_state();
-
/* check seqid for replay. set nfs4_owner */
resp = rqstp->rq_resp;
status = nfsd4_process_open1(&resp->cstate, open, nn);
@@ -471,7 +469,6 @@ out:
}
nfsd4_cleanup_open_state(cstate, open, status);
nfsd4_bump_seqid(cstate, status);
- nfs4_unlock_state();
return status;
}
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ae261d0e62e1..02caef874d67 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4003,9 +4003,6 @@ static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
*/
}
-/*
- * called with nfs4_lock_state() held.
- */
__be32
nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_open *open)
{
@@ -4686,8 +4683,6 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (status)
return status;
- nfs4_lock_state();
-
status = nfs4_preprocess_seqid_op(cstate,
oc->oc_seqid, &oc->oc_req_stateid,
NFS4_OPEN_STID, &stp, nn);
@@ -4781,7 +4776,6 @@ put_stateid:
nfs4_put_stid(&stp->st_stid);
out:
nfsd4_bump_seqid(cstate, status);
- nfs4_unlock_state();
return status;
}
--
1.9.3
...instead of relying on the client_mutex.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/fault_inject.c | 16 ++---
fs/nfsd/nfs4state.c | 175 +++++++++++++++++++++++++++++++++++++++++--------
fs/nfsd/state.h | 11 ++--
3 files changed, 162 insertions(+), 40 deletions(-)
diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
index d4472cd19807..2479dba71c3c 100644
--- a/fs/nfsd/fault_inject.c
+++ b/fs/nfsd/fault_inject.c
@@ -152,19 +152,15 @@ static struct nfsd_fault_inject_op inject_ops[] = {
},
{
.file = "forget_delegations",
- .get = nfsd_inject_get,
- .set_val = nfsd_inject_set,
- .set_clnt = nfsd_inject_set_client,
- .forget = nfsd_forget_client_delegations,
- .print = nfsd_print_client_delegations,
+ .get = nfsd_inject_print_delegations,
+ .set_val = nfsd_inject_forget_delegations,
+ .set_clnt = nfsd_inject_forget_client_delegations,
},
{
.file = "recall_delegations",
- .get = nfsd_inject_get,
- .set_val = nfsd_inject_set,
- .set_clnt = nfsd_inject_set_client,
- .forget = nfsd_recall_client_delegations,
- .print = nfsd_print_client_delegations,
+ .get = nfsd_inject_print_delegations,
+ .set_val = nfsd_inject_recall_delegations,
+ .set_clnt = nfsd_inject_recall_client_delegations,
},
};
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 996287e1f0be..f558b12af267 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6098,9 +6098,13 @@ static u64 nfsd_find_all_delegations(struct nfs4_client *clp, u64 max,
struct list_head *victims)
{
struct nfs4_delegation *dp, *next;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
u64 count = 0;
- lockdep_assert_held(&state_lock);
+ lockdep_assert_held(&nn->client_lock);
+
+ spin_lock(&state_lock);
list_for_each_entry_safe(dp, next, &clp->cl_delegations, dl_perclnt) {
if (victims) {
/*
@@ -6112,62 +6116,180 @@ static u64 nfsd_find_all_delegations(struct nfs4_client *clp, u64 max,
if (dp->dl_time != 0)
continue;
+ atomic_inc(&clp->cl_refcount);
unhash_delegation_locked(dp);
list_add(&dp->dl_recall_lru, victims);
}
- if (++count == max)
+ ++count;
+ /*
+ * Despite the fact that these functions deal with
+ * 64-bit integers for "count", we must ensure that
+ * it doesn't blow up the clp->cl_refcount. Throw a
+ * warning if we start to approach INT_MAX here.
+ */
+ WARN_ON_ONCE(count == (INT_MAX / 2));
+ if (count == max)
break;
}
+ spin_unlock(&state_lock);
return count;
}
-u64 nfsd_forget_client_delegations(struct nfs4_client *clp, u64 max)
+static u64
+nfsd_print_client_delegations(struct nfs4_client *clp)
{
- struct nfs4_delegation *dp, *next;
- LIST_HEAD(victims);
- u64 count;
+ u64 count = nfsd_find_all_delegations(clp, 0, NULL);
- spin_lock(&state_lock);
- count = nfsd_find_all_delegations(clp, max, &victims);
- spin_unlock(&state_lock);
+ nfsd_print_count(clp, count, "delegations");
+ return count;
+}
+
+u64
+nfsd_inject_print_delegations(struct nfsd_fault_inject_op *op)
+{
+ struct nfs4_client *clp;
+ u64 count = 0;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+
+ if (!nfsd_netns_ready(nn))
+ return 0;
- list_for_each_entry_safe(dp, next, &victims, dl_recall_lru) {
+ spin_lock(&nn->client_lock);
+ list_for_each_entry(clp, &nn->client_lru, cl_lru)
+ count += nfsd_print_client_delegations(clp);
+ spin_unlock(&nn->client_lock);
+
+ return count;
+}
+
+static void
+nfsd_forget_delegations(struct list_head *reaplist)
+{
+ struct nfs4_client *clp;
+ struct nfs4_delegation *dp, *next;
+
+ list_for_each_entry_safe(dp, next, reaplist, dl_recall_lru) {
list_del_init(&dp->dl_recall_lru);
+ clp = dp->dl_stid.sc_client;
revoke_delegation(dp);
+ put_client(clp);
}
+}
+
+u64
+nfsd_inject_forget_client_delegations(struct nfsd_fault_inject_op *op,
+ struct sockaddr_storage *addr, size_t addr_size)
+{
+ u64 count = 0;
+ struct nfs4_client *clp;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+ LIST_HEAD(reaplist);
+
+ if (!nfsd_netns_ready(nn))
+ return count;
+
+ spin_lock(&nn->client_lock);
+ clp = nfsd_find_client(addr, addr_size);
+ if (clp)
+ count = nfsd_find_all_delegations(clp, 0, &reaplist);
+ spin_unlock(&nn->client_lock);
+
+ nfsd_forget_delegations(&reaplist);
+ return count;
+}
+u64
+nfsd_inject_forget_delegations(struct nfsd_fault_inject_op *op, u64 max)
+{
+ u64 count = 0;
+ struct nfs4_client *clp;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+ LIST_HEAD(reaplist);
+
+ if (!nfsd_netns_ready(nn))
+ return count;
+
+ spin_lock(&nn->client_lock);
+ list_for_each_entry(clp, &nn->client_lru, cl_lru) {
+ count += nfsd_find_all_delegations(clp, max - count, &reaplist);
+ if (max != 0 && count >= max)
+ break;
+ }
+ spin_unlock(&nn->client_lock);
+ nfsd_forget_delegations(&reaplist);
return count;
}
-u64 nfsd_recall_client_delegations(struct nfs4_client *clp, u64 max)
+static void
+nfsd_recall_delegations(struct list_head *reaplist)
{
- struct nfs4_delegation *dp;
- LIST_HEAD(victims);
- u64 count;
+ struct nfs4_client *clp;
+ struct nfs4_delegation *dp, *next;
- spin_lock(&state_lock);
- count = nfsd_find_all_delegations(clp, max, &victims);
- while (!list_empty(&victims)) {
- dp = list_first_entry(&victims, struct nfs4_delegation,
- dl_recall_lru);
+ list_for_each_entry_safe(dp, next, reaplist, dl_recall_lru) {
list_del_init(&dp->dl_recall_lru);
+ clp = dp->dl_stid.sc_client;
+ /*
+ * We skipped all entries that had a zero dl_time before,
+ * so we can now reset the dl_time back to 0. If a delegation
+ * break comes in now, then it won't make any difference since
+ * we're recalling it either way.
+ */
+ spin_lock(&state_lock);
dp->dl_time = 0;
+ spin_unlock(&state_lock);
nfsd_break_one_deleg(dp);
+ put_client(clp);
}
- spin_unlock(&state_lock);
+}
+u64
+nfsd_inject_recall_client_delegations(struct nfsd_fault_inject_op *op,
+ struct sockaddr_storage *addr,
+ size_t addr_size)
+{
+ u64 count = 0;
+ struct nfs4_client *clp;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+ LIST_HEAD(reaplist);
+
+ if (!nfsd_netns_ready(nn))
+ return count;
+
+ spin_lock(&nn->client_lock);
+ clp = nfsd_find_client(addr, addr_size);
+ if (clp)
+ count = nfsd_find_all_delegations(clp, 0, &reaplist);
+ spin_unlock(&nn->client_lock);
+
+ nfsd_recall_delegations(&reaplist);
return count;
}
-u64 nfsd_print_client_delegations(struct nfs4_client *clp, u64 max)
+u64
+nfsd_inject_recall_delegations(struct nfsd_fault_inject_op *op, u64 max)
{
u64 count = 0;
+ struct nfs4_client *clp, *next;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+ LIST_HEAD(reaplist);
- spin_lock(&state_lock);
- count = nfsd_find_all_delegations(clp, max, NULL);
- spin_unlock(&state_lock);
+ if (!nfsd_netns_ready(nn))
+ return count;
- nfsd_print_count(clp, count, "delegations");
+ spin_lock(&nn->client_lock);
+ list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) {
+ count += nfsd_find_all_delegations(clp, max - count, &reaplist);
+ if (max != 0 && ++count >= max)
+ break;
+ }
+ spin_unlock(&nn->client_lock);
+ nfsd_recall_delegations(&reaplist);
return count;
}
@@ -6175,7 +6297,8 @@ u64 nfsd_for_n_state(u64 max, u64 (*func)(struct nfs4_client *, u64))
{
struct nfs4_client *clp, *next;
u64 count = 0;
- struct nfsd_net *nn = net_generic(current->nsproxy->net_ns, nfsd_net_id);
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
if (!nfsd_netns_ready(nn))
return 0;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 881ea7d5c69e..1a9ba857cbab 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -493,10 +493,13 @@ u64 nfsd_inject_forget_client_openowners(struct nfsd_fault_inject_op *,
struct sockaddr_storage *, size_t);
u64 nfsd_inject_forget_openowners(struct nfsd_fault_inject_op *, u64);
-u64 nfsd_forget_client_delegations(struct nfs4_client *, u64);
-u64 nfsd_recall_client_delegations(struct nfs4_client *, u64);
-
-u64 nfsd_print_client_delegations(struct nfs4_client *, u64);
+u64 nfsd_inject_print_delegations(struct nfsd_fault_inject_op *);
+u64 nfsd_inject_forget_client_delegations(struct nfsd_fault_inject_op *,
+ struct sockaddr_storage *, size_t);
+u64 nfsd_inject_forget_delegations(struct nfsd_fault_inject_op *, u64);
+u64 nfsd_inject_recall_client_delegations(struct nfsd_fault_inject_op *,
+ struct sockaddr_storage *, size_t);
+u64 nfsd_inject_recall_delegations(struct nfsd_fault_inject_op *, u64);
#else /* CONFIG_NFSD_FAULT_INJECTION */
static inline int nfsd_fault_inject_init(void) { return 0; }
static inline void nfsd_fault_inject_cleanup(void) {}
--
1.9.3
From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f9d077d800ee..e7dfd4e9d942 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -139,6 +139,10 @@ static __be32 mark_client_expired_locked(struct nfs4_client *clp)
static __be32 get_client_locked(struct nfs4_client *clp)
{
+ struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+
+ lockdep_assert_held(&nn->client_lock);
+
if (is_client_expired(clp))
return nfserr_expired;
atomic_inc(&clp->cl_refcount);
@@ -179,6 +183,10 @@ renew_client(struct nfs4_client *clp)
static void put_client_renew_locked(struct nfs4_client *clp)
{
+ struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+
+ lockdep_assert_held(&nn->client_lock);
+
if (!atomic_dec_and_test(&clp->cl_refcount))
return;
if (!is_client_expired(clp))
@@ -212,6 +220,9 @@ static __be32 nfsd4_get_session_locked(struct nfsd4_session *ses)
static void nfsd4_put_session_locked(struct nfsd4_session *ses)
{
struct nfs4_client *clp = ses->se_client;
+ struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+
+ lockdep_assert_held(&nn->client_lock);
if (atomic_dec_and_test(&ses->se_ref) && is_session_dead(ses))
free_session(ses);
@@ -1453,6 +1464,8 @@ __find_in_sessionid_hashtbl(struct nfs4_sessionid *sessionid, struct net *net)
int idx;
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+ lockdep_assert_held(&nn->client_lock);
+
dump_sessionid(__func__, sessionid);
idx = hash_sessionid(sessionid);
/* Search in the appropriate list */
@@ -1489,6 +1502,11 @@ out:
static void
unhash_session(struct nfsd4_session *ses)
{
+ struct nfs4_client *clp = ses->se_client;
+ struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+
+ lockdep_assert_held(&nn->client_lock);
+
list_del(&ses->se_hash);
spin_lock(&ses->se_client->cl_lock);
list_del(&ses->se_perclnt);
@@ -1575,6 +1593,8 @@ unhash_client_locked(struct nfs4_client *clp)
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
struct nfsd4_session *ses;
+ lockdep_assert_held(&nn->client_lock);
+
/* Mark the client as expired! */
clp->cl_time = 0;
/* Make it invisible */
@@ -1906,6 +1926,8 @@ add_to_unconfirmed(struct nfs4_client *clp)
unsigned int idhashval;
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+ lockdep_assert_held(&nn->client_lock);
+
clear_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags);
add_clp_to_name_tree(clp, &nn->unconf_name_tree);
idhashval = clientid_hashval(clp->cl_clientid.cl_id);
@@ -1919,6 +1941,8 @@ move_to_confirmed(struct nfs4_client *clp)
unsigned int idhashval = clientid_hashval(clp->cl_clientid.cl_id);
struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+ lockdep_assert_held(&nn->client_lock);
+
dprintk("NFSD: move_to_confirm nfs4_client %p\n", clp);
list_move(&clp->cl_idhash, &nn->conf_id_hashtbl[idhashval]);
rb_erase(&clp->cl_namenode, &nn->unconf_name_tree);
@@ -1949,6 +1973,7 @@ find_confirmed_client(clientid_t *clid, bool sessions, struct nfsd_net *nn)
{
struct list_head *tbl = nn->conf_id_hashtbl;
+ lockdep_assert_held(&nn->client_lock);
return find_client_in_id_table(tbl, clid, sessions);
}
@@ -1957,6 +1982,7 @@ find_unconfirmed_client(clientid_t *clid, bool sessions, struct nfsd_net *nn)
{
struct list_head *tbl = nn->unconf_id_hashtbl;
+ lockdep_assert_held(&nn->client_lock);
return find_client_in_id_table(tbl, clid, sessions);
}
@@ -1968,12 +1994,14 @@ static bool clp_used_exchangeid(struct nfs4_client *clp)
static struct nfs4_client *
find_confirmed_client_by_name(struct xdr_netobj *name, struct nfsd_net *nn)
{
+ lockdep_assert_held(&nn->client_lock);
return find_clp_in_name_tree(name, &nn->conf_name_tree);
}
static struct nfs4_client *
find_unconfirmed_client_by_name(struct xdr_netobj *name, struct nfsd_net *nn)
{
+ lockdep_assert_held(&nn->client_lock);
return find_clp_in_name_tree(name, &nn->unconf_name_tree);
}
@@ -4907,6 +4935,8 @@ find_lockowner_str_locked(clientid_t *clid, struct xdr_netobj *owner,
unsigned int strhashval = ownerstr_hashval(owner);
struct nfs4_stateowner *so;
+ lockdep_assert_held(&clp->cl_lock);
+
list_for_each_entry(so, &clp->cl_ownerstr_hashtbl[strhashval],
so_strhash) {
if (so->so_is_open_owner)
--
1.9.3
It's possible that we'll have an in-progress call on some of the clients
while a rogue EXCHANGE_ID or DESTROY_CLIENTID call comes in. Be sure to
try and mark the client expired first, so that the refcount is
respected.
This will only be a problem once the client_mutex is removed.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 43e66fc1b90d..1fd8a4773191 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2263,8 +2263,11 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
/* case 1 (normal case) */
out_new:
- if (conf)
- unhash_client_locked(conf);
+ if (conf) {
+ status = mark_client_expired_locked(conf);
+ if (status)
+ goto out;
+ }
new->cl_minorversion = cstate->minorversion;
new->cl_mach_cred = (exid->spa_how == SP4_MACH_CRED);
@@ -2877,6 +2880,9 @@ nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
status = nfserr_clientid_busy;
goto out;
}
+ status = mark_client_expired_locked(conf);
+ if (status)
+ goto out;
clp = conf;
} else if (unconf)
clp = unconf;
--
1.9.3
Add some comments that describe what each of these objects is, and how
they related to one another.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/netns.h | 8 +++++
fs/nfsd/state.h | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 92 insertions(+), 7 deletions(-)
diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 3831ef6e5c75..46680da55cd7 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -34,6 +34,14 @@
struct cld_net;
struct nfsd4_client_tracking_ops;
+/*
+ * Represents a nfsd "container". With respect to nfsv4 state tracking, the
+ * fields of interest are the *_id_hashtbls and the *_name_tree. These track
+ * the nfs4_client objects by either short or long form clientid.
+ *
+ * Each nfsd_net runs a nfs4_laundromat workqueue job every lease period to
+ * clean up expired clients and delegations within the container.
+ */
struct nfsd_net {
struct cld_net *cld_net;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index a363f6bb621e..52ccd07d92ed 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -72,6 +72,11 @@ struct nfsd4_callback {
bool cb_done;
};
+/*
+ * A core object that represents a "common" stateid. These are generally
+ * embedded within the different (more specific) stateid objects and contain
+ * fields that are of general use to any stateid.
+ */
struct nfs4_stid {
atomic_t sc_count;
#define NFS4_OPEN_STID 1
@@ -89,6 +94,18 @@ struct nfs4_stid {
void (*sc_free)(struct nfs4_stid *);
};
+/*
+ * Represents a delegation stateid. The nfs4_client holds references to these
+ * and they are put when it is being destroyed or when the delegation is
+ * returned by the client.
+ *
+ * If the server attempts to recall a delegation and the client doesn't do so
+ * before a timeout, the server may also revoke the delegation. In that case,
+ * the object will either be destroyed (v4.0) or moved to a per-client list of
+ * revoked delegations (v4.1+).
+ *
+ * This object is a superset of the nfs4_stid.
+ */
struct nfs4_delegation {
struct nfs4_stid dl_stid; /* must be first field */
struct list_head dl_perfile;
@@ -195,6 +212,11 @@ struct nfsd4_conn {
unsigned char cn_flags;
};
+/*
+ * Representation of a v4.1+ session. These are refcounted in a similar fashion
+ * to the nfs4_client. References are only taken when the server is actively
+ * working on the object (primarily during the processing of compounds).
+ */
struct nfsd4_session {
atomic_t se_ref;
struct list_head se_hash; /* hash by sessionid */
@@ -224,13 +246,30 @@ struct nfsd4_sessionid {
/*
* struct nfs4_client - one per client. Clientids live here.
- * o Each nfs4_client is hashed by clientid.
*
- * o Each nfs4_clients is also hashed by name
- * (the opaque quantity initially sent by the client to identify itself).
+ * The initial object created by an NFS client using SETCLIENTID (for NFSv4.0)
+ * or EXCHANGE_ID (for NFSv4.1+). These objects are refcounted and timestamped.
+ * Each nfsd_net_ns object contains a set of these and they are tracked via
+ * short and long form clientid. They are hashed and searched for under the
+ * per-nfsd_net client_lock spinlock.
+ *
+ * References to it are only held during the processing of compounds, and in
+ * certain other operations. In their "resting state" they have a refcount of
+ * 0. If they are not renewed within a lease period, they become eligible for
+ * destruction by the laundromat.
+ *
+ * These objects can also be destroyed prematurely by the fault injection code,
+ * or if the client sends certain forms of SETCLIENTID or EXCHANGE_ID updates.
+ * Care is taken *not* to do this however when the objects have an elevated
+ * refcount.
+ *
+ * o Each nfs4_client is hashed by clientid
+ *
+ * o Each nfs4_clients is also hashed by name (the opaque quantity initially
+ * sent by the client to identify itself).
*
- * o cl_perclient list is used to ensure no dangling stateowner references
- * when we expire the nfs4_client
+ * o cl_perclient list is used to ensure no dangling stateowner references
+ * when we expire the nfs4_client
*/
struct nfs4_client {
struct list_head cl_idhash; /* hash by cl_clientid.id */
@@ -340,6 +379,12 @@ struct nfs4_stateowner_operations {
void (*so_free)(struct nfs4_stateowner *);
};
+/*
+ * A core object that represents either an open or lock owner. The object and
+ * lock owner objects have one of these embedded within them. Refcounts and
+ * other fields common to both owner types are contained within these
+ * structures.
+ */
struct nfs4_stateowner {
struct list_head so_strhash;
struct list_head so_stateids;
@@ -354,6 +399,12 @@ struct nfs4_stateowner {
bool so_is_open_owner;
};
+/*
+ * When a file is opened, the client provides an open state owner opaque string
+ * that indicates the "owner" of that open. These objects are refcounted.
+ * References to it are held by each open state associated with it. This object
+ * is a superset of the nfs4_stateowner struct.
+ */
struct nfs4_openowner {
struct nfs4_stateowner oo_owner; /* must be first field */
struct list_head oo_perclient;
@@ -371,6 +422,12 @@ struct nfs4_openowner {
unsigned char oo_flags;
};
+/*
+ * Represents a generic "lockowner". Similar to an openowner. References to it
+ * are held by the lock stateids that are created on its behalf. This object is
+ * a superset of the nfs4_stateowner struct (or would be if it needed any extra
+ * fields).
+ */
struct nfs4_lockowner {
struct nfs4_stateowner lo_owner; /* must be first element */
};
@@ -385,7 +442,14 @@ static inline struct nfs4_lockowner * lockowner(struct nfs4_stateowner *so)
return container_of(so, struct nfs4_lockowner, lo_owner);
}
-/* nfs4_file: a file opened by some number of (open) nfs4_stateowners. */
+/*
+ * nfs4_file: a file opened by some number of (open) nfs4_stateowners.
+ *
+ * These objects are global. nfsd only keeps one instance of a nfs4_file per
+ * inode (though it may keep multiple file descriptors open per inode). These
+ * are tracked in the file_hashtbl which is protected by the state_lock
+ * spinlock.
+ */
struct nfs4_file {
atomic_t fi_ref;
spinlock_t fi_lock;
@@ -410,7 +474,20 @@ struct nfs4_file {
bool fi_had_conflict;
};
-/* "ol" stands for "Open or Lock". Better suggestions welcome. */
+/*
+ * A generic struct representing either a open or lock stateid. The nfs4_client
+ * holds a reference to each of these objects, and they in turn hold a
+ * reference to their respective stateowners. The client's reference is
+ * released in response to a close or unlock (depending on whether it's an open
+ * or lock stateid) or when the client is being destroyed.
+ *
+ * In the case of v4.0 open stateids, these objects are preserved for a little
+ * while after close in order to handle CLOSE replays. Those are eventually
+ * reclaimed via a LRU scheme by the laundromat.
+ *
+ * This object is a superset of the nfs4_stid. "ol" stands for "Open or Lock".
+ * Better suggestions welcome.
+ */
struct nfs4_ol_stateid {
struct nfs4_stid st_stid; /* must be first field */
struct list_head st_perfile;
--
1.9.3
Currently, it's protected by the client_mutex. Move it so that the list
and the fields in the openowner are protected by the client_lock.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 38 +++++++++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 7 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index e7dfd4e9d942..818480035453 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1122,13 +1122,19 @@ static void unhash_openowner_locked(struct nfs4_openowner *oo)
static void release_last_closed_stateid(struct nfs4_openowner *oo)
{
- struct nfs4_ol_stateid *s = oo->oo_last_closed_stid;
+ struct nfsd_net *nn = net_generic(oo->oo_owner.so_client->net,
+ nfsd_net_id);
+ struct nfs4_ol_stateid *s;
+ spin_lock(&nn->client_lock);
+ s = oo->oo_last_closed_stid;
if (s) {
list_del_init(&oo->oo_close_lru);
oo->oo_last_closed_stid = NULL;
- nfs4_put_stid(&s->st_stid);
}
+ spin_unlock(&nn->client_lock);
+ if (s)
+ nfs4_put_stid(&s->st_stid);
}
static void release_openowner(struct nfs4_openowner *oo)
@@ -3265,6 +3271,7 @@ static void init_open_stateid(struct nfs4_ol_stateid *stp, struct nfs4_file *fp,
static void
move_to_close_lru(struct nfs4_ol_stateid *s, struct net *net)
{
+ struct nfs4_ol_stateid *last;
struct nfs4_openowner *oo = openowner(s->st_stateowner);
struct nfsd_net *nn = net_generic(s->st_stid.sc_client->net,
nfsd_net_id);
@@ -3287,10 +3294,15 @@ move_to_close_lru(struct nfs4_ol_stateid *s, struct net *net)
put_nfs4_file(s->st_stid.sc_file);
s->st_stid.sc_file = NULL;
}
- release_last_closed_stateid(oo);
+
+ spin_lock(&nn->client_lock);
+ last = oo->oo_last_closed_stid;
oo->oo_last_closed_stid = s;
list_move_tail(&oo->oo_close_lru, &nn->close_lru);
oo->oo_time = get_seconds();
+ spin_unlock(&nn->client_lock);
+ if (last)
+ nfs4_put_stid(&last->st_stid);
}
/* search file_hashtbl[] for file */
@@ -4148,6 +4160,7 @@ nfs4_laundromat(struct nfsd_net *nn)
struct nfs4_client *clp;
struct nfs4_openowner *oo;
struct nfs4_delegation *dp;
+ struct nfs4_ol_stateid *stp;
struct list_head *pos, *next, reaplist;
time_t cutoff = get_seconds() - nn->nfsd4_lease;
time_t t, new_timeo = nn->nfsd4_lease;
@@ -4201,15 +4214,26 @@ nfs4_laundromat(struct nfsd_net *nn)
list_del_init(&dp->dl_recall_lru);
revoke_delegation(dp);
}
- list_for_each_safe(pos, next, &nn->close_lru) {
- oo = container_of(pos, struct nfs4_openowner, oo_close_lru);
- if (time_after((unsigned long)oo->oo_time, (unsigned long)cutoff)) {
+
+ spin_lock(&nn->client_lock);
+ while (!list_empty(&nn->close_lru)) {
+ oo = list_first_entry(&nn->close_lru, struct nfs4_openowner,
+ oo_close_lru);
+ if (time_after((unsigned long)oo->oo_time,
+ (unsigned long)cutoff)) {
t = oo->oo_time - cutoff;
new_timeo = min(new_timeo, t);
break;
}
- release_last_closed_stateid(oo);
+ list_del_init(&oo->oo_close_lru);
+ stp = oo->oo_last_closed_stid;
+ oo->oo_last_closed_stid = NULL;
+ spin_unlock(&nn->client_lock);
+ nfs4_put_stid(&stp->st_stid);
+ spin_lock(&nn->client_lock);
}
+ spin_unlock(&nn->client_lock);
+
new_timeo = max_t(time_t, new_timeo, NFSD_LAUNDROMAT_MINTIMEOUT);
nfs4_unlock_state();
return new_timeo;
--
1.9.3
From: Trond Myklebust <[email protected]>
...instead of relying on the client_mutex.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 4b42cb95e315..f149e30475db 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1923,7 +1923,7 @@ add_to_unconfirmed(struct nfs4_client *clp)
add_clp_to_name_tree(clp, &nn->unconf_name_tree);
idhashval = clientid_hashval(clp->cl_clientid.cl_id);
list_add(&clp->cl_idhash, &nn->unconf_id_hashtbl[idhashval]);
- renew_client(clp);
+ renew_client_locked(clp);
}
static void
@@ -1937,7 +1937,7 @@ move_to_confirmed(struct nfs4_client *clp)
rb_erase(&clp->cl_namenode, &nn->unconf_name_tree);
add_clp_to_name_tree(clp, &nn->conf_name_tree);
set_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags);
- renew_client(clp);
+ renew_client_locked(clp);
}
static struct nfs4_client *
@@ -1950,7 +1950,7 @@ find_client_in_id_table(struct list_head *tbl, clientid_t *clid, bool sessions)
if (same_clid(&clp->cl_clientid, clid)) {
if ((bool)clp->cl_minorversion != sessions)
return NULL;
- renew_client(clp);
+ renew_client_locked(clp);
return clp;
}
}
@@ -2152,7 +2152,8 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
struct nfsd4_compound_state *cstate,
struct nfsd4_exchange_id *exid)
{
- struct nfs4_client *unconf, *conf, *new;
+ struct nfs4_client *conf, *new;
+ struct nfs4_client *unconf = NULL;
__be32 status;
char addr_str[INET6_ADDRSTRLEN];
nfs4_verifier verf = exid->verifier;
@@ -2187,6 +2188,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
/* Cases below refer to rfc 5661 section 18.35.4: */
nfs4_lock_state();
+ spin_lock(&nn->client_lock);
conf = find_confirmed_client_by_name(&exid->clname, nn);
if (conf) {
bool creds_match = same_creds(&conf->cl_cred, &rqstp->rq_cred);
@@ -2218,7 +2220,6 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
status = nfserr_clid_inuse;
goto out;
}
- expire_client(conf);
goto out_new;
}
if (verfs_match) { /* case 2 */
@@ -2226,6 +2227,7 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
goto out_copy;
}
/* case 5, client reboot */
+ conf = NULL;
goto out_new;
}
@@ -2236,17 +2238,18 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
unconf = find_unconfirmed_client_by_name(&exid->clname, nn);
if (unconf) /* case 4, possible retry or client restart */
- expire_client(unconf);
+ unhash_client_locked(unconf);
/* case 1 (normal case) */
out_new:
+ if (conf)
+ unhash_client_locked(conf);
new->cl_minorversion = cstate->minorversion;
new->cl_mach_cred = (exid->spa_how == SP4_MACH_CRED);
gen_clid(new, nn);
add_to_unconfirmed(new);
- conf = new;
- new = NULL;
+ swap(new, conf);
out_copy:
exid->clientid.cl_boot = conf->cl_clientid.cl_boot;
exid->clientid.cl_id = conf->cl_clientid.cl_id;
@@ -2259,9 +2262,12 @@ out_copy:
status = nfs_ok;
out:
+ spin_unlock(&nn->client_lock);
nfs4_unlock_state();
if (new)
- free_client(new);
+ expire_client(new);
+ if (unconf)
+ expire_client(unconf);
return status;
}
@@ -2900,7 +2906,8 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
{
struct xdr_netobj clname = setclid->se_name;
nfs4_verifier clverifier = setclid->se_verf;
- struct nfs4_client *conf, *unconf, *new;
+ struct nfs4_client *conf, *new;
+ struct nfs4_client *unconf = NULL;
__be32 status;
struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
@@ -2909,6 +2916,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return nfserr_jukebox;
/* Cases below refer to rfc 3530 section 14.2.33: */
nfs4_lock_state();
+ spin_lock(&nn->client_lock);
conf = find_confirmed_client_by_name(&clname, nn);
if (conf) {
/* case 0: */
@@ -2926,7 +2934,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
}
unconf = find_unconfirmed_client_by_name(&clname, nn);
if (unconf)
- expire_client(unconf);
+ unhash_client_locked(unconf);
if (conf && same_verf(&conf->cl_verifier, &clverifier))
/* case 1: probable callback update */
copy_clid(new, conf);
@@ -2941,9 +2949,12 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
new = NULL;
status = nfs_ok;
out:
+ spin_unlock(&nn->client_lock);
nfs4_unlock_state();
if (new)
free_client(new);
+ if (unconf)
+ expire_client(unconf);
return status;
}
--
1.9.3
From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index cd9da9891039..4f04a4a776cd 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5573,11 +5573,9 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n",
clid->cl_boot, clid->cl_id);
- nfs4_lock_state();
-
status = lookup_clientid(clid, cstate, nn);
if (status)
- goto out;
+ return status;
clp = cstate->clp;
/* Find the matching lock stateowner */
@@ -5594,7 +5592,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
if (check_for_locks(stp->st_stid.sc_file, lo)) {
status = nfserr_locks_held;
spin_unlock(&clp->cl_lock);
- goto out;
+ return status;
}
}
@@ -5604,8 +5602,6 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp,
spin_unlock(&clp->cl_lock);
if (lo)
release_lockowner(lo);
-out:
- nfs4_unlock_state();
return status;
}
--
1.9.3
From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index de3bb633d96b..eba119a46add 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4709,7 +4709,6 @@ put_stateid:
nfs4_put_stid(&stp->st_stid);
out:
nfsd4_bump_seqid(cstate, status);
- nfs4_unlock_state();
return status;
}
@@ -4756,7 +4755,6 @@ nfsd4_open_downgrade(struct svc_rqst *rqstp,
dprintk("NFSD: %s: od_deleg_want=0x%x ignored\n", __func__,
od->od_deleg_want);
- nfs4_lock_state();
status = nfs4_preprocess_confirmed_seqid_op(cstate, od->od_seqid,
&od->od_stateid, &stp, nn);
if (status)
@@ -4822,7 +4820,6 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
dprintk("NFSD: nfsd4_close on file %pd\n",
cstate->current_fh.fh_dentry);
- nfs4_lock_state();
status = nfs4_preprocess_seqid_op(cstate, close->cl_seqid,
&close->cl_stateid,
NFS4_OPEN_STID|NFS4_CLOSED_STID,
@@ -4838,7 +4835,6 @@ nfsd4_close(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
/* put reference from nfs4_preprocess_seqid_op */
nfs4_put_stid(&stp->st_stid);
out:
- nfs4_unlock_state();
return status;
}
--
1.9.3
From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 3b7fceed0a0a..a45dcd15b8f7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2944,7 +2944,6 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (new == NULL)
return nfserr_jukebox;
/* Cases below refer to rfc 3530 section 14.2.33: */
- nfs4_lock_state();
spin_lock(&nn->client_lock);
conf = find_confirmed_client_by_name(&clname, nn);
if (conf) {
@@ -2979,7 +2978,6 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
status = nfs_ok;
out:
spin_unlock(&nn->client_lock);
- nfs4_unlock_state();
if (new)
free_client(new);
if (unconf)
@@ -3002,7 +3000,6 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
if (STALE_CLIENTID(clid, nn))
return nfserr_stale_clientid;
- nfs4_lock_state();
spin_lock(&nn->client_lock);
conf = find_confirmed_client(clid, false, nn);
@@ -3052,7 +3049,6 @@ out:
spin_unlock(&nn->client_lock);
if (old)
expire_client(old);
- nfs4_unlock_state();
return status;
}
@@ -4109,7 +4105,6 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
__be32 status;
struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
- nfs4_lock_state();
dprintk("process_renew(%08x/%08x): starting\n",
clid->cl_boot, clid->cl_id);
status = lookup_clientid(clid, cstate, nn);
@@ -4122,7 +4117,6 @@ nfsd4_renew(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
goto out;
status = nfs_ok;
out:
- nfs4_unlock_state();
return status;
}
--
1.9.3
From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a45dcd15b8f7..d98d33d0e304 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2905,7 +2905,6 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
return nfs_ok;
}
- nfs4_lock_state();
status = nfserr_complete_already;
if (test_and_set_bit(NFSD4_CLIENT_RECLAIM_COMPLETE,
&cstate->session->se_client->cl_flags))
@@ -2925,7 +2924,6 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
status = nfs_ok;
nfsd4_client_record_create(cstate->session->se_client);
out:
- nfs4_unlock_state();
return status;
}
--
1.9.3
From: Trond Myklebust <[email protected]>
Also destroy_clientid and bind_conn_to_session.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 02caef874d67..3b7fceed0a0a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2205,7 +2205,6 @@ nfsd4_exchange_id(struct svc_rqst *rqstp,
return nfserr_jukebox;
/* Cases below refer to rfc 5661 section 18.35.4: */
- nfs4_lock_state();
spin_lock(&nn->client_lock);
conf = find_confirmed_client_by_name(&exid->clname, nn);
if (conf) {
@@ -2284,7 +2283,6 @@ out_copy:
out:
spin_unlock(&nn->client_lock);
- nfs4_unlock_state();
if (new)
expire_client(new);
if (unconf)
@@ -2458,7 +2456,6 @@ nfsd4_create_session(struct svc_rqst *rqstp,
if (!conn)
goto out_free_session;
- nfs4_lock_state();
spin_lock(&nn->client_lock);
unconf = find_unconfirmed_client(&cr_ses->clientid, true, nn);
conf = find_confirmed_client(&cr_ses->clientid, true, nn);
@@ -2528,13 +2525,11 @@ nfsd4_create_session(struct svc_rqst *rqstp,
/* init connection and backchannel */
nfsd4_init_conn(rqstp, conn, new);
nfsd4_put_session(new);
- nfs4_unlock_state();
if (old)
expire_client(old);
return status;
out_free_conn:
spin_unlock(&nn->client_lock);
- nfs4_unlock_state();
free_conn(conn);
if (old)
expire_client(old);
@@ -2590,7 +2585,6 @@ __be32 nfsd4_bind_conn_to_session(struct svc_rqst *rqstp,
if (!nfsd4_last_compound_op(rqstp))
return nfserr_not_only_op;
- nfs4_lock_state();
spin_lock(&nn->client_lock);
session = find_in_sessionid_hashtbl(&bcts->sessionid, net, &status);
spin_unlock(&nn->client_lock);
@@ -2611,7 +2605,6 @@ __be32 nfsd4_bind_conn_to_session(struct svc_rqst *rqstp,
out:
nfsd4_put_session(session);
out_no_session:
- nfs4_unlock_state();
return status;
}
@@ -2633,7 +2626,6 @@ nfsd4_destroy_session(struct svc_rqst *r,
struct net *net = SVC_NET(r);
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
- nfs4_lock_state();
status = nfserr_not_only_op;
if (nfsd4_compound_in_session(cstate->session, &sessionid->sessionid)) {
if (!nfsd4_last_compound_op(r))
@@ -2663,7 +2655,6 @@ out_put_session:
out_client_lock:
spin_unlock(&nn->client_lock);
out:
- nfs4_unlock_state();
return status;
}
@@ -2866,7 +2857,6 @@ nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
__be32 status = 0;
struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
- nfs4_lock_state();
spin_lock(&nn->client_lock);
unconf = find_unconfirmed_client(&dc->clientid, true, nn);
conf = find_confirmed_client(&dc->clientid, true, nn);
@@ -2895,7 +2885,6 @@ nfsd4_destroy_clientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *csta
unhash_client_locked(clp);
out:
spin_unlock(&nn->client_lock);
- nfs4_unlock_state();
if (clp)
expire_client(clp);
return status;
--
1.9.3
From: Trond Myklebust <[email protected]>
Ensure that the client lookup is done safely under the client_lock, so
we're not relying on the client_mutex.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 68383b09c7dc..f9d077d800ee 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3451,13 +3451,17 @@ static __be32 lookup_clientid(clientid_t *clid,
* will be false.
*/
WARN_ON_ONCE(cstate->session);
+ spin_lock(&nn->client_lock);
found = find_confirmed_client(clid, false, nn);
- if (!found)
+ if (!found) {
+ spin_unlock(&nn->client_lock);
return nfserr_expired;
+ }
+ atomic_inc(&found->cl_refcount);
+ spin_unlock(&nn->client_lock);
/* Cache the nfs4_client in cstate! */
cstate->clp = found;
- atomic_inc(&found->cl_refcount);
return nfs_ok;
}
--
1.9.3
If it fails, it means that the client is in use and so destroying it
would be bad. Currently, the client_mutex prevents this from happening
but once we remove it, we won't be able to do this.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 56999cbe84a7..43e66fc1b90d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2496,8 +2496,10 @@ nfsd4_create_session(struct svc_rqst *rqstp,
old = find_confirmed_client_by_name(&unconf->cl_name, nn);
if (old) {
status = mark_client_expired_locked(old);
- if (status)
+ if (status) {
+ old = NULL;
goto out_free_conn;
+ }
}
move_to_confirmed(unconf);
conf = unconf;
@@ -3041,8 +3043,10 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
old = find_confirmed_client_by_name(&unconf->cl_name, nn);
if (old) {
status = mark_client_expired_locked(old);
- if (status)
+ if (status) {
+ old = NULL;
goto out;
+ }
}
move_to_confirmed(unconf);
conf = unconf;
--
1.9.3
Now that we've added more granular locking in other places, it's time
to address the fault injection code. This code is currently quite
reliant on the client_mutex for protection. Start to change this by
adding a new set of fault injection op vectors.
For now they all use the legacy ones. In later patches we'll add new
routines that can deal with more granular locking.
Also, move some of the printk routines into the callers to make the
results of the operations more uniform.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/fault_inject.c | 129 ++++++++++++++++++++++++++++++-------------------
1 file changed, 78 insertions(+), 51 deletions(-)
diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
index f1333fc35b33..b1159900d934 100644
--- a/fs/nfsd/fault_inject.c
+++ b/fs/nfsd/fault_inject.c
@@ -17,79 +17,50 @@
struct nfsd_fault_inject_op {
char *file;
+ u64 (*get)(struct nfsd_fault_inject_op *);
+ u64 (*set_val)(struct nfsd_fault_inject_op *, u64);
+ u64 (*set_clnt)(struct nfsd_fault_inject_op *,
+ struct sockaddr_storage *, size_t);
u64 (*forget)(struct nfs4_client *, u64);
u64 (*print)(struct nfs4_client *, u64);
};
-static struct nfsd_fault_inject_op inject_ops[] = {
- {
- .file = "forget_clients",
- .forget = nfsd_forget_client,
- .print = nfsd_print_client,
- },
- {
- .file = "forget_locks",
- .forget = nfsd_forget_client_locks,
- .print = nfsd_print_client_locks,
- },
- {
- .file = "forget_openowners",
- .forget = nfsd_forget_client_openowners,
- .print = nfsd_print_client_openowners,
- },
- {
- .file = "forget_delegations",
- .forget = nfsd_forget_client_delegations,
- .print = nfsd_print_client_delegations,
- },
- {
- .file = "recall_delegations",
- .forget = nfsd_recall_client_delegations,
- .print = nfsd_print_client_delegations,
- },
-};
-
-static long int NUM_INJECT_OPS = sizeof(inject_ops) / sizeof(struct nfsd_fault_inject_op);
static struct dentry *debug_dir;
-static void nfsd_inject_set(struct nfsd_fault_inject_op *op, u64 val)
+static u64 nfsd_inject_set(struct nfsd_fault_inject_op *op, u64 val)
{
- u64 count = 0;
-
- if (val == 0)
- printk(KERN_INFO "NFSD Fault Injection: %s (all)", op->file);
- else
- printk(KERN_INFO "NFSD Fault Injection: %s (n = %llu)", op->file, val);
+ u64 count;
nfs4_lock_state();
count = nfsd_for_n_state(val, op->forget);
nfs4_unlock_state();
- printk(KERN_INFO "NFSD: %s: found %llu", op->file, count);
+ return count;
}
-static void nfsd_inject_set_client(struct nfsd_fault_inject_op *op,
+static u64 nfsd_inject_set_client(struct nfsd_fault_inject_op *op,
struct sockaddr_storage *addr,
size_t addr_size)
{
- char buf[INET6_ADDRSTRLEN];
struct nfs4_client *clp;
- u64 count;
+ u64 count = 0;
nfs4_lock_state();
clp = nfsd_find_client(addr, addr_size);
- if (clp) {
+ if (clp)
count = op->forget(clp, 0);
- rpc_ntop((struct sockaddr *)&clp->cl_addr, buf, sizeof(buf));
- printk(KERN_INFO "NFSD [%s]: Client %s had %llu state object(s)\n", op->file, buf, count);
- }
nfs4_unlock_state();
+ return count;
}
-static void nfsd_inject_get(struct nfsd_fault_inject_op *op, u64 *val)
+static u64 nfsd_inject_get(struct nfsd_fault_inject_op *op)
{
+ u64 count;
+
nfs4_lock_state();
- *val = nfsd_for_n_state(0, op->print);
+ count = nfsd_for_n_state(0, op->print);
nfs4_unlock_state();
+
+ return count;
}
static ssize_t fault_inject_read(struct file *file, char __user *buf,
@@ -99,9 +70,10 @@ static ssize_t fault_inject_read(struct file *file, char __user *buf,
char read_buf[25];
size_t size;
loff_t pos = *ppos;
+ struct nfsd_fault_inject_op *op = file_inode(file)->i_private;
if (!pos)
- nfsd_inject_get(file_inode(file)->i_private, &val);
+ val = op->get(op);
size = scnprintf(read_buf, sizeof(read_buf), "%llu\n", val);
return simple_read_from_buffer(buf, len, ppos, read_buf, size);
@@ -114,6 +86,7 @@ static ssize_t fault_inject_write(struct file *file, const char __user *buf,
size_t size = min(sizeof(write_buf) - 1, len);
struct net *net = current->nsproxy->net_ns;
struct sockaddr_storage sa;
+ struct nfsd_fault_inject_op *op = file_inode(file)->i_private;
u64 val;
char *nl;
@@ -129,11 +102,20 @@ static ssize_t fault_inject_write(struct file *file, const char __user *buf,
}
size = rpc_pton(net, write_buf, size, (struct sockaddr *)&sa, sizeof(sa));
- if (size > 0)
- nfsd_inject_set_client(file_inode(file)->i_private, &sa, size);
- else {
+ if (size > 0) {
+ val = op->set_clnt(op, &sa, size);
+ if (val)
+ pr_info("NFSD [%s]: Client %s had %llu state object(s)\n",
+ op->file, write_buf, val);
+ } else {
val = simple_strtoll(write_buf, NULL, 0);
- nfsd_inject_set(file_inode(file)->i_private, val);
+ if (val == 0)
+ pr_info("NFSD Fault Injection: %s (all)", op->file);
+ else
+ pr_info("NFSD Fault Injection: %s (n = %llu)",
+ op->file, val);
+ val = op->set_val(op, val);
+ pr_info("NFSD: %s: found %llu", op->file, val);
}
return len; /* on success, claim we got the whole input */
}
@@ -149,6 +131,51 @@ void nfsd_fault_inject_cleanup(void)
debugfs_remove_recursive(debug_dir);
}
+static struct nfsd_fault_inject_op inject_ops[] = {
+ {
+ .file = "forget_clients",
+ .get = nfsd_inject_get,
+ .set_val = nfsd_inject_set,
+ .set_clnt = nfsd_inject_set_client,
+ .forget = nfsd_forget_client,
+ .print = nfsd_print_client,
+ },
+ {
+ .file = "forget_locks",
+ .get = nfsd_inject_get,
+ .set_val = nfsd_inject_set,
+ .set_clnt = nfsd_inject_set_client,
+ .forget = nfsd_forget_client_locks,
+ .print = nfsd_print_client_locks,
+ },
+ {
+ .file = "forget_openowners",
+ .get = nfsd_inject_get,
+ .set_val = nfsd_inject_set,
+ .set_clnt = nfsd_inject_set_client,
+ .forget = nfsd_forget_client_openowners,
+ .print = nfsd_print_client_openowners,
+ },
+ {
+ .file = "forget_delegations",
+ .get = nfsd_inject_get,
+ .set_val = nfsd_inject_set,
+ .set_clnt = nfsd_inject_set_client,
+ .forget = nfsd_forget_client_delegations,
+ .print = nfsd_print_client_delegations,
+ },
+ {
+ .file = "recall_delegations",
+ .get = nfsd_inject_get,
+ .set_val = nfsd_inject_set,
+ .set_clnt = nfsd_inject_set_client,
+ .forget = nfsd_recall_client_delegations,
+ .print = nfsd_print_client_delegations,
+ },
+};
+
+#define NUM_INJECT_OPS (sizeof(inject_ops)/sizeof(struct nfsd_fault_inject_op))
+
int nfsd_fault_inject_init(void)
{
unsigned int i;
--
1.9.3
From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 4f04a4a776cd..de3bb633d96b 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5201,8 +5201,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return status;
}
- nfs4_lock_state();
-
if (lock->lk_is_new) {
if (nfsd4_has_session(cstate))
/* See rfc 5661 18.10.3: given clientid is ignored: */
@@ -5345,7 +5343,6 @@ out:
if (open_stp)
nfs4_put_stid(&open_stp->st_stid);
nfsd4_bump_seqid(cstate, status);
- nfs4_unlock_state();
if (file_lock)
locks_free_lock(file_lock);
if (conflock)
@@ -5388,8 +5385,6 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (check_lock_length(lockt->lt_offset, lockt->lt_length))
return nfserr_inval;
- nfs4_lock_state();
-
if (!nfsd4_has_session(cstate)) {
status = lookup_clientid(&lockt->lt_clientid, cstate, nn);
if (status)
@@ -5444,7 +5439,6 @@ nfsd4_lockt(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
out:
if (lo)
nfs4_put_stateowner(&lo->lo_owner);
- nfs4_unlock_state();
if (file_lock)
locks_free_lock(file_lock);
return status;
@@ -5468,8 +5462,6 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (check_lock_length(locku->lu_offset, locku->lu_length))
return nfserr_inval;
- nfs4_lock_state();
-
status = nfs4_preprocess_seqid_op(cstate, locku->lu_seqid,
&locku->lu_stateid, NFS4_LOCK_STID,
&stp, nn);
@@ -5512,7 +5504,6 @@ put_stateid:
nfs4_put_stid(&stp->st_stid);
out:
nfsd4_bump_seqid(cstate, status);
- nfs4_unlock_state();
if (file_lock)
locks_free_lock(file_lock);
return status;
--
1.9.3
Add a new "get" routine for forget_clients that relies on the
client_lock instead of the client_mutex.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/fault_inject.c | 3 +--
fs/nfsd/nfs4state.c | 30 ++++++++++++++++++++++--------
fs/nfsd/state.h | 4 +++-
3 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
index b1159900d934..a0387fd47e14 100644
--- a/fs/nfsd/fault_inject.c
+++ b/fs/nfsd/fault_inject.c
@@ -134,11 +134,10 @@ void nfsd_fault_inject_cleanup(void)
static struct nfsd_fault_inject_op inject_ops[] = {
{
.file = "forget_clients",
- .get = nfsd_inject_get,
+ .get = nfsd_inject_print_clients,
.set_val = nfsd_inject_set,
.set_clnt = nfsd_inject_set_client,
.forget = nfsd_forget_client,
- .print = nfsd_print_client,
},
{
.file = "forget_locks",
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 64caf119ab53..60f6e2f2463e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5719,6 +5719,28 @@ nfs4_check_open_reclaim(clientid_t *clid,
}
#ifdef CONFIG_NFSD_FAULT_INJECTION
+u64
+nfsd_inject_print_clients(struct nfsd_fault_inject_op *op)
+{
+ struct nfs4_client *clp;
+ u64 count = 0;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+ char buf[INET6_ADDRSTRLEN];
+
+ if (!nfsd_netns_ready(nn))
+ return 0;
+
+ spin_lock(&nn->client_lock);
+ list_for_each_entry(clp, &nn->client_lru, cl_lru) {
+ rpc_ntop((struct sockaddr *)&clp->cl_addr, buf, sizeof(buf));
+ pr_info("NFS Client: %s\n", buf);
+ ++count;
+ }
+ spin_unlock(&nn->client_lock);
+
+ return count;
+}
u64 nfsd_forget_client(struct nfs4_client *clp, u64 max)
{
@@ -5734,14 +5756,6 @@ u64 nfsd_forget_client(struct nfs4_client *clp, u64 max)
return 1;
}
-u64 nfsd_print_client(struct nfs4_client *clp, u64 num)
-{
- char buf[INET6_ADDRSTRLEN];
- rpc_ntop((struct sockaddr *)&clp->cl_addr, buf, sizeof(buf));
- printk(KERN_INFO "NFS Client: %s\n", buf);
- return 1;
-}
-
static void nfsd_print_count(struct nfs4_client *clp, unsigned int count,
const char *type)
{
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 73a209dc352b..02707375cb4b 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -471,18 +471,20 @@ extern void nfsd4_record_grace_done(struct nfsd_net *nn, time_t boot_time);
/* nfs fault injection functions */
#ifdef CONFIG_NFSD_FAULT_INJECTION
+struct nfsd_fault_inject_op;
+
int nfsd_fault_inject_init(void);
void nfsd_fault_inject_cleanup(void);
u64 nfsd_for_n_state(u64, u64 (*)(struct nfs4_client *, u64));
struct nfs4_client *nfsd_find_client(struct sockaddr_storage *, size_t);
+u64 nfsd_inject_print_clients(struct nfsd_fault_inject_op *op);
u64 nfsd_forget_client(struct nfs4_client *, u64);
u64 nfsd_forget_client_locks(struct nfs4_client*, u64);
u64 nfsd_forget_client_openowners(struct nfs4_client *, u64);
u64 nfsd_forget_client_delegations(struct nfs4_client *, u64);
u64 nfsd_recall_client_delegations(struct nfs4_client *, u64);
-u64 nfsd_print_client(struct nfs4_client *, u64);
u64 nfsd_print_client_locks(struct nfs4_client *, u64);
u64 nfsd_print_client_openowners(struct nfs4_client *, u64);
u64 nfsd_print_client_delegations(struct nfs4_client *, u64);
--
1.9.3
The clid counter is a global counter currently. Move it to be a per-net
property so that it can be properly protected by the nn->client_lock
instead of relying on the client_mutex.
The verifier generator is also potentially racy if there are two
simultaneous callers. Generate the verifier when we generate the clid
value, so it's also created under the client_lock. With this, there's
no need to keep two counters as they'd always be in sync anyway, so
just use the clientid_counter for both.
As Trond points out, what would be best is to eventually move this
code to use IDR instead of the hash tables. That would also help ensure
uniqueness, but that's probably best done as a separate project.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/netns.h | 6 +++---
fs/nfsd/nfs4state.c | 21 +++++++++------------
2 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index e1f479c162b5..3831ef6e5c75 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -92,9 +92,7 @@ struct nfsd_net {
bool nfsd_net_up;
bool lockd_up;
- /*
- * Time of server startup
- */
+ /* Time of server startup */
struct timeval nfssvc_boot;
/*
@@ -103,6 +101,8 @@ struct nfsd_net {
*/
unsigned int max_connections;
+ u32 clientid_counter;
+
struct svc_serv *nfsd_serv;
};
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1fd8a4773191..64caf119ab53 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1808,28 +1808,26 @@ static bool mach_creds_match(struct nfs4_client *cl, struct svc_rqst *rqstp)
return 0 == strcmp(cl->cl_cred.cr_principal, cr->cr_principal);
}
-static void gen_clid(struct nfs4_client *clp, struct nfsd_net *nn)
-{
- static u32 current_clientid = 1;
-
- clp->cl_clientid.cl_boot = nn->boot_time;
- clp->cl_clientid.cl_id = current_clientid++;
-}
-
-static void gen_confirm(struct nfs4_client *clp)
+static void gen_confirm(struct nfs4_client *clp, struct nfsd_net *nn)
{
__be32 verf[2];
- static u32 i;
/*
* This is opaque to client, so no need to byte-swap. Use
* __force to keep sparse happy
*/
verf[0] = (__force __be32)get_seconds();
- verf[1] = (__force __be32)i++;
+ verf[1] = (__force __be32)nn->clientid_counter;
memcpy(clp->cl_confirm.data, verf, sizeof(clp->cl_confirm.data));
}
+static void gen_clid(struct nfs4_client *clp, struct nfsd_net *nn)
+{
+ clp->cl_clientid.cl_boot = nn->boot_time;
+ clp->cl_clientid.cl_id = nn->clientid_counter++;
+ gen_confirm(clp, nn);
+}
+
static struct nfs4_stid *
find_stateid_locked(struct nfs4_client *cl, stateid_t *t)
{
@@ -1880,7 +1878,6 @@ static struct nfs4_client *create_client(struct xdr_netobj name,
clear_bit(0, &clp->cl_cb_slot_busy);
copy_verf(clp, verf);
rpc_copy_addr((struct sockaddr *) &clp->cl_addr, sa);
- gen_confirm(clp);
clp->cl_cb_session = NULL;
clp->net = net;
return clp;
--
1.9.3
...that relies on the client_lock instead of client_mutex.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/fault_inject.c | 2 +-
fs/nfsd/nfs4state.c | 28 ++++++++++++++++++++++++++++
fs/nfsd/state.h | 3 +++
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
index a0387fd47e14..5f3ead0c72fb 100644
--- a/fs/nfsd/fault_inject.c
+++ b/fs/nfsd/fault_inject.c
@@ -136,7 +136,7 @@ static struct nfsd_fault_inject_op inject_ops[] = {
.file = "forget_clients",
.get = nfsd_inject_print_clients,
.set_val = nfsd_inject_set,
- .set_clnt = nfsd_inject_set_client,
+ .set_clnt = nfsd_inject_forget_client,
.forget = nfsd_forget_client,
},
{
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 60f6e2f2463e..2dd9819e205b 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5756,6 +5756,34 @@ u64 nfsd_forget_client(struct nfs4_client *clp, u64 max)
return 1;
}
+u64
+nfsd_inject_forget_client(struct nfsd_fault_inject_op *op,
+ struct sockaddr_storage *addr, size_t addr_size)
+{
+ u64 count = 0;
+ struct nfs4_client *clp;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+
+ if (!nfsd_netns_ready(nn))
+ return count;
+
+ spin_lock(&nn->client_lock);
+ clp = nfsd_find_client(addr, addr_size);
+ if (clp) {
+ if (mark_client_expired_locked(clp) == nfs_ok)
+ ++count;
+ else
+ clp = NULL;
+ }
+ spin_unlock(&nn->client_lock);
+
+ if (clp)
+ expire_client(clp);
+
+ return count;
+}
+
static void nfsd_print_count(struct nfs4_client *clp, unsigned int count,
const char *type)
{
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 02707375cb4b..e37520e1f767 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -480,6 +480,9 @@ struct nfs4_client *nfsd_find_client(struct sockaddr_storage *, size_t);
u64 nfsd_inject_print_clients(struct nfsd_fault_inject_op *op);
u64 nfsd_forget_client(struct nfs4_client *, u64);
+u64 nfsd_inject_forget_client(struct nfsd_fault_inject_op *,
+ struct sockaddr_storage *, size_t);
+
u64 nfsd_forget_client_locks(struct nfs4_client*, u64);
u64 nfsd_forget_client_openowners(struct nfs4_client *, u64);
u64 nfsd_forget_client_delegations(struct nfs4_client *, u64);
--
1.9.3
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d98d33d0e304..7eea82e1d779 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4148,8 +4148,6 @@ nfs4_laundromat(struct nfsd_net *nn)
time_t cutoff = get_seconds() - nn->nfsd4_lease;
time_t t, new_timeo = nn->nfsd4_lease;
- nfs4_lock_state();
-
dprintk("NFSD: laundromat service - starting\n");
nfsd4_end_grace(nn);
INIT_LIST_HEAD(&reaplist);
@@ -4217,7 +4215,6 @@ nfs4_laundromat(struct nfsd_net *nn)
spin_unlock(&nn->client_lock);
new_timeo = max_t(time_t, new_timeo, NFSD_LAUNDROMAT_MINTIMEOUT);
- nfs4_unlock_state();
return new_timeo;
}
--
1.9.3
From: Trond Myklebust <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9013a30874e3..cd9da9891039 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4528,11 +4528,9 @@ nfsd4_test_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd4_test_stateid_id *stateid;
struct nfs4_client *cl = cstate->session->se_client;
- nfs4_lock_state();
list_for_each_entry(stateid, &test_stateid->ts_stateid_list, ts_id_list)
stateid->ts_id_status =
nfsd4_validate_stateid(cl, &stateid->ts_id_stateid);
- nfs4_unlock_state();
return nfs_ok;
}
@@ -4548,7 +4546,6 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfs4_client *cl = cstate->session->se_client;
__be32 ret = nfserr_bad_stateid;
- nfs4_lock_state();
spin_lock(&cl->cl_lock);
s = find_stateid_locked(cl, stateid);
if (!s)
@@ -4589,7 +4586,6 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
out_unlock:
spin_unlock(&cl->cl_lock);
out:
- nfs4_unlock_state();
return ret;
}
--
1.9.3
...instead of relying on the client_mutex.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/fault_inject.c | 8 ++-
fs/nfsd/nfs4state.c | 132 +++++++++++++++++++++++++++++++++++++++++++++----
fs/nfsd/state.h | 7 ++-
3 files changed, 131 insertions(+), 16 deletions(-)
diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c
index 76ecdff37ea2..a444d821d2a5 100644
--- a/fs/nfsd/fault_inject.c
+++ b/fs/nfsd/fault_inject.c
@@ -140,11 +140,9 @@ static struct nfsd_fault_inject_op inject_ops[] = {
},
{
.file = "forget_locks",
- .get = nfsd_inject_get,
- .set_val = nfsd_inject_set,
- .set_clnt = nfsd_inject_set_client,
- .forget = nfsd_forget_client_locks,
- .print = nfsd_print_client_locks,
+ .get = nfsd_inject_print_locks,
+ .set_val = nfsd_inject_forget_locks,
+ .set_clnt = nfsd_inject_forget_client_locks,
},
{
.file = "forget_openowners",
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b6020e4a2102..594aa01806d1 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5719,6 +5719,12 @@ nfs4_check_open_reclaim(clientid_t *clid,
}
#ifdef CONFIG_NFSD_FAULT_INJECTION
+static inline void
+put_client(struct nfs4_client *clp)
+{
+ atomic_dec(&clp->cl_refcount);
+}
+
u64
nfsd_inject_print_clients(struct nfsd_fault_inject_op *op)
{
@@ -5806,6 +5812,22 @@ static void nfsd_print_count(struct nfs4_client *clp, unsigned int count,
printk(KERN_INFO "NFS Client: %s has %u %s\n", buf, count, type);
}
+static void
+nfsd_inject_add_lock_to_list(struct nfs4_ol_stateid *lst,
+ struct list_head *collect)
+{
+ struct nfs4_client *clp = lst->st_stid.sc_client;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+
+ if (!collect)
+ return;
+
+ lockdep_assert_held(&nn->client_lock);
+ atomic_inc(&clp->cl_refcount);
+ list_add(&lst->st_locks, collect);
+}
+
static u64 nfsd_foreach_client_lock(struct nfs4_client *clp, u64 max,
struct list_head *collect,
void (*func)(struct nfs4_ol_stateid *))
@@ -5815,6 +5837,7 @@ static u64 nfsd_foreach_client_lock(struct nfs4_client *clp, u64 max,
struct nfs4_ol_stateid *lst, *lst_next;
u64 count = 0;
+ spin_lock(&clp->cl_lock);
list_for_each_entry(oop, &clp->cl_openowners, oo_perclient) {
list_for_each_entry_safe(stp, st_next,
&oop->oo_owner.so_stateids, st_perstateowner) {
@@ -5822,31 +5845,122 @@ static u64 nfsd_foreach_client_lock(struct nfs4_client *clp, u64 max,
&stp->st_locks, st_locks) {
if (func) {
func(lst);
- if (collect)
- list_add(&lst->st_locks,
- collect);
+ nfsd_inject_add_lock_to_list(lst,
+ collect);
}
- if (++count == max)
- return count;
+ ++count;
+ /*
+ * Despite the fact that these functions deal
+ * with 64-bit integers for "count", we must
+ * ensure that it doesn't blow up the
+ * clp->cl_refcount. Throw a warning if we
+ * start to approach INT_MAX here.
+ */
+ WARN_ON_ONCE(count == (INT_MAX / 2));
+ if (count == max)
+ goto out;
}
}
}
+out:
+ spin_unlock(&clp->cl_lock);
return count;
}
-u64 nfsd_forget_client_locks(struct nfs4_client *clp, u64 max)
+static u64
+nfsd_collect_client_locks(struct nfs4_client *clp, struct list_head *collect,
+ u64 max)
{
- return nfsd_foreach_client_lock(clp, max, NULL, release_lock_stateid);
+ return nfsd_foreach_client_lock(clp, max, collect, unhash_lock_stateid);
}
-u64 nfsd_print_client_locks(struct nfs4_client *clp, u64 max)
+static u64
+nfsd_print_client_locks(struct nfs4_client *clp)
{
- u64 count = nfsd_foreach_client_lock(clp, max, NULL, NULL);
+ u64 count = nfsd_foreach_client_lock(clp, 0, NULL, NULL);
nfsd_print_count(clp, count, "locked files");
return count;
}
+u64
+nfsd_inject_print_locks(struct nfsd_fault_inject_op *op)
+{
+ struct nfs4_client *clp;
+ u64 count = 0;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+
+ if (!nfsd_netns_ready(nn))
+ return 0;
+
+ spin_lock(&nn->client_lock);
+ list_for_each_entry(clp, &nn->client_lru, cl_lru)
+ count += nfsd_print_client_locks(clp);
+ spin_unlock(&nn->client_lock);
+
+ return count;
+}
+
+static void
+nfsd_reap_locks(struct list_head *reaplist)
+{
+ struct nfs4_client *clp;
+ struct nfs4_ol_stateid *stp, *next;
+
+ list_for_each_entry_safe(stp, next, reaplist, st_locks) {
+ list_del_init(&stp->st_locks);
+ clp = stp->st_stid.sc_client;
+ nfs4_put_stid(&stp->st_stid);
+ put_client(clp);
+ }
+}
+
+u64
+nfsd_inject_forget_client_locks(struct nfsd_fault_inject_op *op,
+ struct sockaddr_storage *addr, size_t addr_size)
+{
+ unsigned int count = 0;
+ struct nfs4_client *clp;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+ LIST_HEAD(reaplist);
+
+ if (!nfsd_netns_ready(nn))
+ return count;
+
+ spin_lock(&nn->client_lock);
+ clp = nfsd_find_client(addr, addr_size);
+ if (clp)
+ count = nfsd_collect_client_locks(clp, &reaplist, 0);
+ spin_unlock(&nn->client_lock);
+ nfsd_reap_locks(&reaplist);
+ return count;
+}
+
+u64
+nfsd_inject_forget_locks(struct nfsd_fault_inject_op *op, u64 max)
+{
+ u64 count = 0;
+ struct nfs4_client *clp;
+ struct nfsd_net *nn = net_generic(current->nsproxy->net_ns,
+ nfsd_net_id);
+ LIST_HEAD(reaplist);
+
+ if (!nfsd_netns_ready(nn))
+ return count;
+
+ spin_lock(&nn->client_lock);
+ list_for_each_entry(clp, &nn->client_lru, cl_lru) {
+ count += nfsd_collect_client_locks(clp, &reaplist, max - count);
+ if (max != 0 && count >= max)
+ break;
+ }
+ spin_unlock(&nn->client_lock);
+ nfsd_reap_locks(&reaplist);
+ return count;
+}
+
static u64 nfsd_foreach_client_open(struct nfs4_client *clp, u64 max, void (*func)(struct nfs4_openowner *))
{
struct nfs4_openowner *oop, *next;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 480fce8d5cc8..cfd6277b966d 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -483,12 +483,15 @@ u64 nfsd_inject_forget_client(struct nfsd_fault_inject_op *,
struct sockaddr_storage *, size_t);
u64 nfsd_inject_forget_clients(struct nfsd_fault_inject_op *, u64);
-u64 nfsd_forget_client_locks(struct nfs4_client*, u64);
+u64 nfsd_inject_print_locks(struct nfsd_fault_inject_op *);
+u64 nfsd_inject_forget_client_locks(struct nfsd_fault_inject_op *,
+ struct sockaddr_storage *, size_t);
+u64 nfsd_inject_forget_locks(struct nfsd_fault_inject_op *, u64);
+
u64 nfsd_forget_client_openowners(struct nfs4_client *, u64);
u64 nfsd_forget_client_delegations(struct nfs4_client *, u64);
u64 nfsd_recall_client_delegations(struct nfs4_client *, u64);
-u64 nfsd_print_client_locks(struct nfs4_client *, u64);
u64 nfsd_print_client_openowners(struct nfs4_client *, u64);
u64 nfsd_print_client_delegations(struct nfs4_client *, u64);
#else /* CONFIG_NFSD_FAULT_INJECTION */
--
1.9.3
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7eea82e1d779..a33ce55925fb 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6402,7 +6402,6 @@ nfs4_state_shutdown_net(struct net *net)
cancel_delayed_work_sync(&nn->laundromat_work);
locks_end_grace(&nn->nfsd4_manager);
- nfs4_lock_state();
INIT_LIST_HEAD(&reaplist);
spin_lock(&state_lock);
list_for_each_safe(pos, next, &nn->del_recall_lru) {
@@ -6419,7 +6418,6 @@ nfs4_state_shutdown_net(struct net *net)
nfsd4_client_tracking_exit(net);
nfs4_state_destroy_net(net);
- nfs4_unlock_state();
}
void
--
1.9.3
From: Trond Myklebust <[email protected]>
When we remove the client_mutex protection, we will need to ensure
that it can't be found by other threads while we're destroying it.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 43 +++++++++++++++++++++++++++++++++----------
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 52ec47de1185..cb630db015b0 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1588,12 +1588,23 @@ free_client(struct nfs4_client *clp)
}
/* must be called under the client_lock */
-static inline void
+static void
unhash_client_locked(struct nfs4_client *clp)
{
+ struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
struct nfsd4_session *ses;
- list_del(&clp->cl_lru);
+ /* Mark the client as expired! */
+ clp->cl_time = 0;
+ /* Make it invisible */
+ if (!list_empty(&clp->cl_idhash)) {
+ list_del_init(&clp->cl_idhash);
+ if (test_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags))
+ rb_erase(&clp->cl_namenode, &nn->conf_name_tree);
+ else
+ rb_erase(&clp->cl_namenode, &nn->unconf_name_tree);
+ }
+ list_del_init(&clp->cl_lru);
spin_lock(&clp->cl_lock);
list_for_each_entry(ses, &clp->cl_sessions, se_perclnt)
list_del_init(&ses->se_hash);
@@ -1601,7 +1612,17 @@ unhash_client_locked(struct nfs4_client *clp)
}
static void
-destroy_client(struct nfs4_client *clp)
+unhash_client(struct nfs4_client *clp)
+{
+ struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+
+ spin_lock(&nn->client_lock);
+ unhash_client_locked(clp);
+ spin_unlock(&nn->client_lock);
+}
+
+static void
+__destroy_client(struct nfs4_client *clp)
{
struct nfs4_openowner *oo;
struct nfs4_delegation *dp;
@@ -1634,22 +1655,24 @@ destroy_client(struct nfs4_client *clp)
nfsd4_shutdown_callback(clp);
if (clp->cl_cb_conn.cb_xprt)
svc_xprt_put(clp->cl_cb_conn.cb_xprt);
- list_del(&clp->cl_idhash);
- if (test_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags))
- rb_erase(&clp->cl_namenode, &nn->conf_name_tree);
- else
- rb_erase(&clp->cl_namenode, &nn->unconf_name_tree);
spin_lock(&nn->client_lock);
- unhash_client_locked(clp);
WARN_ON_ONCE(atomic_read(&clp->cl_refcount));
free_client(clp);
spin_unlock(&nn->client_lock);
}
+static void
+destroy_client(struct nfs4_client *clp)
+{
+ unhash_client(clp);
+ __destroy_client(clp);
+}
+
static void expire_client(struct nfs4_client *clp)
{
+ unhash_client(clp);
nfsd4_client_record_remove(clp);
- destroy_client(clp);
+ __destroy_client(clp);
}
static void copy_verf(struct nfs4_client *target, nfs4_verifier *source)
--
1.9.3
From: Trond Myklebust <[email protected]>
If we leave the client on the confirmed/unconfirmed tables, and leave
the sessions visible on the sessionid_hashtbl, then someone might
find them before we've had a chance to destroy them.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfsd/nfs4state.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index cb630db015b0..a374592e7dcf 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4125,13 +4125,15 @@ nfs4_laundromat(struct nfsd_net *nn)
clp->cl_clientid.cl_id);
continue;
}
- list_move(&clp->cl_lru, &reaplist);
+ unhash_client_locked(clp);
+ list_add(&clp->cl_lru, &reaplist);
}
spin_unlock(&nn->client_lock);
list_for_each_safe(pos, next, &reaplist) {
clp = list_entry(pos, struct nfs4_client, cl_lru);
dprintk("NFSD: purging unused client (clientid %08x)\n",
clp->cl_clientid.cl_id);
+ list_del_init(&clp->cl_lru);
expire_client(clp);
}
spin_lock(&state_lock);
--
1.9.3
In particular, we want to ensure that the move_to_confirmed() is
protected by the nn->client_lock spin lock, so that we can use that when
looking up the clientid etc. instead of relying on the client_mutex.
Signed-off-by: Trond Myklebust <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4state.c | 65 ++++++++++++++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 26 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f149e30475db..52a4677f6f35 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -137,17 +137,6 @@ static __be32 mark_client_expired_locked(struct nfs4_client *clp)
return nfs_ok;
}
-static __be32 mark_client_expired(struct nfs4_client *clp)
-{
- struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
- __be32 ret;
-
- spin_lock(&nn->client_lock);
- ret = mark_client_expired_locked(clp);
- spin_unlock(&nn->client_lock);
- return ret;
-}
-
static __be32 get_client_locked(struct nfs4_client *clp)
{
if (is_client_expired(clp))
@@ -1437,12 +1426,10 @@ static void init_session(struct svc_rqst *rqstp, struct nfsd4_session *new, stru
new->se_cb_sec = cses->cb_sec;
atomic_set(&new->se_ref, 0);
idx = hash_sessionid(&new->se_sessionid);
- spin_lock(&nn->client_lock);
list_add(&new->se_hash, &nn->sessionid_hashtbl[idx]);
spin_lock(&clp->cl_lock);
list_add(&new->se_perclnt, &clp->cl_sessions);
spin_unlock(&clp->cl_lock);
- spin_unlock(&nn->client_lock);
if (cses->flags & SESSION4_BACK_CHAN) {
struct sockaddr *sa = svc_addr(rqstp);
@@ -2411,6 +2398,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
{
struct sockaddr *sa = svc_addr(rqstp);
struct nfs4_client *conf, *unconf;
+ struct nfs4_client *old = NULL;
struct nfsd4_session *new;
struct nfsd4_conn *conn;
struct nfsd4_clid_slot *cs_slot = NULL;
@@ -2437,6 +2425,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
goto out_free_session;
nfs4_lock_state();
+ spin_lock(&nn->client_lock);
unconf = find_unconfirmed_client(&cr_ses->clientid, true, nn);
conf = find_confirmed_client(&cr_ses->clientid, true, nn);
WARN_ON_ONCE(conf && unconf);
@@ -2455,7 +2444,6 @@ nfsd4_create_session(struct svc_rqst *rqstp,
goto out_free_conn;
}
} else if (unconf) {
- struct nfs4_client *old;
if (!same_creds(&unconf->cl_cred, &rqstp->rq_cred) ||
!rpc_cmp_addr(sa, (struct sockaddr *) &unconf->cl_addr)) {
status = nfserr_clid_inuse;
@@ -2473,10 +2461,10 @@ nfsd4_create_session(struct svc_rqst *rqstp,
}
old = find_confirmed_client_by_name(&unconf->cl_name, nn);
if (old) {
- status = mark_client_expired(old);
+ status = mark_client_expired_locked(old);
if (status)
goto out_free_conn;
- expire_client(old);
+ unhash_client_locked(old);
}
move_to_confirmed(unconf);
conf = unconf;
@@ -2492,20 +2480,29 @@ nfsd4_create_session(struct svc_rqst *rqstp,
cr_ses->flags &= ~SESSION4_RDMA;
init_session(rqstp, new, conf, cr_ses);
- nfsd4_init_conn(rqstp, conn, new);
+ nfsd4_get_session_locked(new);
memcpy(cr_ses->sessionid.data, new->se_sessionid.data,
NFS4_MAX_SESSIONID_LEN);
cs_slot->sl_seqid++;
cr_ses->seqid = cs_slot->sl_seqid;
- /* cache solo and embedded create sessions under the state lock */
+ /* cache solo and embedded create sessions under the client_lock */
nfsd4_cache_create_session(cr_ses, cs_slot, status);
+ spin_unlock(&nn->client_lock);
+ /* init connection and backchannel */
+ nfsd4_init_conn(rqstp, conn, new);
+ nfsd4_put_session(new);
nfs4_unlock_state();
+ if (old)
+ expire_client(old);
return status;
out_free_conn:
+ spin_unlock(&nn->client_lock);
nfs4_unlock_state();
free_conn(conn);
+ if (old)
+ expire_client(old);
out_free_session:
__free_session(new);
out_release_drc_mem:
@@ -2965,6 +2962,7 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
struct nfsd4_setclientid_confirm *setclientid_confirm)
{
struct nfs4_client *conf, *unconf;
+ struct nfs4_client *old = NULL;
nfs4_verifier confirm = setclientid_confirm->sc_confirm;
clientid_t * clid = &setclientid_confirm->sc_clientid;
__be32 status;
@@ -2974,6 +2972,7 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
return nfserr_stale_clientid;
nfs4_lock_state();
+ spin_lock(&nn->client_lock);
conf = find_confirmed_client(clid, false, nn);
unconf = find_unconfirmed_client(clid, false, nn);
/*
@@ -2997,21 +2996,29 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
}
status = nfs_ok;
if (conf) { /* case 1: callback update */
+ old = unconf;
+ unhash_client_locked(old);
nfsd4_change_callback(conf, &unconf->cl_cb_conn);
- nfsd4_probe_callback(conf);
- expire_client(unconf);
} else { /* case 3: normal case; new or rebooted client */
- conf = find_confirmed_client_by_name(&unconf->cl_name, nn);
- if (conf) {
- status = mark_client_expired(conf);
+ old = find_confirmed_client_by_name(&unconf->cl_name, nn);
+ if (old) {
+ status = mark_client_expired_locked(old);
if (status)
goto out;
- expire_client(conf);
+ unhash_client_locked(old);
}
move_to_confirmed(unconf);
- nfsd4_probe_callback(unconf);
+ conf = unconf;
}
+ get_client_locked(conf);
+ spin_unlock(&nn->client_lock);
+ nfsd4_probe_callback(conf);
+ spin_lock(&nn->client_lock);
+ put_client_renew_locked(conf);
out:
+ spin_unlock(&nn->client_lock);
+ if (old)
+ expire_client(old);
nfs4_unlock_state();
return status;
}
@@ -5648,7 +5655,13 @@ nfs4_check_open_reclaim(clientid_t *clid,
u64 nfsd_forget_client(struct nfs4_client *clp, u64 max)
{
- if (mark_client_expired(clp))
+ __be32 ret;
+ struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+
+ spin_lock(&nn->client_lock);
+ ret = mark_client_expired_locked(clp);
+ spin_unlock(&nn->client_lock);
+ if (ret != nfs_ok)
return 0;
expire_client(clp);
return 1;
--
1.9.3
Thanks for the documentation! A couple comments:
On Wed, Jul 30, 2014 at 08:27:38AM -0400, Jeff Layton wrote:
> Add some comments that describe what each of these objects is, and how
> they related to one another.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/nfsd/netns.h | 8 +++++
> fs/nfsd/state.h | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 92 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index 3831ef6e5c75..46680da55cd7 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -34,6 +34,14 @@
> struct cld_net;
> struct nfsd4_client_tracking_ops;
>
> +/*
> + * Represents a nfsd "container". With respect to nfsv4 state tracking, the
> + * fields of interest are the *_id_hashtbls and the *_name_tree. These track
> + * the nfs4_client objects by either short or long form clientid.
> + *
> + * Each nfsd_net runs a nfs4_laundromat workqueue job every lease period to
It's a little more complicated than that suggests--maybe replace "every
lease period" by "when necessary"?
> + * clean up expired clients and delegations within the container.
> + */
> struct nfsd_net {
> struct cld_net *cld_net;
>
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index a363f6bb621e..52ccd07d92ed 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -72,6 +72,11 @@ struct nfsd4_callback {
> bool cb_done;
> };
>
> +/*
> + * A core object that represents a "common" stateid. These are generally
> + * embedded within the different (more specific) stateid objects and contain
> + * fields that are of general use to any stateid.
> + */
> struct nfs4_stid {
> atomic_t sc_count;
> #define NFS4_OPEN_STID 1
> @@ -89,6 +94,18 @@ struct nfs4_stid {
> void (*sc_free)(struct nfs4_stid *);
> };
>
> +/*
> + * Represents a delegation stateid. The nfs4_client holds references to these
> + * and they are put when it is being destroyed or when the delegation is
> + * returned by the client.
This might be worth another sentence or two. I believe the references
are:
- 1 reference as long as a delegation is still in force (taken
when it's alloc'd, put when it's returned or revoked)
- 1 reference as long as a recall rpc is in progress (taken when
the lease is broken, put when the rpc exits.
- 1 more ephemeral reference for each nfsd thread currently
doing something with that delegation without holding
cl_lock?
(By the way, I wonder if that list_for_each_entry() at the end of
nfsd4_process_cb_update actually does anything: hasn't
nfsd4_cb_recall_release() already run and removed any delegation from
the cl_callbacks list? I may just be forgetting how this works.)
--b.
> + * If the server attempts to recall a delegation and the client doesn't do so
> + * before a timeout, the server may also revoke the delegation. In that case,
> + * the object will either be destroyed (v4.0) or moved to a per-client list of
> + * revoked delegations (v4.1+).
> + *
> + * This object is a superset of the nfs4_stid.
> + */
> struct nfs4_delegation {
> struct nfs4_stid dl_stid; /* must be first field */
> struct list_head dl_perfile;
> @@ -195,6 +212,11 @@ struct nfsd4_conn {
> unsigned char cn_flags;
> };
>
> +/*
> + * Representation of a v4.1+ session. These are refcounted in a similar fashion
> + * to the nfs4_client. References are only taken when the server is actively
> + * working on the object (primarily during the processing of compounds).
> + */
> struct nfsd4_session {
> atomic_t se_ref;
> struct list_head se_hash; /* hash by sessionid */
> @@ -224,13 +246,30 @@ struct nfsd4_sessionid {
>
> /*
> * struct nfs4_client - one per client. Clientids live here.
> - * o Each nfs4_client is hashed by clientid.
> *
> - * o Each nfs4_clients is also hashed by name
> - * (the opaque quantity initially sent by the client to identify itself).
> + * The initial object created by an NFS client using SETCLIENTID (for NFSv4.0)
> + * or EXCHANGE_ID (for NFSv4.1+). These objects are refcounted and timestamped.
> + * Each nfsd_net_ns object contains a set of these and they are tracked via
> + * short and long form clientid. They are hashed and searched for under the
> + * per-nfsd_net client_lock spinlock.
> + *
> + * References to it are only held during the processing of compounds, and in
> + * certain other operations. In their "resting state" they have a refcount of
> + * 0. If they are not renewed within a lease period, they become eligible for
> + * destruction by the laundromat.
> + *
> + * These objects can also be destroyed prematurely by the fault injection code,
> + * or if the client sends certain forms of SETCLIENTID or EXCHANGE_ID updates.
> + * Care is taken *not* to do this however when the objects have an elevated
> + * refcount.
> + *
> + * o Each nfs4_client is hashed by clientid
> + *
> + * o Each nfs4_clients is also hashed by name (the opaque quantity initially
> + * sent by the client to identify itself).
> *
> - * o cl_perclient list is used to ensure no dangling stateowner references
> - * when we expire the nfs4_client
> + * o cl_perclient list is used to ensure no dangling stateowner references
> + * when we expire the nfs4_client
> */
> struct nfs4_client {
> struct list_head cl_idhash; /* hash by cl_clientid.id */
> @@ -340,6 +379,12 @@ struct nfs4_stateowner_operations {
> void (*so_free)(struct nfs4_stateowner *);
> };
>
> +/*
> + * A core object that represents either an open or lock owner. The object and
> + * lock owner objects have one of these embedded within them. Refcounts and
> + * other fields common to both owner types are contained within these
> + * structures.
> + */
> struct nfs4_stateowner {
> struct list_head so_strhash;
> struct list_head so_stateids;
> @@ -354,6 +399,12 @@ struct nfs4_stateowner {
> bool so_is_open_owner;
> };
>
> +/*
> + * When a file is opened, the client provides an open state owner opaque string
> + * that indicates the "owner" of that open. These objects are refcounted.
> + * References to it are held by each open state associated with it. This object
> + * is a superset of the nfs4_stateowner struct.
> + */
> struct nfs4_openowner {
> struct nfs4_stateowner oo_owner; /* must be first field */
> struct list_head oo_perclient;
> @@ -371,6 +422,12 @@ struct nfs4_openowner {
> unsigned char oo_flags;
> };
>
> +/*
> + * Represents a generic "lockowner". Similar to an openowner. References to it
> + * are held by the lock stateids that are created on its behalf. This object is
> + * a superset of the nfs4_stateowner struct (or would be if it needed any extra
> + * fields).
> + */
> struct nfs4_lockowner {
> struct nfs4_stateowner lo_owner; /* must be first element */
> };
> @@ -385,7 +442,14 @@ static inline struct nfs4_lockowner * lockowner(struct nfs4_stateowner *so)
> return container_of(so, struct nfs4_lockowner, lo_owner);
> }
>
> -/* nfs4_file: a file opened by some number of (open) nfs4_stateowners. */
> +/*
> + * nfs4_file: a file opened by some number of (open) nfs4_stateowners.
> + *
> + * These objects are global. nfsd only keeps one instance of a nfs4_file per
> + * inode (though it may keep multiple file descriptors open per inode). These
> + * are tracked in the file_hashtbl which is protected by the state_lock
> + * spinlock.
> + */
> struct nfs4_file {
> atomic_t fi_ref;
> spinlock_t fi_lock;
> @@ -410,7 +474,20 @@ struct nfs4_file {
> bool fi_had_conflict;
> };
>
> -/* "ol" stands for "Open or Lock". Better suggestions welcome. */
> +/*
> + * A generic struct representing either a open or lock stateid. The nfs4_client
> + * holds a reference to each of these objects, and they in turn hold a
> + * reference to their respective stateowners. The client's reference is
> + * released in response to a close or unlock (depending on whether it's an open
> + * or lock stateid) or when the client is being destroyed.
> + *
> + * In the case of v4.0 open stateids, these objects are preserved for a little
> + * while after close in order to handle CLOSE replays. Those are eventually
> + * reclaimed via a LRU scheme by the laundromat.
> + *
> + * This object is a superset of the nfs4_stid. "ol" stands for "Open or Lock".
> + * Better suggestions welcome.
> + */
> struct nfs4_ol_stateid {
> struct nfs4_stid st_stid; /* must be first field */
> struct list_head st_perfile;
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 30 Jul 2014 08:27:37 -0400
Jeff Layton <[email protected]> wrote:
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index a33ce55925fb..de811dc39061 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -75,9 +75,6 @@ static void nfs4_free_ol_stateid(struct nfs4_stid *stid);
>
> /* Locking: */
>
> -/* Currently used for almost all code touching nfsv4 state: */
> -static DEFINE_MUTEX(client_mutex);
> -
> /*
> * Currently used for the del_recall_lru and file hash table. In an
> * effort to decrease the scope of the client_mutex, this spinlock may
> @@ -97,12 +94,6 @@ static struct kmem_cache *file_slab;
> static struct kmem_cache *stateid_slab;
> static struct kmem_cache *deleg_slab;
>
> -void
> -nfs4_lock_state(void)
> -{
> - mutex_lock(&client_mutex);
> -}
> -
> static void free_session(struct nfsd4_session *);
>
> static bool is_session_dead(struct nfsd4_session *ses)
> @@ -118,12 +109,6 @@ static __be32 mark_session_dead_locked(struct nfsd4_session *ses, int ref_held_b
> return nfs_ok;
> }
>
> -void
> -nfs4_unlock_state(void)
> -{
> - mutex_unlock(&client_mutex);
> -}
> -
> static bool is_client_expired(struct nfs4_client *clp)
> {
> return clp->cl_time == 0;
I just noticed that the above patch neglects to remove the declarations
of nfs4_lock/unlock_state from state.h. Bruce, let me know if you want me
to resend. Otherwise, I'll assume that you can fix that up.
Thanks,
--
Jeff Layton <[email protected]>
I have these queued for 3.17 up through this patch; I'll continue
working through these tomorrow.
(In my git tree, anything in the for-3.17 branch I consider committed,
anything in nfsd-next is stuff I'm still considering for this merge
window but may not really have reviewed yet.)
--b.
On Wed, Jul 30, 2014 at 08:27:10AM -0400, Jeff Layton wrote:
> From: Trond Myklebust <[email protected]>
>
> Signed-off-by: Trond Myklebust <[email protected]>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index f9d077d800ee..e7dfd4e9d942 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -139,6 +139,10 @@ static __be32 mark_client_expired_locked(struct nfs4_client *clp)
>
> static __be32 get_client_locked(struct nfs4_client *clp)
> {
> + struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
> +
> + lockdep_assert_held(&nn->client_lock);
> +
> if (is_client_expired(clp))
> return nfserr_expired;
> atomic_inc(&clp->cl_refcount);
> @@ -179,6 +183,10 @@ renew_client(struct nfs4_client *clp)
>
> static void put_client_renew_locked(struct nfs4_client *clp)
> {
> + struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
> +
> + lockdep_assert_held(&nn->client_lock);
> +
> if (!atomic_dec_and_test(&clp->cl_refcount))
> return;
> if (!is_client_expired(clp))
> @@ -212,6 +220,9 @@ static __be32 nfsd4_get_session_locked(struct nfsd4_session *ses)
> static void nfsd4_put_session_locked(struct nfsd4_session *ses)
> {
> struct nfs4_client *clp = ses->se_client;
> + struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
> +
> + lockdep_assert_held(&nn->client_lock);
>
> if (atomic_dec_and_test(&ses->se_ref) && is_session_dead(ses))
> free_session(ses);
> @@ -1453,6 +1464,8 @@ __find_in_sessionid_hashtbl(struct nfs4_sessionid *sessionid, struct net *net)
> int idx;
> struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>
> + lockdep_assert_held(&nn->client_lock);
> +
> dump_sessionid(__func__, sessionid);
> idx = hash_sessionid(sessionid);
> /* Search in the appropriate list */
> @@ -1489,6 +1502,11 @@ out:
> static void
> unhash_session(struct nfsd4_session *ses)
> {
> + struct nfs4_client *clp = ses->se_client;
> + struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
> +
> + lockdep_assert_held(&nn->client_lock);
> +
> list_del(&ses->se_hash);
> spin_lock(&ses->se_client->cl_lock);
> list_del(&ses->se_perclnt);
> @@ -1575,6 +1593,8 @@ unhash_client_locked(struct nfs4_client *clp)
> struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
> struct nfsd4_session *ses;
>
> + lockdep_assert_held(&nn->client_lock);
> +
> /* Mark the client as expired! */
> clp->cl_time = 0;
> /* Make it invisible */
> @@ -1906,6 +1926,8 @@ add_to_unconfirmed(struct nfs4_client *clp)
> unsigned int idhashval;
> struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
>
> + lockdep_assert_held(&nn->client_lock);
> +
> clear_bit(NFSD4_CLIENT_CONFIRMED, &clp->cl_flags);
> add_clp_to_name_tree(clp, &nn->unconf_name_tree);
> idhashval = clientid_hashval(clp->cl_clientid.cl_id);
> @@ -1919,6 +1941,8 @@ move_to_confirmed(struct nfs4_client *clp)
> unsigned int idhashval = clientid_hashval(clp->cl_clientid.cl_id);
> struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
>
> + lockdep_assert_held(&nn->client_lock);
> +
> dprintk("NFSD: move_to_confirm nfs4_client %p\n", clp);
> list_move(&clp->cl_idhash, &nn->conf_id_hashtbl[idhashval]);
> rb_erase(&clp->cl_namenode, &nn->unconf_name_tree);
> @@ -1949,6 +1973,7 @@ find_confirmed_client(clientid_t *clid, bool sessions, struct nfsd_net *nn)
> {
> struct list_head *tbl = nn->conf_id_hashtbl;
>
> + lockdep_assert_held(&nn->client_lock);
> return find_client_in_id_table(tbl, clid, sessions);
> }
>
> @@ -1957,6 +1982,7 @@ find_unconfirmed_client(clientid_t *clid, bool sessions, struct nfsd_net *nn)
> {
> struct list_head *tbl = nn->unconf_id_hashtbl;
>
> + lockdep_assert_held(&nn->client_lock);
> return find_client_in_id_table(tbl, clid, sessions);
> }
>
> @@ -1968,12 +1994,14 @@ static bool clp_used_exchangeid(struct nfs4_client *clp)
> static struct nfs4_client *
> find_confirmed_client_by_name(struct xdr_netobj *name, struct nfsd_net *nn)
> {
> + lockdep_assert_held(&nn->client_lock);
> return find_clp_in_name_tree(name, &nn->conf_name_tree);
> }
>
> static struct nfs4_client *
> find_unconfirmed_client_by_name(struct xdr_netobj *name, struct nfsd_net *nn)
> {
> + lockdep_assert_held(&nn->client_lock);
> return find_clp_in_name_tree(name, &nn->unconf_name_tree);
> }
>
> @@ -4907,6 +4935,8 @@ find_lockowner_str_locked(clientid_t *clid, struct xdr_netobj *owner,
> unsigned int strhashval = ownerstr_hashval(owner);
> struct nfs4_stateowner *so;
>
> + lockdep_assert_held(&clp->cl_lock);
> +
> list_for_each_entry(so, &clp->cl_ownerstr_hashtbl[strhashval],
> so_strhash) {
> if (so->so_is_open_owner)
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 05, 2014 at 03:13:30PM -0400, Jeff Layton wrote:
> Add some comments that describe what each of these objects is, and how
> they related to one another.
Thanks!--b.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/nfsd/netns.h | 8 +++++
> fs/nfsd/state.h | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 101 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index 3831ef6e5c75..ea6749a32760 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -34,6 +34,14 @@
> struct cld_net;
> struct nfsd4_client_tracking_ops;
>
> +/*
> + * Represents a nfsd "container". With respect to nfsv4 state tracking, the
> + * fields of interest are the *_id_hashtbls and the *_name_tree. These track
> + * the nfs4_client objects by either short or long form clientid.
> + *
> + * Each nfsd_net runs a nfs4_laundromat workqueue job when necessary to clean
> + * up expired clients and delegations within the container.
> + */
> struct nfsd_net {
> struct cld_net *cld_net;
>
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 32a7c290d027..4a89e00d7461 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -72,6 +72,11 @@ struct nfsd4_callback {
> bool cb_done;
> };
>
> +/*
> + * A core object that represents a "common" stateid. These are generally
> + * embedded within the different (more specific) stateid objects and contain
> + * fields that are of general use to any stateid.
> + */
> struct nfs4_stid {
> atomic_t sc_count;
> #define NFS4_OPEN_STID 1
> @@ -89,6 +94,27 @@ struct nfs4_stid {
> void (*sc_free)(struct nfs4_stid *);
> };
>
> +/*
> + * Represents a delegation stateid. The nfs4_client holds references to these
> + * and they are put when it is being destroyed or when the delegation is
> + * returned by the client:
> + *
> + * o 1 reference as long as a delegation is still in force (taken when it's
> + * alloc'd, put when it's returned or revoked)
> + *
> + * o 1 reference as long as a recall rpc is in progress (taken when the lease
> + * is broken, put when the rpc exits)
> + *
> + * o 1 more ephemeral reference for each nfsd thread currently doing something
> + * with that delegation without holding the cl_lock
> + *
> + * If the server attempts to recall a delegation and the client doesn't do so
> + * before a timeout, the server may also revoke the delegation. In that case,
> + * the object will either be destroyed (v4.0) or moved to a per-client list of
> + * revoked delegations (v4.1+).
> + *
> + * This object is a superset of the nfs4_stid.
> + */
> struct nfs4_delegation {
> struct nfs4_stid dl_stid; /* must be first field */
> struct list_head dl_perfile;
> @@ -195,6 +221,11 @@ struct nfsd4_conn {
> unsigned char cn_flags;
> };
>
> +/*
> + * Representation of a v4.1+ session. These are refcounted in a similar fashion
> + * to the nfs4_client. References are only taken when the server is actively
> + * working on the object (primarily during the processing of compounds).
> + */
> struct nfsd4_session {
> atomic_t se_ref;
> struct list_head se_hash; /* hash by sessionid */
> @@ -224,13 +255,30 @@ struct nfsd4_sessionid {
>
> /*
> * struct nfs4_client - one per client. Clientids live here.
> - * o Each nfs4_client is hashed by clientid.
> *
> - * o Each nfs4_clients is also hashed by name
> - * (the opaque quantity initially sent by the client to identify itself).
> + * The initial object created by an NFS client using SETCLIENTID (for NFSv4.0)
> + * or EXCHANGE_ID (for NFSv4.1+). These objects are refcounted and timestamped.
> + * Each nfsd_net_ns object contains a set of these and they are tracked via
> + * short and long form clientid. They are hashed and searched for under the
> + * per-nfsd_net client_lock spinlock.
> + *
> + * References to it are only held during the processing of compounds, and in
> + * certain other operations. In their "resting state" they have a refcount of
> + * 0. If they are not renewed within a lease period, they become eligible for
> + * destruction by the laundromat.
> + *
> + * These objects can also be destroyed prematurely by the fault injection code,
> + * or if the client sends certain forms of SETCLIENTID or EXCHANGE_ID updates.
> + * Care is taken *not* to do this however when the objects have an elevated
> + * refcount.
> + *
> + * o Each nfs4_client is hashed by clientid
> + *
> + * o Each nfs4_clients is also hashed by name (the opaque quantity initially
> + * sent by the client to identify itself).
> *
> - * o cl_perclient list is used to ensure no dangling stateowner references
> - * when we expire the nfs4_client
> + * o cl_perclient list is used to ensure no dangling stateowner references
> + * when we expire the nfs4_client
> */
> struct nfs4_client {
> struct list_head cl_idhash; /* hash by cl_clientid.id */
> @@ -340,6 +388,12 @@ struct nfs4_stateowner_operations {
> void (*so_free)(struct nfs4_stateowner *);
> };
>
> +/*
> + * A core object that represents either an open or lock owner. The object and
> + * lock owner objects have one of these embedded within them. Refcounts and
> + * other fields common to both owner types are contained within these
> + * structures.
> + */
> struct nfs4_stateowner {
> struct list_head so_strhash;
> struct list_head so_stateids;
> @@ -354,6 +408,12 @@ struct nfs4_stateowner {
> bool so_is_open_owner;
> };
>
> +/*
> + * When a file is opened, the client provides an open state owner opaque string
> + * that indicates the "owner" of that open. These objects are refcounted.
> + * References to it are held by each open state associated with it. This object
> + * is a superset of the nfs4_stateowner struct.
> + */
> struct nfs4_openowner {
> struct nfs4_stateowner oo_owner; /* must be first field */
> struct list_head oo_perclient;
> @@ -371,6 +431,12 @@ struct nfs4_openowner {
> unsigned char oo_flags;
> };
>
> +/*
> + * Represents a generic "lockowner". Similar to an openowner. References to it
> + * are held by the lock stateids that are created on its behalf. This object is
> + * a superset of the nfs4_stateowner struct (or would be if it needed any extra
> + * fields).
> + */
> struct nfs4_lockowner {
> struct nfs4_stateowner lo_owner; /* must be first element */
> };
> @@ -385,7 +451,14 @@ static inline struct nfs4_lockowner * lockowner(struct nfs4_stateowner *so)
> return container_of(so, struct nfs4_lockowner, lo_owner);
> }
>
> -/* nfs4_file: a file opened by some number of (open) nfs4_stateowners. */
> +/*
> + * nfs4_file: a file opened by some number of (open) nfs4_stateowners.
> + *
> + * These objects are global. nfsd only keeps one instance of a nfs4_file per
> + * inode (though it may keep multiple file descriptors open per inode). These
> + * are tracked in the file_hashtbl which is protected by the state_lock
> + * spinlock.
> + */
> struct nfs4_file {
> atomic_t fi_ref;
> spinlock_t fi_lock;
> @@ -410,7 +483,20 @@ struct nfs4_file {
> bool fi_had_conflict;
> };
>
> -/* "ol" stands for "Open or Lock". Better suggestions welcome. */
> +/*
> + * A generic struct representing either a open or lock stateid. The nfs4_client
> + * holds a reference to each of these objects, and they in turn hold a
> + * reference to their respective stateowners. The client's reference is
> + * released in response to a close or unlock (depending on whether it's an open
> + * or lock stateid) or when the client is being destroyed.
> + *
> + * In the case of v4.0 open stateids, these objects are preserved for a little
> + * while after close in order to handle CLOSE replays. Those are eventually
> + * reclaimed via a LRU scheme by the laundromat.
> + *
> + * This object is a superset of the nfs4_stid. "ol" stands for "Open or Lock".
> + * Better suggestions welcome.
> + */
> struct nfs4_ol_stateid {
> struct nfs4_stid st_stid; /* must be first field */
> struct list_head st_perfile;
> --
> 1.9.3
>
On Tue, Aug 05, 2014 at 01:01:05PM -0400, Jeff Layton wrote:
> On Tue, 5 Aug 2014 11:36:28 -0400
> "J. Bruce Fields" <[email protected]> wrote:
>
> > Thanks for the documentation! A couple comments:
> >
> > On Wed, Jul 30, 2014 at 08:27:38AM -0400, Jeff Layton wrote:
> > > Add some comments that describe what each of these objects is, and how
> > > they related to one another.
> > >
> > > Signed-off-by: Jeff Layton <[email protected]>
> > > ---
> > > fs/nfsd/netns.h | 8 +++++
> > > fs/nfsd/state.h | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> > > 2 files changed, 92 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> > > index 3831ef6e5c75..46680da55cd7 100644
> > > --- a/fs/nfsd/netns.h
> > > +++ b/fs/nfsd/netns.h
> > > @@ -34,6 +34,14 @@
> > > struct cld_net;
> > > struct nfsd4_client_tracking_ops;
> > >
> > > +/*
> > > + * Represents a nfsd "container". With respect to nfsv4 state tracking, the
> > > + * fields of interest are the *_id_hashtbls and the *_name_tree. These track
> > > + * the nfs4_client objects by either short or long form clientid.
> > > + *
> > > + * Each nfsd_net runs a nfs4_laundromat workqueue job every lease period to
> >
> > It's a little more complicated than that suggests--maybe replace "every
> > lease period" by "when necessary"?
> >
>
> Ok, sure...
>
> > > + * clean up expired clients and delegations within the container.
> > > + */
> > > struct nfsd_net {
> > > struct cld_net *cld_net;
> > >
> > > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > > index a363f6bb621e..52ccd07d92ed 100644
> > > --- a/fs/nfsd/state.h
> > > +++ b/fs/nfsd/state.h
> > > @@ -72,6 +72,11 @@ struct nfsd4_callback {
> > > bool cb_done;
> > > };
> > >
> > > +/*
> > > + * A core object that represents a "common" stateid. These are generally
> > > + * embedded within the different (more specific) stateid objects and contain
> > > + * fields that are of general use to any stateid.
> > > + */
> > > struct nfs4_stid {
> > > atomic_t sc_count;
> > > #define NFS4_OPEN_STID 1
> > > @@ -89,6 +94,18 @@ struct nfs4_stid {
> > > void (*sc_free)(struct nfs4_stid *);
> > > };
> > >
> > > +/*
> > > + * Represents a delegation stateid. The nfs4_client holds references to these
> > > + * and they are put when it is being destroyed or when the delegation is
> > > + * returned by the client.
> >
> > This might be worth another sentence or two. I believe the references
> > are:
> >
> > - 1 reference as long as a delegation is still in force (taken
> > when it's alloc'd, put when it's returned or revoked)
> > - 1 reference as long as a recall rpc is in progress (taken when
> > the lease is broken, put when the rpc exits.
> > - 1 more ephemeral reference for each nfsd thread currently
> > doing something with that delegation without holding
> > cl_lock?
> >
>
> Sounds about right. Would you rather just add that and the other
> changes you suggested above, or do you want me to resend the patch?
Would you mind resending?
> > (By the way, I wonder if that list_for_each_entry() at the end of
> > nfsd4_process_cb_update actually does anything: hasn't
> > nfsd4_cb_recall_release() already run and removed any delegation from
> > the cl_callbacks list? I may just be forgetting how this works.)
> >
>
> Oh, hm. I'm not sure about that. I didn't touch that bit of code, so I
> don't profess to really understand it either.
>
> Let's see...
>
> It's only going to remove it from the list if cb_done is true, and that
> doesn't happen if there was an error and dl_retries isn't exhausted
> yet. So no, I don't think it's dead code...
Ah, I think you're right. So "when the rpc exits" above should be
something like "when we get a reply to the recall or give up trying".
Thanks again for the documentation here. Notes on the various data
structures' lifetimes are especially useful, I think.
--b.
On Fri, Aug 01, 2014 at 07:20:25AM -0400, Jeff Layton wrote:
> I just noticed that the above patch neglects to remove the declarations
> of nfs4_lock/unlock_state from state.h. Bruce, let me know if you want me
> to resend. Otherwise, I'll assume that you can fix that up.
Done. And merged everything up to here, thanks!
--b.
On Tue, 5 Aug 2014 11:36:28 -0400
"J. Bruce Fields" <[email protected]> wrote:
> Thanks for the documentation! A couple comments:
>
> On Wed, Jul 30, 2014 at 08:27:38AM -0400, Jeff Layton wrote:
> > Add some comments that describe what each of these objects is, and how
> > they related to one another.
> >
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> > fs/nfsd/netns.h | 8 +++++
> > fs/nfsd/state.h | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> > 2 files changed, 92 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> > index 3831ef6e5c75..46680da55cd7 100644
> > --- a/fs/nfsd/netns.h
> > +++ b/fs/nfsd/netns.h
> > @@ -34,6 +34,14 @@
> > struct cld_net;
> > struct nfsd4_client_tracking_ops;
> >
> > +/*
> > + * Represents a nfsd "container". With respect to nfsv4 state tracking, the
> > + * fields of interest are the *_id_hashtbls and the *_name_tree. These track
> > + * the nfs4_client objects by either short or long form clientid.
> > + *
> > + * Each nfsd_net runs a nfs4_laundromat workqueue job every lease period to
>
> It's a little more complicated than that suggests--maybe replace "every
> lease period" by "when necessary"?
>
Ok, sure...
> > + * clean up expired clients and delegations within the container.
> > + */
> > struct nfsd_net {
> > struct cld_net *cld_net;
> >
> > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > index a363f6bb621e..52ccd07d92ed 100644
> > --- a/fs/nfsd/state.h
> > +++ b/fs/nfsd/state.h
> > @@ -72,6 +72,11 @@ struct nfsd4_callback {
> > bool cb_done;
> > };
> >
> > +/*
> > + * A core object that represents a "common" stateid. These are generally
> > + * embedded within the different (more specific) stateid objects and contain
> > + * fields that are of general use to any stateid.
> > + */
> > struct nfs4_stid {
> > atomic_t sc_count;
> > #define NFS4_OPEN_STID 1
> > @@ -89,6 +94,18 @@ struct nfs4_stid {
> > void (*sc_free)(struct nfs4_stid *);
> > };
> >
> > +/*
> > + * Represents a delegation stateid. The nfs4_client holds references to these
> > + * and they are put when it is being destroyed or when the delegation is
> > + * returned by the client.
>
> This might be worth another sentence or two. I believe the references
> are:
>
> - 1 reference as long as a delegation is still in force (taken
> when it's alloc'd, put when it's returned or revoked)
> - 1 reference as long as a recall rpc is in progress (taken when
> the lease is broken, put when the rpc exits.
> - 1 more ephemeral reference for each nfsd thread currently
> doing something with that delegation without holding
> cl_lock?
>
Sounds about right. Would you rather just add that and the other
changes you suggested above, or do you want me to resend the patch?
> (By the way, I wonder if that list_for_each_entry() at the end of
> nfsd4_process_cb_update actually does anything: hasn't
> nfsd4_cb_recall_release() already run and removed any delegation from
> the cl_callbacks list? I may just be forgetting how this works.)
>
Oh, hm. I'm not sure about that. I didn't touch that bit of code, so I
don't profess to really understand it either.
Let's see...
It's only going to remove it from the list if cb_done is true, and that
doesn't happen if there was an error and dl_retries isn't exhausted
yet. So no, I don't think it's dead code...
> --b.
>
> > + * If the server attempts to recall a delegation and the client doesn't do so
> > + * before a timeout, the server may also revoke the delegation. In that case,
> > + * the object will either be destroyed (v4.0) or moved to a per-client list of
> > + * revoked delegations (v4.1+).
> > + *
> > + * This object is a superset of the nfs4_stid.
> > + */
> > struct nfs4_delegation {
> > struct nfs4_stid dl_stid; /* must be first field */
> > struct list_head dl_perfile;
> > @@ -195,6 +212,11 @@ struct nfsd4_conn {
> > unsigned char cn_flags;
> > };
> >
> > +/*
> > + * Representation of a v4.1+ session. These are refcounted in a similar fashion
> > + * to the nfs4_client. References are only taken when the server is actively
> > + * working on the object (primarily during the processing of compounds).
> > + */
> > struct nfsd4_session {
> > atomic_t se_ref;
> > struct list_head se_hash; /* hash by sessionid */
> > @@ -224,13 +246,30 @@ struct nfsd4_sessionid {
> >
> > /*
> > * struct nfs4_client - one per client. Clientids live here.
> > - * o Each nfs4_client is hashed by clientid.
> > *
> > - * o Each nfs4_clients is also hashed by name
> > - * (the opaque quantity initially sent by the client to identify itself).
> > + * The initial object created by an NFS client using SETCLIENTID (for NFSv4.0)
> > + * or EXCHANGE_ID (for NFSv4.1+). These objects are refcounted and timestamped.
> > + * Each nfsd_net_ns object contains a set of these and they are tracked via
> > + * short and long form clientid. They are hashed and searched for under the
> > + * per-nfsd_net client_lock spinlock.
> > + *
> > + * References to it are only held during the processing of compounds, and in
> > + * certain other operations. In their "resting state" they have a refcount of
> > + * 0. If they are not renewed within a lease period, they become eligible for
> > + * destruction by the laundromat.
> > + *
> > + * These objects can also be destroyed prematurely by the fault injection code,
> > + * or if the client sends certain forms of SETCLIENTID or EXCHANGE_ID updates.
> > + * Care is taken *not* to do this however when the objects have an elevated
> > + * refcount.
> > + *
> > + * o Each nfs4_client is hashed by clientid
> > + *
> > + * o Each nfs4_clients is also hashed by name (the opaque quantity initially
> > + * sent by the client to identify itself).
> > *
> > - * o cl_perclient list is used to ensure no dangling stateowner references
> > - * when we expire the nfs4_client
> > + * o cl_perclient list is used to ensure no dangling stateowner references
> > + * when we expire the nfs4_client
> > */
> > struct nfs4_client {
> > struct list_head cl_idhash; /* hash by cl_clientid.id */
> > @@ -340,6 +379,12 @@ struct nfs4_stateowner_operations {
> > void (*so_free)(struct nfs4_stateowner *);
> > };
> >
> > +/*
> > + * A core object that represents either an open or lock owner. The object and
> > + * lock owner objects have one of these embedded within them. Refcounts and
> > + * other fields common to both owner types are contained within these
> > + * structures.
> > + */
> > struct nfs4_stateowner {
> > struct list_head so_strhash;
> > struct list_head so_stateids;
> > @@ -354,6 +399,12 @@ struct nfs4_stateowner {
> > bool so_is_open_owner;
> > };
> >
> > +/*
> > + * When a file is opened, the client provides an open state owner opaque string
> > + * that indicates the "owner" of that open. These objects are refcounted.
> > + * References to it are held by each open state associated with it. This object
> > + * is a superset of the nfs4_stateowner struct.
> > + */
> > struct nfs4_openowner {
> > struct nfs4_stateowner oo_owner; /* must be first field */
> > struct list_head oo_perclient;
> > @@ -371,6 +422,12 @@ struct nfs4_openowner {
> > unsigned char oo_flags;
> > };
> >
> > +/*
> > + * Represents a generic "lockowner". Similar to an openowner. References to it
> > + * are held by the lock stateids that are created on its behalf. This object is
> > + * a superset of the nfs4_stateowner struct (or would be if it needed any extra
> > + * fields).
> > + */
> > struct nfs4_lockowner {
> > struct nfs4_stateowner lo_owner; /* must be first element */
> > };
> > @@ -385,7 +442,14 @@ static inline struct nfs4_lockowner * lockowner(struct nfs4_stateowner *so)
> > return container_of(so, struct nfs4_lockowner, lo_owner);
> > }
> >
> > -/* nfs4_file: a file opened by some number of (open) nfs4_stateowners. */
> > +/*
> > + * nfs4_file: a file opened by some number of (open) nfs4_stateowners.
> > + *
> > + * These objects are global. nfsd only keeps one instance of a nfs4_file per
> > + * inode (though it may keep multiple file descriptors open per inode). These
> > + * are tracked in the file_hashtbl which is protected by the state_lock
> > + * spinlock.
> > + */
> > struct nfs4_file {
> > atomic_t fi_ref;
> > spinlock_t fi_lock;
> > @@ -410,7 +474,20 @@ struct nfs4_file {
> > bool fi_had_conflict;
> > };
> >
> > -/* "ol" stands for "Open or Lock". Better suggestions welcome. */
> > +/*
> > + * A generic struct representing either a open or lock stateid. The nfs4_client
> > + * holds a reference to each of these objects, and they in turn hold a
> > + * reference to their respective stateowners. The client's reference is
> > + * released in response to a close or unlock (depending on whether it's an open
> > + * or lock stateid) or when the client is being destroyed.
> > + *
> > + * In the case of v4.0 open stateids, these objects are preserved for a little
> > + * while after close in order to handle CLOSE replays. Those are eventually
> > + * reclaimed via a LRU scheme by the laundromat.
> > + *
> > + * This object is a superset of the nfs4_stid. "ol" stands for "Open or Lock".
> > + * Better suggestions welcome.
> > + */
> > struct nfs4_ol_stateid {
> > struct nfs4_stid st_stid; /* must be first field */
> > struct list_head st_perfile;
> > --
> > 1.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jeff Layton <[email protected]>
Add some comments that describe what each of these objects is, and how
they related to one another.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/netns.h | 8 +++++
fs/nfsd/state.h | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 101 insertions(+), 7 deletions(-)
diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index 3831ef6e5c75..ea6749a32760 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -34,6 +34,14 @@
struct cld_net;
struct nfsd4_client_tracking_ops;
+/*
+ * Represents a nfsd "container". With respect to nfsv4 state tracking, the
+ * fields of interest are the *_id_hashtbls and the *_name_tree. These track
+ * the nfs4_client objects by either short or long form clientid.
+ *
+ * Each nfsd_net runs a nfs4_laundromat workqueue job when necessary to clean
+ * up expired clients and delegations within the container.
+ */
struct nfsd_net {
struct cld_net *cld_net;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 32a7c290d027..4a89e00d7461 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -72,6 +72,11 @@ struct nfsd4_callback {
bool cb_done;
};
+/*
+ * A core object that represents a "common" stateid. These are generally
+ * embedded within the different (more specific) stateid objects and contain
+ * fields that are of general use to any stateid.
+ */
struct nfs4_stid {
atomic_t sc_count;
#define NFS4_OPEN_STID 1
@@ -89,6 +94,27 @@ struct nfs4_stid {
void (*sc_free)(struct nfs4_stid *);
};
+/*
+ * Represents a delegation stateid. The nfs4_client holds references to these
+ * and they are put when it is being destroyed or when the delegation is
+ * returned by the client:
+ *
+ * o 1 reference as long as a delegation is still in force (taken when it's
+ * alloc'd, put when it's returned or revoked)
+ *
+ * o 1 reference as long as a recall rpc is in progress (taken when the lease
+ * is broken, put when the rpc exits)
+ *
+ * o 1 more ephemeral reference for each nfsd thread currently doing something
+ * with that delegation without holding the cl_lock
+ *
+ * If the server attempts to recall a delegation and the client doesn't do so
+ * before a timeout, the server may also revoke the delegation. In that case,
+ * the object will either be destroyed (v4.0) or moved to a per-client list of
+ * revoked delegations (v4.1+).
+ *
+ * This object is a superset of the nfs4_stid.
+ */
struct nfs4_delegation {
struct nfs4_stid dl_stid; /* must be first field */
struct list_head dl_perfile;
@@ -195,6 +221,11 @@ struct nfsd4_conn {
unsigned char cn_flags;
};
+/*
+ * Representation of a v4.1+ session. These are refcounted in a similar fashion
+ * to the nfs4_client. References are only taken when the server is actively
+ * working on the object (primarily during the processing of compounds).
+ */
struct nfsd4_session {
atomic_t se_ref;
struct list_head se_hash; /* hash by sessionid */
@@ -224,13 +255,30 @@ struct nfsd4_sessionid {
/*
* struct nfs4_client - one per client. Clientids live here.
- * o Each nfs4_client is hashed by clientid.
*
- * o Each nfs4_clients is also hashed by name
- * (the opaque quantity initially sent by the client to identify itself).
+ * The initial object created by an NFS client using SETCLIENTID (for NFSv4.0)
+ * or EXCHANGE_ID (for NFSv4.1+). These objects are refcounted and timestamped.
+ * Each nfsd_net_ns object contains a set of these and they are tracked via
+ * short and long form clientid. They are hashed and searched for under the
+ * per-nfsd_net client_lock spinlock.
+ *
+ * References to it are only held during the processing of compounds, and in
+ * certain other operations. In their "resting state" they have a refcount of
+ * 0. If they are not renewed within a lease period, they become eligible for
+ * destruction by the laundromat.
+ *
+ * These objects can also be destroyed prematurely by the fault injection code,
+ * or if the client sends certain forms of SETCLIENTID or EXCHANGE_ID updates.
+ * Care is taken *not* to do this however when the objects have an elevated
+ * refcount.
+ *
+ * o Each nfs4_client is hashed by clientid
+ *
+ * o Each nfs4_clients is also hashed by name (the opaque quantity initially
+ * sent by the client to identify itself).
*
- * o cl_perclient list is used to ensure no dangling stateowner references
- * when we expire the nfs4_client
+ * o cl_perclient list is used to ensure no dangling stateowner references
+ * when we expire the nfs4_client
*/
struct nfs4_client {
struct list_head cl_idhash; /* hash by cl_clientid.id */
@@ -340,6 +388,12 @@ struct nfs4_stateowner_operations {
void (*so_free)(struct nfs4_stateowner *);
};
+/*
+ * A core object that represents either an open or lock owner. The object and
+ * lock owner objects have one of these embedded within them. Refcounts and
+ * other fields common to both owner types are contained within these
+ * structures.
+ */
struct nfs4_stateowner {
struct list_head so_strhash;
struct list_head so_stateids;
@@ -354,6 +408,12 @@ struct nfs4_stateowner {
bool so_is_open_owner;
};
+/*
+ * When a file is opened, the client provides an open state owner opaque string
+ * that indicates the "owner" of that open. These objects are refcounted.
+ * References to it are held by each open state associated with it. This object
+ * is a superset of the nfs4_stateowner struct.
+ */
struct nfs4_openowner {
struct nfs4_stateowner oo_owner; /* must be first field */
struct list_head oo_perclient;
@@ -371,6 +431,12 @@ struct nfs4_openowner {
unsigned char oo_flags;
};
+/*
+ * Represents a generic "lockowner". Similar to an openowner. References to it
+ * are held by the lock stateids that are created on its behalf. This object is
+ * a superset of the nfs4_stateowner struct (or would be if it needed any extra
+ * fields).
+ */
struct nfs4_lockowner {
struct nfs4_stateowner lo_owner; /* must be first element */
};
@@ -385,7 +451,14 @@ static inline struct nfs4_lockowner * lockowner(struct nfs4_stateowner *so)
return container_of(so, struct nfs4_lockowner, lo_owner);
}
-/* nfs4_file: a file opened by some number of (open) nfs4_stateowners. */
+/*
+ * nfs4_file: a file opened by some number of (open) nfs4_stateowners.
+ *
+ * These objects are global. nfsd only keeps one instance of a nfs4_file per
+ * inode (though it may keep multiple file descriptors open per inode). These
+ * are tracked in the file_hashtbl which is protected by the state_lock
+ * spinlock.
+ */
struct nfs4_file {
atomic_t fi_ref;
spinlock_t fi_lock;
@@ -410,7 +483,20 @@ struct nfs4_file {
bool fi_had_conflict;
};
-/* "ol" stands for "Open or Lock". Better suggestions welcome. */
+/*
+ * A generic struct representing either a open or lock stateid. The nfs4_client
+ * holds a reference to each of these objects, and they in turn hold a
+ * reference to their respective stateowners. The client's reference is
+ * released in response to a close or unlock (depending on whether it's an open
+ * or lock stateid) or when the client is being destroyed.
+ *
+ * In the case of v4.0 open stateids, these objects are preserved for a little
+ * while after close in order to handle CLOSE replays. Those are eventually
+ * reclaimed via a LRU scheme by the laundromat.
+ *
+ * This object is a superset of the nfs4_stid. "ol" stands for "Open or Lock".
+ * Better suggestions welcome.
+ */
struct nfs4_ol_stateid {
struct nfs4_stid st_stid; /* must be first field */
struct list_head st_perfile;
--
1.9.3
On Wed, Jul 30, 2014 at 08:27:01AM -0400, Jeff Layton wrote:
> This patchset is the remainder of the client_mutex removal patches,
> which should be merged after the stateid and stateowner refcounting
> overhaul. The main focus of this part is to ensure that the nfs4_client
> refcounting is up to snuff and then to remove the client_mutex.
>
> There's also a series in here that basically rewrites the fault
> injection code. That rewrite has some significant holes in it (note the
> WARN_ONs when the client refcount goes too high), but it should keep it
> limping along for now.
>
> FWIW, I think we should probably go ahead and mark the fault injection
> code deprecated in the upcoming merge window and then plan to remove
> it in 2-3 releases.
I basically skipped reading the fault injection patches because I don't
care much about them.
The other patches look good to me, but I'd like to take a more careful
look at a few of them and give anyone else a chance to comment before
committing.
---b.
>
> Jeff Layton (19):
> nfsd: Protect session creation and client confirm using client_lock
> nfsd: protect the close_lru list and oo_last_closed_stid with
> client_lock
> nfsd: move unhash_client_locked call into mark_client_expired_locked
> nfsd: don't destroy client if mark_client_expired_locked fails
> nfsd: don't destroy clients that are busy
> nfsd: protect clid and verifier generation with client_lock
> nfsd: abstract out the get and set routines into the fault injection
> ops
> nfsd: add a forget_clients "get" routine with proper locking
> nfsd: add a forget_client set_clnt routine
> nfsd: add nfsd_inject_forget_clients
> nfsd: add a list_head arg to nfsd_foreach_client_lock
> nfsd: add more granular locking to forget_locks fault injector
> nfsd: add more granular locking to forget_openowners fault injector
> nfsd: add more granular locking to *_delegations fault injectors
> nfsd: remove old fault injection infrastructure
> nfsd: remove nfs4_lock_state: nfs4_laundromat
> nfsd: remove nfs4_lock_state: nfs4_state_shutdown_net
> nfsd: remove the client_mutex and the nfs4_lock/unlock_state wrappers
> nfsd: add some comments to the nfsd4 object definitions
>
> Trond Myklebust (18):
> nfsd: Ensure struct nfs4_client is unhashed before we try to destroy
> it
> nfsd: Ensure that the laundromat unhashes the client before releasing
> locks
> nfsd: Don't require client_lock in free_client
> nfsd: Move create_client() call outside the lock
> nfsd: Protect unconfirmed client creation using client_lock
> nfsd: Protect nfsd4_destroy_clientid using client_lock
> nfsd: Ensure lookup_clientid() takes client_lock
> nfsd: Add lockdep assertions to document the nfs4_client/session
> locking
> nfsd: Remove nfs4_lock_state(): nfs4_preprocess_stateid_op()
> nfsd: Remove nfs4_lock_state(): nfsd4_test_stateid/nfsd4_free_stateid
> nfsd: Remove nfs4_lock_state(): nfsd4_release_lockowner
> nfsd: Remove nfs4_lock_state(): nfsd4_lock/locku/lockt()
> nfsd: Remove nfs4_lock_state(): nfsd4_open_downgrade + nfsd4_close
> nfsd: Remove nfs4_lock_state(): nfsd4_delegreturn()
> nfsd: Remove nfs4_lock_state(): nfsd4_open and nfsd4_open_confirm
> nfsd: Remove nfs4_lock_state(): exchange_id, create/destroy_session()
> nfsd: Remove nfs4_lock_state(): setclientid, setclientid_confirm,
> renew
> nfsd: Remove nfs4_lock_state(): reclaim_complete()
>
> fs/nfsd/fault_inject.c | 130 +++----
> fs/nfsd/netns.h | 14 +-
> fs/nfsd/nfs4proc.c | 3 -
> fs/nfsd/nfs4state.c | 922 +++++++++++++++++++++++++++++++++++--------------
> fs/nfsd/state.h | 122 +++++--
> 5 files changed, 836 insertions(+), 355 deletions(-)
>
> --
> 1.9.3
>