nfsd: remaining client_mutex removal patches
Subsystems (2)
cover nfsd: remaining client_mutex removal patches
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(-)
Signature · 1 line
1.9.3
PATCH 1/37 nfsd: Ensure struct nfs4_client is unhashed before we try to destroy it
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.
--- fs/nfsd/nfs4state.c | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-)
fs/nfsd/nfs4state.c +33 -10
@@ -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)
Signature · 1 line
1.9.3
PATCH 2/37 nfsd: Ensure that the laundromat unhashes the client before releasing locks
In reply to Jeff LaytonFrom: 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.
--- fs/nfsd/nfs4state.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
fs/nfsd/nfs4state.c +3 -1
@@ -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);
Signature · 1 line
1.9.3
PATCH 3/37 nfsd: Don't require client_lock in free_client
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
The struct nfs_client is supposed to be invisible and unreferenced before it gets here.
--- fs/nfsd/nfs4state.c | 13 ------------- 1 file changed, 13 deletions(-)
fs/nfsd/nfs4state.c +0 -13
@@ -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);
Signature · 1 line
1.9.3
PATCH 4/37 nfsd: Move create_client() call outside the lock
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
For efficiency reasons, and because we want to use spin locks instead of relying on the client_mutex.
--- fs/nfsd/nfs4state.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)
fs/nfsd/nfs4state.c +19 -16
@@ -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; }
Signature · 1 line
1.9.3
PATCH 5/37 nfsd: Protect unconfirmed client creation using client_lock
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
...instead of relying on the client_mutex.
--- fs/nfsd/nfs4state.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-)
fs/nfsd/nfs4state.c +22 -11
@@ -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; }
Signature · 1 line
1.9.3
PATCH 6/37 nfsd: Protect session creation and client confirm using client_lock
In reply to Jeff LaytonIn 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.
--- fs/nfsd/nfs4state.c | 65 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 26 deletions(-)
fs/nfsd/nfs4state.c +39 -26
@@ -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 ;
Signature · 1 line
1.9.3
PATCH 7/37 nfsd: Protect nfsd4_destroy_clientid using client_lock
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
...instead of relying on the client_mutex.
--- fs/nfsd/nfs4state.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
fs/nfsd/nfs4state.c +9 -4
@@ -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; }
Signature · 1 line
1.9.3
PATCH 8/37 nfsd: Ensure lookup_clientid() takes client_lock
In reply to Jeff LaytonFrom: 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.
--- fs/nfsd/nfs4state.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
fs/nfsd/nfs4state.c +6 -2
@@ -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; }
Signature · 1 line
1.9.3
PATCH 9/37 nfsd: Add lockdep assertions to document the nfs4_client/session locking
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
--- fs/nfsd/nfs4state.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
fs/nfsd/nfs4state.c +30 -0
@@ -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)
Signature · 1 line
1.9.3
Re: [PATCH 09/37] nfsd: Add lockdep assertions to document the nfs4_client/session locking
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.
Quoted 139 lines · Jeff Layton
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(+)fs/nfsd/nfs4state.c +30
-0@@ -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)Signature · 6 lines
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
PATCH 10/37 nfsd: protect the close_lru list and oo_last_closed_stid with client_lock
In reply to Jeff LaytonCurrently, 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.
--- fs/nfsd/nfs4state.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-)
fs/nfsd/nfs4state.c +31 -7
@@ -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;
Signature · 1 line
1.9.3
PATCH 11/37 nfsd: move unhash_client_locked call into mark_client_expired_locked
In reply to Jeff LaytonAll 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.
--- fs/nfsd/nfs4state.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)
fs/nfsd/nfs4state.c +8 -11
@@ -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);
Signature · 1 line
1.9.3
PATCH 12/37 nfsd: don't destroy client if mark_client_expired_locked fails
In reply to Jeff LaytonIf 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.
--- fs/nfsd/nfs4state.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
fs/nfsd/nfs4state.c +6 -2
@@ -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;
Signature · 1 line
1.9.3
PATCH 13/37 nfsd: don't destroy clients that are busy
In reply to Jeff LaytonIt'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.
--- fs/nfsd/nfs4state.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
fs/nfsd/nfs4state.c +8 -2
@@ -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;
Signature · 1 line
1.9.3
PATCH 14/37 nfsd: protect clid and verifier generation with client_lock
In reply to Jeff LaytonThe 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.
--- fs/nfsd/netns.h | 6 +++--- fs/nfsd/nfs4state.c | 21 +++++++++------------ 2 files changed, 12 insertions(+), 15 deletions(-)
fs/nfsd/netns.h +3 -3
@@ -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; };
fs/nfsd/nfs4state.c +9 -12
@@ -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;
Signature · 1 line
1.9.3
PATCH 15/37 nfsd: abstract out the get and set routines into the fault injection ops
In reply to Jeff LaytonNow 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.
--- fs/nfsd/fault_inject.c | 129 ++++++++++++++++++++++++++++++------------------- 1 file changed, 78 insertions(+), 51 deletions(-)
fs/nfsd/fault_inject.c +78 -51
@@ -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;
Signature · 1 line
1.9.3
PATCH 16/37 nfsd: add a forget_clients "get" routine with proper locking
In reply to Jeff LaytonAdd a new "get" routine for forget_clients that relies on the client_lock instead of the client_mutex.
--- fs/nfsd/fault_inject.c | 3 +-- fs/nfsd/nfs4state.c | 30 ++++++++++++++++++++++-------- fs/nfsd/state.h | 4 +++- 3 files changed, 26 insertions(+), 11 deletions(-)
fs/nfsd/fault_inject.c +1 -2
@@ -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" ,
fs/nfsd/nfs4state.c +22 -8
@@ -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) {
fs/nfsd/state.h +3 -1
@@ -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 );
Signature · 1 line
1.9.3
PATCH 17/37 nfsd: add a forget_client set_clnt routine
In reply to Jeff Layton...that relies on the client_lock instead of client_mutex.
--- fs/nfsd/fault_inject.c | 2 +- fs/nfsd/nfs4state.c | 28 ++++++++++++++++++++++++++++ fs/nfsd/state.h | 3 +++ 3 files changed, 32 insertions(+), 1 deletion(-)
fs/nfsd/fault_inject.c +1 -1
@@ -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, }, {
fs/nfsd/nfs4state.c +28 -0
@@ -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) {
fs/nfsd/state.h +3 -0
@@ -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 );
Signature · 1 line
1.9.3
PATCH 18/37 nfsd: add nfsd_inject_forget_clients
In reply to Jeff Layton...which uses the client_lock for protection instead of client_mutex. Also remove nfsd_forget_client as there are no more callers.
--- fs/nfsd/fault_inject.c | 3 +-- fs/nfsd/nfs4state.c | 42 ++++++++++++++++++++++++++++-------------- fs/nfsd/state.h | 2 +- 3 files changed, 30 insertions(+), 17 deletions(-)
fs/nfsd/fault_inject.c +1 -2
@@ -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",
fs/nfsd/nfs4state.c +28 -14
54 lines not on this page · raw message
fs/nfsd/state.h +1 -1
@@ -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 );
Signature · 1 line
1.9.3
PATCH 19/37 nfsd: add a list_head arg to nfsd_foreach_client_lock
In reply to Jeff LaytonIn 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.
--- fs/nfsd/nfs4state.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
fs/nfsd/nfs4state.c +8 -3
@@ -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; }
Signature · 1 line
1.9.3
PATCH 20/37 nfsd: add more granular locking to forget_locks fault injector
In reply to Jeff Layton...instead of relying on the client_mutex.
--- fs/nfsd/fault_inject.c | 8 ++- fs/nfsd/nfs4state.c | 132 +++++++++++++++++++++++++++++++++++++++++++++---- fs/nfsd/state.h | 7 ++- 3 files changed, 131 insertions(+), 16 deletions(-)
fs/nfsd/fault_inject.c +3 -5
@@ -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",
fs/nfsd/nfs4state.c +123 -9
172 lines not on this page · raw message
fs/nfsd/state.h +5 -2
@@ -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 */
Signature · 1 line
1.9.3
PATCH 21/37 nfsd: add more granular locking to forget_openowners fault injector
In reply to Jeff Layton...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.
--- fs/nfsd/fault_inject.c | 8 ++-- fs/nfsd/nfs4state.c | 122 +++++++++++++++++++++++++++++++++++++++++++++---- fs/nfsd/state.h | 7 ++- 3 files changed, 122 insertions(+), 15 deletions(-)
fs/nfsd/fault_inject.c +3 -5
@@ -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",
fs/nfsd/nfs4state.c +114 -8
144 lines not on this page · raw message
fs/nfsd/state.h +5 -2
@@ -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 ; }
Signature · 1 line
1.9.3
PATCH 22/37 nfsd: add more granular locking to *_delegations fault injectors
In reply to Jeff Layton...instead of relying on the client_mutex.
--- fs/nfsd/fault_inject.c | 16 ++--- fs/nfsd/nfs4state.c | 175 +++++++++++++++++++++++++++++++++++++++++-------- fs/nfsd/state.h | 11 ++-- 3 files changed, 162 insertions(+), 40 deletions(-)
fs/nfsd/fault_inject.c +6 -10
@@ -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, }, };
fs/nfsd/nfs4state.c +149 -26
227 lines not on this page · raw message
fs/nfsd/state.h +7 -4
@@ -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 ) {}
Signature · 1 line
1.9.3
PATCH 23/37 nfsd: remove old fault injection infrastructure
In reply to Jeff LaytonRemove 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.
--- fs/nfsd/fault_inject.c | 51 ++++------------------------- fs/nfsd/nfs4state.c | 87 +++++++++++++++++++------------------------------- fs/nfsd/state.h | 45 +++++++++++--------------- 3 files changed, 57 insertions(+), 126 deletions(-)
fs/nfsd/fault_inject.c +6 -45
@@ -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 */
fs/nfsd/nfs4state.c +33 -54
178 lines not on this page · raw message
fs/nfsd/state.h +18 -27
53 lines not on this page · raw message
Signature · 1 line
1.9.3
PATCH 24/37 nfsd: Remove nfs4_lock_state(): nfs4_preprocess_stateid_op()
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
--- fs/nfsd/nfs4state.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
fs/nfsd/nfs4state.c +1 -5
@@ -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; }
Signature · 1 line
1.9.3
PATCH 25/37 nfsd: Remove nfs4_lock_state(): nfsd4_test_stateid/nfsd4_free_stateid
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
--- fs/nfsd/nfs4state.c | 4 ---- 1 file changed, 4 deletions(-)
fs/nfsd/nfs4state.c +0 -4
@@ -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; }
Signature · 1 line
1.9.3
PATCH 26/37 nfsd: Remove nfs4_lock_state(): nfsd4_release_lockowner
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
--- fs/nfsd/nfs4state.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
fs/nfsd/nfs4state.c +2 -6
@@ -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; }
Signature · 1 line
1.9.3
PATCH 27/37 nfsd: Remove nfs4_lock_state(): nfsd4_lock/locku/lockt()
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
--- fs/nfsd/nfs4state.c | 9 --------- 1 file changed, 9 deletions(-)
fs/nfsd/nfs4state.c +0 -9
@@ -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;
Signature · 1 line
1.9.3
PATCH 28/37 nfsd: Remove nfs4_lock_state(): nfsd4_open_downgrade + nfsd4_close
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
--- fs/nfsd/nfs4state.c | 4 ---- 1 file changed, 4 deletions(-)
fs/nfsd/nfs4state.c +0 -4
@@ -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; }
Signature · 1 line
1.9.3
PATCH 29/37 nfsd: Remove nfs4_lock_state(): nfsd4_delegreturn()
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
--- fs/nfsd/nfs4state.c | 3 --- 1 file changed, 3 deletions(-)
fs/nfsd/nfs4state.c +0 -3
@@ -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; }
Signature · 1 line
1.9.3
PATCH 30/37 nfsd: Remove nfs4_lock_state(): nfsd4_open and nfsd4_open_confirm
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
--- fs/nfsd/nfs4proc.c | 3 --- fs/nfsd/nfs4state.c | 6 ------ 2 files changed, 9 deletions(-)
fs/nfsd/nfs4proc.c +0 -3
@@ -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; }
fs/nfsd/nfs4state.c +0 -6
@@ -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; }
Signature · 1 line
1.9.3
PATCH 31/37 nfsd: Remove nfs4_lock_state(): exchange_id, create/destroy_session()
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
Also destroy_clientid and bind_conn_to_session.
--- fs/nfsd/nfs4state.c | 11 ----------- 1 file changed, 11 deletions(-)
fs/nfsd/nfs4state.c +0 -11
@@ -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 *cstaunhash_client_locked (clp); out:spin_unlock (&nn->client_lock); -nfs4_unlock_state ();if (clp)expire_client (clp);return status;
Signature · 1 line
1.9.3
PATCH 32/37 nfsd: Remove nfs4_lock_state(): setclientid, setclientid_confirm, renew
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
--- fs/nfsd/nfs4state.c | 6 ------ 1 file changed, 6 deletions(-)
fs/nfsd/nfs4state.c +0 -6
@@ -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; }
Signature · 1 line
1.9.3
PATCH 33/37 nfsd: Remove nfs4_lock_state(): reclaim_complete()
In reply to Jeff LaytonFrom: Trond Myklebust <[email protected]>
--- fs/nfsd/nfs4state.c | 2 -- 1 file changed, 2 deletions(-)
fs/nfsd/nfs4state.c +0 -2
@@ -2905,7 +2905,6 @@ nfsd4_reclaim_complete(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstareturn 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; }
Signature · 1 line
1.9.3
PATCH 34/37 nfsd: remove nfs4_lock_state: nfs4_laundromat
In reply to Jeff Layton--- fs/nfsd/nfs4state.c | 3 --- 1 file changed, 3 deletions(-)
fs/nfsd/nfs4state.c +0 -3
@@ -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; }
Signature · 1 line
1.9.3
PATCH 35/37 nfsd: remove nfs4_lock_state: nfs4_state_shutdown_net
In reply to Jeff Layton--- fs/nfsd/nfs4state.c | 2 -- 1 file changed, 2 deletions(-)
fs/nfsd/nfs4state.c +0 -2
@@ -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
Signature · 1 line
1.9.3
PATCH 36/37 nfsd: remove the client_mutex and the nfs4_lock/unlock_state wrappers
In reply to Jeff Layton--- fs/nfsd/nfs4state.c | 15 --------------- 1 file changed, 15 deletions(-)
fs/nfsd/nfs4state.c +0 -15
@@ -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_breturn 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 ;
Signature · 1 line
1.9.3
Re: [PATCH 36/37] nfsd: remove the client_mutex and the nfs4_lock/unlock_state wrappers
Quoted 35 lines · 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(-)fs/nfsd/nfs4state.c +0
-9@@ -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_breturn nfs_ok; }
fs/nfsd/nfs4state.c static __be32 mark_session_dead_locked(struct nfsd4_session *ses, int ref_held_b · line 114
Quoted 10 lines
fs/nfsd/nfs4state.c +0
-6-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,
Signature · 1 line
Jeff Layton <[email protected]>
Re: [PATCH 36/37] nfsd: remove the client_mutex and the nfs4_lock/unlock_state wrappers
Quoted 3 lines · Jeff Layton
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.
PATCH 37/37 nfsd: add some comments to the nfsd4 object definitions
In reply to Jeff LaytonAdd some comments that describe what each of these objects is, and how they related to one another.
--- fs/nfsd/netns.h | 8 +++++ fs/nfsd/state.h | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 92 insertions(+), 7 deletions(-)
fs/nfsd/netns.h +8 -0
@@ -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;
fs/nfsd/state.h +84 -7
147 lines not on this page · raw message
Signature · 1 line
1.9.3
Re: [PATCH 37/37] nfsd: add some comments to the nfsd4 object definitions
Thanks for the documentation! A couple comments:
Quoted 14 lines · Jeff Layton
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(-)fs/nfsd/netns.h +0
-0
fs/nfsd/netns.h line 42
Quoted 9 lines
fs/nfsd/netns.h +6
-0@@ -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"?
Quoted 22 lines
fs/nfsd/netns.h +2
-0+* clean up expired clients and delegations within the container. +*/ struct nfsd_net {struct cld_net *cld_net;fs/nfsd/state.h +5
-0@@ -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
fs/nfsd/state.h struct nfs4_stid { · line 100
Quoted 7 lines
fs/nfsd/state.h +4
-0@@ -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.
Quoted 142 lines
fs/nfsd/state.h +74
-7+* 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;Signature · 6 lines
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
Re: [PATCH 37/37] nfsd: add some comments to the nfsd4 object definitions
Quoted 20 lines · On Tue, 5 Aug 2014 11:36:28 -0400 "J. Bruce Fields" <[email protected]>…
Thanks for the documentation! A couple comments:
Quoted 17 lines · Jeff Layton
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(-)fs/nfsd/netns.h +0
-0@@ -34,6 +34,14 @@struct cld_net ;struct nfsd4_client_tracking_ops ;
Quoted 10 lines
fs/nfsd/netns.h line 42
Quoted 6 lines
fs/nfsd/netns.h +6
-0+/* +* 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...
Quoted 33 lines
Quoted 22 lines
fs/nfsd/netns.h +2
-0+* clean up expired clients and delegations within the container. +*/ struct nfsd_net {struct cld_net *cld_net;fs/nfsd/state.h +5
-0@@ -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 1fs/nfsd/state.h struct nfs4_stid { · line 100
Quoted 7 lines
fs/nfsd/state.h +4
-0@@ -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:
Quoted 8 lines
- 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 holdingcl_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?
Quoted 5 lines
(By the way, I wonder if that
list_for_each_entry()at the end ofnfsd4_process_cb_updateactually does anything: hasn'tnfsd4_cb_recall_release()already run and removed any delegation from thecl_callbackslist? 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...
Quoted 144 lines
--b.
Quoted 142 lines
fs/nfsd/state.h +74
-7+* 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;Signature · 6 lines
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
Signature · 1 line
Jeff Layton <[email protected]>
Re: [PATCH 37/37] nfsd: add some comments to the nfsd4 object definitions
Quoted 78 lines · Jeff Layton
Quoted 20 lines · On Tue, 5 Aug 2014 11:36:28 -0400 "J. Bruce Fields" <[email protected]>…
Thanks for the documentation! A couple comments:
Quoted 17 lines · Jeff Layton
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(-)fs/nfsd/netns.h +0
-0@@ -34,6 +34,14 @@struct cld_net ;struct nfsd4_client_tracking_ops ;Quoted 10 lines
fs/nfsd/netns.h line 42
Quoted 6 lines
fs/nfsd/netns.h +6
-0+/* +* 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...
Quoted 33 lines
Quoted 22 lines
fs/nfsd/netns.h +2
-0+* clean up expired clients and delegations within the container. +*/ struct nfsd_net {struct cld_net *cld_net;fs/nfsd/state.h +5
-0@@ -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 1fs/nfsd/state.h struct nfs4_stid { · line 100
Quoted 7 lines
fs/nfsd/state.h +4
-0@@ -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:
Quoted 8 lines
- 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 holdingcl_lock?
Quoted 2 lines
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?
Quoted 6 lines
Quoted 5 lines
(By the way, I wonder if that
list_for_each_entry()at the end ofnfsd4_process_cb_updateactually does anything: hasn'tnfsd4_cb_recall_release()already run and removed any delegation from thecl_callbackslist? I may just be forgetting how this works.)
Quoted 8 lines
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_doneis true, and that doesn't happen if there was an error anddl_retriesisn'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.
PATCH v2 1/1 nfsd: add some comments to the nfsd4 object definitions
In reply to Jeff LaytonAdd some comments that describe what each of these objects is, and how they related to one another.
--- fs/nfsd/netns.h | 8 +++++ fs/nfsd/state.h | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 101 insertions(+), 7 deletions(-)
fs/nfsd/netns.h +8 -0
@@ -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;
fs/nfsd/state.h +93 -7
156 lines not on this page · raw message
Signature · 1 line
1.9.3
Re: [PATCH v2] nfsd: add some comments to the nfsd4 object definitions
Quoted 2 lines · Jeff Layton
Add some comments that describe what each of these objects is, and how they related to one another.
Thanks!--b.
Quoted 198 lines
Signed-off-by Jeff Layton <[email protected]>--- fs/nfsd/netns.h | 8 +++++ fs/nfsd/state.h | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 101 insertions(+), 7 deletions(-)fs/nfsd/netns.h +8
-0@@ -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;fs/nfsd/state.h +93
-7@@ -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;Signature · 1 line
1.9.3
Quoted 5 lines · Jeff Layton
This patchset is the remainder of the
client_mutexremoval patches, which should be merged after the stateid and stateowner refcounting overhaul. The main focus of this part is to ensure that thenfs4_clientrefcounting is up to snuff and then to remove theclient_mutex.
Quoted 8 lines
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_ONswhen 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.
Quoted 58 lines
Jeff Layton (19):
nfsd: Protect session creation and client confirm usingclient_lock
nfsd: protect theclose_lrulist andoo_last_closed_stidwith
client_lock
nfsd: moveunhash_client_lockedcall intomark_client_expired_locked
nfsd: don't destroy client ifmark_client_expired_lockedfails
nfsd: don't destroy clients that are busy
nfsd: protect clid and verifier generation withclient_lock
nfsd: abstract out the get and set routines into the fault injection
ops
nfsd: add aforget_clients"get" routine with proper locking
nfsd: add aforget_clientset_clntroutine
nfsd: addnfsd_inject_forget_clients
nfsd: add alist_headarg tonfsd_foreach_client_lock
nfsd: add more granular locking toforget_locksfault injector
nfsd: add more granular locking toforget_openownersfault injector
nfsd: add more granular locking to*_delegationsfault injectors
nfsd: remove old fault injection infrastructure
nfsd: removenfs4_lock_state:nfs4_laundromat
nfsd: removenfs4_lock_state:nfs4_state_shutdown_net
nfsd: remove theclient_mutexand the nfs4_lock/unlock_state wrappers
nfsd: add some comments to the nfsd4 object definitionsTrond Myklebust (18):
nfsd: Ensure structnfs4_clientis unhashed before we try to destroy
it
nfsd: Ensure that the laundromat unhashes the client before releasing
locks
nfsd: Don't requireclient_lockinfree_client
nfsd: Movecreate_client()call outside the lock
nfsd: Protect unconfirmed client creation usingclient_lock
nfsd: Protectnfsd4_destroy_clientidusingclient_lock
nfsd: Ensurelookup_clientid()takesclient_lock
nfsd: Add lockdep assertions to document the nfs4_client/session
locking
nfsd: Removenfs4_lock_state():nfs4_preprocess_stateid_op()
nfsd: Removenfs4_lock_state(): nfsd4_test_stateid/nfsd4_free_stateid
nfsd: Removenfs4_lock_state():nfsd4_release_lockowner
nfsd: Removenfs4_lock_state(): nfsd4_lock/locku/lockt()
nfsd: Removenfs4_lock_state():nfsd4_open_downgrade+nfsd4_close
nfsd: Removenfs4_lock_state():nfsd4_delegreturn()
nfsd: Removenfs4_lock_state():nfsd4_openandnfsd4_open_confirm
nfsd: Removenfs4_lock_state():exchange_id, create/destroy_session()
nfsd: Removenfs4_lock_state(): setclientid,setclientid_confirm,
renew
nfsd: Removenfs4_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(-)Signature · 1 line
1.9.3